Restore recursive behavior of AbstractTreeViwer#internalExpandToLevel()

The method AbstractTreeViewer#internalExpandToLevel() used be a
recursively called method (by implementation and specification). As a
protected method, it is part of the API.
With a recent change, the method was changed to delegate to a different
method, removing it's recursive property. Consumers that have created
subtypes of the AbstractTreeViewer and rely the recursive behavior of
that method by overwriting it face a regression by that change.

This change restores the existing, recursive behavior of that method
while still keeping the reuse of the implementation across other use
cases. A test case ensuring the contractual recursive execution is
added.

Author: HeikoKlare

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

@@ -28,7 +28,6 @@
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.function.Function;
 
 import org.eclipse.core.runtime.Assert;
 import org.eclipse.core.runtime.IStatus;
@@ -1833,23 +1832,22 @@ protected Widget internalGetWidgetToSelect(Object elementOrTreePath) {
 	}
 
 	/**
-	 * Recursively, conditionally expands the subtree rooted at the given widget to
-	 * the given level. Takes the {@code shouldChildrenExpand} predicate that
-	 * defines for a given widget if it shall be expanded.
+	 * Recursively expands the subtree rooted at the given widget to the given level
+	 * based on the {@code childExpansionFunction} function being executed for a
+	 * child to be (potentially conditionally) expanded.
 	 * <p>
 	 * Note that the default implementation of this method does not call
 	 * {@code setRedraw}.
 	 * </p>
 	 *
-	 * @param widget               the widget
-	 * @param level                non-negative level, or {@code ALL_LEVELS} to
-	 *                             expand all levels of the tree
-	 * @param shouldChildrenExpand predicate that defines for a given widget if it
-	 *                             should be expanded.
-	 * @since 3.32
+	 * @param widget                    the widget
+	 * @param level                     non-negative level, or {@code ALL_LEVELS} to
+	 *                                  expand all levels of the tree
+	 * @param childrenExpansionFunction function to be called on a child to be
+	 *                                  expanded
 	 */
