Align method and field search algorithms with Java semantics

This is a work in progress.

Currently based on main (5.11 M1 snapshots) and currently only applied
to our method search algorithm.

Aside from the renaming of isMethodShadowedByLocalMethods(), this
commit should no longer contain changes to
ReflectionUtils.findAllMethodsInHierarchy() once 64ed21a has been
reverted.

See junit-team#3553

junit-jupiter-engine/src/test/java/demo/AbstractJUnitTest.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2015-2024 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package demo;
+
+import org.junit.jupiter.api.Test;
+
+abstract class AbstractJUnitTest {
+
+	@Test
+	void passingTest() {
+	}
+
+	@Test
+	void someTestMethod() {
+		throw new RuntimeException("Boom!");
+	}
+
+}

junit-jupiter-engine/src/test/java/demo/DisabledReproTests.java

@@ -0,0 +1,22 @@
+/*
+ * Copyright 2015-2024 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * https://www.eclipse.org/legal/epl-v20.html
+ */
+
+package demo;
+
+class DisabledReproTests extends AbstractJUnitTest {
+
+	// @Disabled
+	// @Test
+	@Override
+	void someTestMethod() {
+		super.someTestMethod();
+	}
+
+}

junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/LifecycleMethodOverridingAndSupersedingTests.java

@@ -10,6 +10,7 @@
 
 package org.junit.jupiter.engine;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.fail;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -67,7 +68,7 @@ public void beforeEach() {
 	}
 
 	@Nested
-	@DisplayName("A package-private lifecycle super-method from a different package can be superseded by")
+	@DisplayName("A package-private lifecycle super-method from a different package cannot be superseded by")
 	class PackagePrivateSuperClassInDifferentPackageTests {
 
 		@Nested
@@ -78,6 +79,7 @@ class ProtectedExtendsPackagePrivateLifecycleMethod
 			// @Override
 			@BeforeEach
 			protected void beforeEach() {
+				assertThat(super.beforeEachInvoked).isTrue();
 			}
 
 		}
@@ -90,6 +92,7 @@ class PackagePrivateExtendsPackagePrivateLifecycleMethod
 			// @Override
 			@BeforeEach
 			void beforeEach() {
+				assertThat(super.beforeEachInvoked).isTrue();
 			}
 
 		}
@@ -102,6 +105,7 @@ class PublicExtendsPackagePrivateLifecycleMethod
 			// @Override
 			@BeforeEach
 			public void beforeEach() {
+				assertThat(super.beforeEachInvoked).isTrue();
 			}
 
 		}

junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/BeforeAndAfterAllTests.java

