ViewerCell: eager NPE, final members #177

Do not reuse ViewerCell instance - that only caused trouble and was
clearly forbidden by
org.eclipse.jface.viewers.ViewerColumn.refresh(ViewerCell).
Instead let the GC do his job.

Declaring the constants final formally requires API change. But it was
never intended that the constants could be changed.

#177

Author: EcljpseB0T

bundles/org.eclipse.jface/.settings/.api_filters

@@ -0,0 +1,29 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.jface" version="2">
+    <resource path="src/org/eclipse/jface/viewers/ViewerCell.java" type="org.eclipse.jface.viewers.ViewerCell">
+        <filter comment="constants should be final" id="388100214">
+            <message_arguments>
+                <message_argument value="org.eclipse.jface.viewers.ViewerCell"/>
+                <message_argument value="ABOVE"/>
+            </message_arguments>
+        </filter>
+        <filter comment="constants should be final" id="388100214">
+            <message_arguments>
+                <message_argument value="org.eclipse.jface.viewers.ViewerCell"/>
+                <message_argument value="BELOW"/>
+            </message_arguments>
+        </filter>
+        <filter comment="constants should be final" id="388100214">
+            <message_arguments>
+                <message_argument value="org.eclipse.jface.viewers.ViewerCell"/>
+                <message_argument value="LEFT"/>
+            </message_arguments>
+        </filter>
+        <filter comment="constants should be final" id="388100214">
+            <message_arguments>
+                <message_argument value="org.eclipse.jface.viewers.ViewerCell"/>
+                <message_argument value="RIGHT"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTableViewer.java

@@ -405,19 +405,10 @@ protected void doUpdateItem(Widget widget, Object element, boolean fullMap) {
 					}
 
 					ViewerColumn columnViewer = getViewerColumn(column);
-					ViewerCell cellToUpdate = updateCell(viewerRowFromItem,
-							column, element);
-
-					// If the control is virtual, we cannot use the cached cell object. See bug 188663.
-					if (isVirtual) {
-						cellToUpdate = new ViewerCell(cellToUpdate.getViewerRow(), cellToUpdate.getColumnIndex(), element);
-					}
+					ViewerCell cellToUpdate = new ViewerCell(viewerRowFromItem, column, element);
 
 					columnViewer.refresh(cellToUpdate);
 
