Skip dummy item creation when removing invalid element from tree #3525

Due to the fix for Bug 210747, a dummy tree-item is added to the viewer
if the same element is removed twice. The intend of the original fix is
to remove the "expand" icon from a tree item, when its still collapsed
children are removed from the viewer (but not the model).

The problem with this approach is that it doesn't take the situation
into consideration, where the children of the item have already been
removed.

Given a tree item with at least one child in the data model but where
all child-widgets have been removed in the viewer. When removing the
children again, this internally calls `updatePlus(Item,Object)`.

But because the number of child-widgets doesn't match the number of
children, this method now creates a dummy item, to correct this
inconsistency. This problem doesn't occur when calling
`internalRemove(Object,Object[])`, because here the call to
`updatePlus(Item,Object)` is guarded by a check to make sure the tree
item has at least one child-widget.

To reproduce:

1) Make sure the ".* resources" filter is enabled
2) Create a new project "Project A" and "Project B"
3) Create a new text file "test" in "Project A"
4) Move "test" to "Project B"

Closes #3525

Author: ptziegler

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2019 IBM Corporation and others.
+ * Copyright (c) 2000, 2025 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -2095,12 +2095,7 @@ protected void internalRemove(Object[] elementsOrPaths) {
 				Object parent = getParentElement(element);
 				if (parent != null && !equals(parent, getRoot())
 						&& !(parent instanceof TreePath tp && tp.getSegmentCount() == 0)) {
-					Widget[] parentItems = internalFindItems(parent);
-					for (Widget parentItem : parentItems) {
-						if (parentItem instanceof Item it) {
-							updatePlus(it, parent);
-						}
-					}
+					internalRemove(parent, new Object[] { element });
 				}
 			}
 		}

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2006 IBM Corporation and others.
+ * Copyright (c) 2000, 2025 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -14,7 +14,10 @@
 package org.eclipse.jface.tests.viewers;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -114,4 +117,43 @@ protected void internalExpandToLevel(Widget widget, int level) {
 		}
 	}
 
+	/**
+	 * Removing the same element twice should not produce a dummy tree-item.
+	 */
+	@Test
+	public void testIssue3525() {
+		TestElement modelRoot = TestElement.createModel(2, 1);
+		TestElement modelParent = modelRoot.getChildAt(0);
+		TestElement modelChild = modelParent.getChildAt(0);
+		fTreeViewer.setInput(modelRoot);
+		fTreeViewer.expandAll();
+
+		TreeItem widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent);
+		TreeItem widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild);
+		assertNotNull(widgetParent);
+		assertNotNull(widgetChild);
+		assertArrayEquals(widgetParent.getItems(), new TreeItem[] { widgetChild });
+
+		// This workaround is needed because of TreeViewerWithLimitCompatibilityTest
+		// When calling setDisplayIncrementally(...) with a positive number, you are
+		// no longer able to remove elements from the viewer without first removing
+		// them from the model
+		modelParent.fChildren.remove(modelChild);
+		fTreeViewer.remove(modelChild);
+		modelParent.fChildren.add(modelChild);
+
+		widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent);
+		widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild);
+		assertNotNull(widgetParent);
+		assertNull(widgetChild);
+		assertArrayEquals(widgetParent.getItems(), new TreeItem[0]);
+
+		fTreeViewer.remove(modelChild);
+
+		widgetParent = (TreeItem) fTreeViewer.testFindItem(modelParent);
+		widgetChild = (TreeItem) fTreeViewer.testFindItem(modelChild);
+		assertNotNull(widgetParent);
+		assertNull(widgetChild);
+		assertArrayEquals(widgetParent.getItems(), new TreeItem[0]);
+	}
 }