-	private void internalConditionalExpandToLevel(Widget widget, int level,
-			Function<Widget, Boolean> shouldChildrenExpand) {
+	private void internalCustomizedExpandToLevel(Widget widget, int level,
+			CustomChildrenExpansionFunction childrenExpansionFunction) {
 		if (level == ALL_LEVELS || level > 0) {
 			Object data = widget.getData();
 			if (widget instanceof Item it && data != null && !isExpandable(it, null, data)) {
@@ -1861,19 +1859,18 @@ private void internalConditionalExpandToLevel(Widget widget, int level,
 				setExpanded(it, true);
 			}
 			if (level == ALL_LEVELS || level > 1) {
-				Item[] children = getChildren(widget);
-				if (children != null && shouldChildrenExpand.apply(widget).booleanValue()) {
-					int newLevel = (level == ALL_LEVELS ? ALL_LEVELS
-							: level - 1);
-					for (Item element : children) {
-						internalConditionalExpandToLevel(element, newLevel, shouldChildrenExpand);
-					}
-				}
+				int newLevel = (level == ALL_LEVELS ? ALL_LEVELS : level - 1);
+				childrenExpansionFunction.expandChildren(widget, newLevel);
 			}
 			// XXX expanding here fails on linux
 		}
 	}
 
+	@FunctionalInterface
+	private interface CustomChildrenExpansionFunction {
+		void expandChildren(Widget parent, int previousLevel);
+	}
+
 	/**
 	 * Recursively expands the subtree rooted at the given widget to the given
 	 * level.
@@ -1887,7 +1884,14 @@ private void internalConditionalExpandToLevel(Widget widget, int level,
 	 *               levels of the tree
 	 */
 	protected void internalExpandToLevel(Widget widget, int level) {
-		internalConditionalExpandToLevel(widget, level, w -> Boolean.TRUE);
+		internalCustomizedExpandToLevel(widget, level, (parent, newLevel) -> {
+			Item[] children = getChildren(parent);
+			if (children != null) {
+				for (Item child : children) {
+					internalExpandToLevel(child, newLevel);
+				}
+			}
+		});
 	}
 
 	/**
@@ -2532,14 +2536,22 @@ public void treeCollapsed(TreeExpansionEvent event) {
 			@Override
 			public void treeExpanded(TreeExpansionEvent e) {
 				Widget item = doFindItem(e.getElement());
-
-				internalConditionalExpandToLevel(item, autoExpandOnSingleChildLevels,
-						w -> Boolean.valueOf(doesWidgetHaveExactlyOneChild(w)));
+				internalCustomizedExpandToLevel(item, autoExpandOnSingleChildLevels, singleChildExpansionFunction);
 			}
 		};
 		addTreeListener(autoExpandOnSingleChildListener);
 	}
 
+	private CustomChildrenExpansionFunction singleChildExpansionFunction = (widget, newLevel) -> {
+		Item[] children = getChildren(widget);
+		boolean hasExactlyOneChild = children != null && children.length == 1;
+		if (hasExactlyOneChild) {
+			for (Item child : children) {
+				internalCustomizedExpandToLevel(child, newLevel, this.singleChildExpansionFunction);
+			}
+		}
+	};
+
 	private void removeAutoExpandOnSingleChildListener() {
 		if (autoExpandOnSingleChildListener != null) {
 			removeTreeListener(autoExpandOnSingleChildListener);
@@ -2702,8 +2714,7 @@ public void setExpandedStateWithAutoExpandOnSingleChild(Object elementOrTreePath
 		Widget item = internalGetWidgetToSelect(elementOrTreePath);
 
 		if (autoExpandOnSingleChildLevels != NO_EXPAND && expanded) {
-			internalConditionalExpandToLevel(item, autoExpandOnSingleChildLevels,
-					w -> Boolean.valueOf(doesWidgetHaveExactlyOneChild(w)));
+			internalCustomizedExpandToLevel(item, autoExpandOnSingleChildLevels, singleChildExpansionFunction);
 		}
 	}
 
@@ -3535,8 +3546,4 @@ ISelection getUpdatedSelection(ISelection selection) {
 		return selection;
 	}
 
-	private boolean doesWidgetHaveExactlyOneChild(Widget w) {
-		return getChildren(w).length == 1;
-	}
-
 }

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

@@ -16,13 +16,21 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
+
+import org.eclipse.jface.viewers.AbstractTreeViewer;
 import org.eclipse.jface.viewers.StructuredViewer;
 import org.eclipse.jface.viewers.TreeViewer;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Event;
 import org.eclipse.swt.widgets.Tree;
 import org.eclipse.swt.widgets.TreeItem;
+import org.eclipse.swt.widgets.Widget;
 import org.junit.Test;
 
 public class TreeViewerTest extends AbstractTreeViewerTest {
@@ -77,4 +85,33 @@ public void testAutoExpandOnSingleChildThroughEvent() {
 				fTreeViewer.getExpandedState(trivialPathRoot.getFirstChild().getFirstChild()));
 	}
 
+	@Test
+	public void testInternalExpandToLevelRecursive() {
+		// internalExpandToLevel is recursive by contract, so we track all processed
+		// elements in a subtype to validate contractual recursive execution
+		List<Object> recursiveExpandedElements = new ArrayList<>();
+		TreeViewer viewer = new TreeViewer(fShell) {
+			@Override
+			protected void internalExpandToLevel(Widget widget, int level) {
+				if (widget != this.getTree()) {
+					recursiveExpandedElements.add(widget.getData());
+				}
+				super.internalExpandToLevel(widget, level);
+			}
+		};
+		TestElement rootElement = TestElement.createModel(2, 5);
+		viewer.setContentProvider(new TestModelContentProvider());
+		viewer.setInput(rootElement);
+
+		viewer.expandToLevel(AbstractTreeViewer.ALL_LEVELS);
+
+		Queue<TestElement> elements = new ConcurrentLinkedQueue<>(Arrays.asList(rootElement.getChildren()));
+		while (!elements.isEmpty()) {
+			TestElement currentElement = elements.poll();
+			assertTrue("expansion for child was not processed: " + currentElement,
+					recursiveExpandedElements.contains(currentElement));
+			elements.addAll(Arrays.asList(currentElement.getChildren()));
+		}
+	}
+
 }