-					// clear cell (see bug 201280)
-					updateCell(null, 0, null);
-
 					// As it is possible for user code to run the event
 					// loop check here.
 					if (item.isDisposed()) {

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java

@@ -977,19 +977,10 @@ protected void doUpdateItem(final Item item, Object element) {
 			}
 
 			ViewerColumn columnViewer = getViewerColumn(column);
-			ViewerCell cellToUpdate = updateCell(viewerRowFromItem, column,
-					element);
-
-			// If the control is virtual, we cannot use the cached cell object. See bug 188663.
-			if (isVirtual) {
-				cellToUpdate = new ViewerCell(cellToUpdate.getViewerRow(), cellToUpdate.getColumnIndex(), element);
-			}
+			ViewerCell cellToUpdate = new ViewerCell(viewerRowFromItem, column, element);
 
 			columnViewer.refresh(cellToUpdate);
 
-			// clear cell (see bug 201280)
-			updateCell(null, 0, null);
-
 			// As it is possible for user code to run the event
 			// loop check here.
 			if (item.isDisposed()) {

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewer.java

@@ -64,11 +64,6 @@ public abstract class ColumnViewer extends StructuredViewer {
 
 	private String[] columnProperties;
 
-	/**
-	 * The cell is a cached viewer cell used for refreshing.
-	 */
-	private ViewerCell cell = new ViewerCell(null, 0, null);
-
 	private ColumnViewerEditor viewerEditor;
 
 	private boolean busy;
@@ -307,19 +302,6 @@ private ViewerColumn createViewerColumn(Widget columnOwner,
 		return column;
 	}
 
-	/**
-	 * Update the cached cell object with the given row and column.
-	 *
-	 * @param rowItem
-	 * @param column
-	 * @return ViewerCell
-	 */
-	/* package */ViewerCell updateCell(ViewerRow rowItem, int column,
-			Object element) {
-		cell.update(rowItem, column, element);
-		return cell;
-	}
-
 	/**
 	 * Returns the {@link Item} at the given widget-relative coordinates, or
 	 * <code>null</code> if there is no item at the given coordinates.

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ViewerCell.java

@@ -34,31 +34,31 @@
  *
  */
 public class ViewerCell {
-	private int columnIndex;
+	private final int columnIndex;
 
-	private ViewerRow row;
+	private final ViewerRow row;
 
-	private Object element;
+	private final Object element;
 
 	/**
 	 * Constant denoting the cell above current one (value is 1).
 	 */
-	public static int ABOVE = 1;
+	public static final int ABOVE = 1;
 
 	/**
 	 * Constant denoting the cell below current one (value is 2).
 	 */
-	public static int BELOW = 1 << 1;
+	public static final int BELOW = 1 << 1;
 
 	/**
 	 * Constant denoting the cell to the left of the current one (value is 4).
 	 */
-	public static int LEFT = 1 << 2;
+	public static final int LEFT = 1 << 2;
 
 	/**
 	 * Constant denoting the cell to the right of the current one (value is 8).
 	 */
-	public static int RIGHT = 1 << 3;
+	public static final int RIGHT = 1 << 3;
 
 	/**
 	 * Create a new instance of the receiver on the row.
@@ -67,6 +67,7 @@ public class ViewerCell {
 	 * @param columnIndex
 	 */
 	ViewerCell(ViewerRow row, int columnIndex, Object element) {
+		Objects.requireNonNull(row);
 		this.row = row;
 		this.columnIndex = columnIndex;
 		this.element = element;
@@ -200,29 +201,6 @@ public StyleRange[] getStyleRanges() {
 		return row.getStyleRanges(columnIndex);
 	}
 
-	/**
-	 * Set the columnIndex.
-	 *
-	 * @param column the column index to set
-	 */
-	void setColumn(int column) {
-		columnIndex = column;
-
-	}
-
-	/**
-	 * Set the row to rowItem and the columnIndex to column.
-	 *
-	 * @param rowItem the row item to set
-	 * @param column  the column index to set
-	 * @param element the element to set
-	 */
-	void update(ViewerRow rowItem, int column, Object element) {
-		row = rowItem;
-		columnIndex = column;
-		this.element = element;
-	}
-
 	/**
 	 * Return the item for the receiver.
 	 *

tests/org.eclipse.jface.tests/META-INF/MANIFEST.MF

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-SymbolicName: org.eclipse.jface.tests
-Bundle-Version: 1.4.200.qualifier
+Bundle-Version: 1.4.300.qualifier
 Automatic-Module-Name: org.eclipse.jface.tests
 Bundle-RequiredExecutionEnvironment: JavaSE-17
 Require-Bundle: org.junit;bundle-version="4.12.0",

tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/viewers/AllViewersTests.java

@@ -28,7 +28,7 @@
 		CComboViewerTest.class, TreeViewerComparatorTest.class, ListViewerComparatorTest.class,
 		TableViewerComparatorTest.class, Bug138608Test.class, ComboViewerComparerTest.class,
 		ListViewerRefreshTest.class, Bug200558Test.class, Bug201002TableViewerTest.class, Bug201002TreeViewerTest.class,
-		Bug200337TableViewerTest.class, Bug203657TreeViewerTest.class, Bug203657TableViewerTest.class,
+		Bug200337TableViewerTest.class, //
 		Bug205700TreeViewerTest.class, Bug180504TableViewerTest.class, Bug180504TreeViewerTest.class,
 		Bug256889TableViewerTest.class, Bug287765Test.class, Bug242231Test.class, StyledStringBuilderTest.class,
 		TreeViewerWithLimitTest.class, TreeViewerWithLimitCompatibilityTest.class, TableViewerWithLimitTest.class,