@@ -111,6 +111,8 @@ void beforeAllAndAfterAllCallbacksInSubSubclassWithStaticMethodHiding() {
 			"barBeforeAllCallback",
 				"bazBeforeAllCallback",
 					"quuxBeforeAllCallback",
+						"beforeAllMethod-1",
+						"beforeAllMethod-2",
 						"beforeAllMethod-1-hidden",
 						"beforeAllMethod-2-hidden",
 						"beforeAllMethod-3",
@@ -120,6 +122,8 @@ void beforeAllAndAfterAllCallbacksInSubSubclassWithStaticMethodHiding() {
 						"afterAllMethod-1-hidden",
 						"afterAllMethod-2-hidden",
 						"afterAllMethod-3",
+						"afterAllMethod-2",
+						"afterAllMethod-1",
 					"quuxAfterAllCallback",
 				"bazAfterAllCallback",
 			"barAfterAllCallback",

junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/subpackage/SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase.java

@@ -10,7 +10,7 @@
 
 package org.junit.jupiter.engine.subpackage;
 
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -20,13 +20,16 @@
  */
 public class SuperClassWithPackagePrivateLifecycleMethodInDifferentPackageTestCase {
 
+	protected boolean beforeEachInvoked = false;
+
 	@BeforeEach
 	void beforeEach() {
-		fail();
+		this.beforeEachInvoked = true;
 	}
 
 	@Test
 	void test() {
+		assertThat(this.beforeEachInvoked).isTrue();
 	}
 
 }

junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java

@@ -1455,23 +1455,25 @@ private static List<Method> findAllMethodsInHierarchy(Class<?> clazz, Predicate<
 		Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null");
 
 		// @formatter:off
-		List<Method> localMethods = getDeclaredMethods(clazz, predicate, traversalMode).stream()
+		List<Method> localMethods = getDeclaredMethods(clazz, __ -> true, traversalMode).stream()
 				.filter(method -> !method.isSynthetic())
 				.collect(toList());
 		List<Method> superclassMethods = getSuperclassMethods(clazz, predicate, traversalMode).stream()
-				.filter(method -> !isMethodShadowedByLocalMethods(method, localMethods))
+				.filter(method -> !isMethodOverriddenByLocalMethods(method, localMethods))
 				.collect(toList());
 		List<Method> interfaceMethods = getInterfaceMethods(clazz, predicate, traversalMode).stream()
-				.filter(method -> !isMethodShadowedByLocalMethods(method, localMethods))
+				.filter(method -> !isMethodOverriddenByLocalMethods(method, localMethods))
 				.collect(toList());
 		// @formatter:on
 
+		List<Method> filteredLocalMethods = localMethods.stream().filter(predicate).collect(toList());
+
 		List<Method> methods = new ArrayList<>();
 		if (traversalMode == TOP_DOWN) {
 			methods.addAll(superclassMethods);
 			methods.addAll(interfaceMethods);
 		}
-		methods.addAll(localMethods);
+		methods.addAll(filteredLocalMethods);
 		if (traversalMode == BOTTOM_UP) {
 			methods.addAll(interfaceMethods);
 			methods.addAll(superclassMethods);
@@ -1613,7 +1615,7 @@ private static List<Method> getInterfaceMethods(Class<?> clazz, Predicate<Method
 					.collect(toList());
 
 			List<Method> superinterfaceMethods = getInterfaceMethods(ifc, predicate, traversalMode).stream()
-					.filter(method -> !isMethodShadowedByLocalMethods(method, localInterfaceMethods))
+					.filter(method -> !isMethodOverriddenByLocalMethods(method, localInterfaceMethods))
 					.collect(toList());
 			// @formatter:on
 
@@ -1676,14 +1678,39 @@ private static List<Method> getSuperclassMethods(Class<?> clazz, Predicate<Metho
 		return findAllMethodsInHierarchy(superclass, predicate, traversalMode);
 	}
 
-	private static boolean isMethodShadowedByLocalMethods(Method method, List<Method> localMethods) {
-		return localMethods.stream().anyMatch(local -> isMethodShadowedBy(method, local));
+	private static boolean isMethodOverriddenByLocalMethods(Method method, List<Method> localMethods) {
+		return localMethods.stream().anyMatch(local -> isMethodOverriddenBy(method, local));
 	}
 
-	private static boolean isMethodShadowedBy(Method upper, Method lower) {
+	private static boolean isMethodOverriddenBy(Method upper, Method lower) {
+		// A static method cannot override anything.
+		if (Modifier.isStatic(lower.getModifiers())) {
+			return false;
+		}
+
+		// Cannot override a private, static, or final method.
+		int modifiers = upper.getModifiers();
+		if (Modifier.isPrivate(modifiers) || Modifier.isStatic(modifiers) || Modifier.isFinal(modifiers)) {
+			return false;
+		}
+
+		// Cannot override a package-private method in another package.
+		if (isPackagePrivate(upper) && !declaredInSamePackage(upper, lower)) {
+			return false;
+		}
+
 		return hasCompatibleSignature(upper, lower.getName(), lower.getParameterTypes());
 	}
 
+	private static boolean isPackagePrivate(Member member) {
+		int modifiers = member.getModifiers();
+		return !(Modifier.isPublic(modifiers) || Modifier.isProtected(modifiers) || Modifier.isPrivate(modifiers));
+	}
+
+	private static boolean declaredInSamePackage(Method m1, Method m2) {
+		return m1.getDeclaringClass().getPackage().getName().equals(m2.getDeclaringClass().getPackage().getName());
+	}
+
 	/**
 	 * Determine if the supplied candidate method (typically a method higher in
 	 * the type hierarchy) has a signature that is compatible with a method that

platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java

@@ -1230,23 +1230,27 @@ void findMethodsWithStaticHidingUsingHierarchyUpMode() throws Exception {
 		Class<?> parent = StaticMethodHidingParent.class;
 		Class<?> child = StaticMethodHidingChild.class;
 
+		var ifcMethod1 = ifc.getDeclaredMethod("method1", String.class);
 		var ifcMethod2 = ifc.getDeclaredMethod("method2", int.class, int.class);
 		var childMethod1 = child.getDeclaredMethod("method1", String.class);
 		var childMethod4 = child.getDeclaredMethod("method4", boolean.class);
 		var childMethod5 = child.getDeclaredMethod("method5", Long.class);
+		var parentMethod1 = parent.getDeclaredMethod("method1", String.class);
 		var parentMethod2 = parent.getDeclaredMethod("method2", int.class, int.class, int.class);
+		var parentMethod4 = parent.getDeclaredMethod("method4", boolean.class);
 		var parentMethod5 = parent.getDeclaredMethod("method5", String.class);
 
-		assertThat(findMethods(child, methodContains1, BOTTOM_UP)).containsExactly(childMethod1);
+		assertThat(findMethods(child, methodContains1, BOTTOM_UP)).containsExactly(childMethod1, parentMethod1,
+			ifcMethod1);
 		assertThat(findMethods(child, methodContains2, BOTTOM_UP)).containsExactly(parentMethod2, ifcMethod2);
-		assertThat(findMethods(child, methodContains4, BOTTOM_UP)).containsExactly(childMethod4);
+		assertThat(findMethods(child, methodContains4, BOTTOM_UP)).containsExactly(childMethod4, parentMethod4);
 		assertThat(findMethods(child, methodContains5, BOTTOM_UP)).containsExactly(childMethod5, parentMethod5);
 
 		var methods = findMethods(child, method -> true, BOTTOM_UP);
-		assertEquals(6, methods.size());
+		assertEquals(9, methods.size());
 		assertThat(methods.subList(0, 3)).containsOnly(childMethod1, childMethod4, childMethod5);
-		assertThat(methods.subList(3, 5)).containsOnly(parentMethod2, parentMethod5);
-		assertEquals(ifcMethod2, methods.get(5));
+		assertThat(methods.subList(3, 7)).containsOnly(parentMethod1, parentMethod2, parentMethod4, parentMethod5);
+		assertThat(methods.subList(7, 9)).containsOnly(ifcMethod1, ifcMethod2);
 	}
 
 	@Test
@@ -1255,23 +1259,28 @@ void findMethodsWithStaticHidingUsingHierarchyDownMode() throws Exception {
 		Class<?> parent = StaticMethodHidingParent.class;
 		Class<?> child = StaticMethodHidingChild.class;
 
+		var ifcMethod1 = ifc.getDeclaredMethod("method1", String.class);
 		var ifcMethod2 = ifc.getDeclaredMethod("method2", int.class, int.class);
 		var childMethod1 = child.getDeclaredMethod("method1", String.class);
 		var childMethod4 = child.getDeclaredMethod("method4", boolean.class);
 		var childMethod5 = child.getDeclaredMethod("method5", Long.class);
+		var parentMethod1 = parent.getDeclaredMethod("method1", String.class);
 		var parentMethod2 = parent.getDeclaredMethod("method2", int.class, int.class, int.class);
+		var parentMethod4 = parent.getDeclaredMethod("method4", boolean.class);
 		var parentMethod5 = parent.getDeclaredMethod("method5", String.class);
 
-		assertThat(findMethods(child, methodContains1, TOP_DOWN)).containsExactly(childMethod1);
+		assertThat(findMethods(child, methodContains1, TOP_DOWN)).containsExactly(ifcMethod1, parentMethod1,
+			childMethod1);
 		assertThat(findMethods(child, methodContains2, TOP_DOWN)).containsExactly(ifcMethod2, parentMethod2);
-		assertThat(findMethods(child, methodContains4, TOP_DOWN)).containsExactly(childMethod4);
+		assertThat(findMethods(child, methodContains4, TOP_DOWN)).containsExactly(parentMethod4, childMethod4);
 		assertThat(findMethods(child, methodContains5, TOP_DOWN)).containsExactly(parentMethod5, childMethod5);
 
 		var methods = findMethods(child, method -> true, TOP_DOWN);
-		assertEquals(6, methods.size());
-		assertEquals(ifcMethod2, methods.get(0));
-		assertThat(methods.subList(1, 3)).containsOnly(parentMethod2, parentMethod5);
-		assertThat(methods.subList(3, 6)).containsOnly(childMethod1, childMethod4, childMethod5);
+		assertEquals(9, methods.size());
+		methods.forEach(System.err::println);
+		assertThat(methods.subList(0, 2)).containsOnly(ifcMethod1, ifcMethod2);
+		assertThat(methods.subList(2, 6)).containsOnly(parentMethod1, parentMethod2, parentMethod4, parentMethod5);
+		assertThat(methods.subList(6, 9)).containsOnly(childMethod1, childMethod4, childMethod5);
 	}
 
 	@Test