Apply field predicate before searching type hierarchy

documentation/src/docs/asciidoc/release-notes/release-notes-5.10.1.adoc

@@ -15,6 +15,10 @@ JUnit repository on GitHub.
 
 ==== Bug Fixes
 
+* Field predicates are now applied while searching the type hierarchy. This fixes bugs in
+  `findFields(...)` and `streamFields(...)` in `ReflectionSupport` as well as
+  `findAnnotatedFields(...)` and `findAnnotatedFieldValues(...)` in `AnnotationSupport`.
+  - See link:https://github.com/junit-team/junit5/issues/3532[issue 3532] for details.
 * Method predicates are now applied while searching the type hierarchy. This fixes bugs
   in `findMethods(...)` and `streamMethods(...)` in `ReflectionSupport` as well as
   `findAnnotatedMethods(...)` in `AnnotationSupport`.
@@ -26,16 +30,20 @@ JUnit repository on GitHub.
 
 ==== Bug Fixes
 
+* A package-private static field annotated with `@TempDir` is no longer _shadowed_ by a
+  non-static field annotated with `@TempDir` when the non-static field resides in a
+  different package and has the same name as the static field.
+  - See link:https://github.com/junit-team/junit5/issues/3532[issue 3532] for details.
 * A package-private class-level lifecycle method annotated with `@BeforeAll` or
   `@AfterAll` is no longer _shadowed_ by a method-level lifecycle method annotated with
   `@BeforeEach` or `@AfterEach` when the method-level lifecycle method resides in a
   different package and has the same name as the class-level lifecycle method.
   - See link:https://github.com/junit-team/junit5/issues/3498[issue 3498] for details.
+* The `ON_SUCCESS` cleanup mode of `@TempDir` now takes into account failures of test
+  methods and nested tests when it's declared on the class level, e.g. as a static field.
 * The `RandomNumberExtension` example in the
   <<../user-guide/index.adoc#extensions-RandomNumberExtension, User Guide>> has been
   updated to properly support `Integer` types as well as non-static field injection.
-* The `ON_SUCCESS` cleanup mode of `@TempDir` now takes into account failures of test
-  methods and nested tests when it's declared on the class level, e.g. as a static field.
 
 ==== New Features and Improvements
 

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

@@ -1238,6 +1238,7 @@ public static List<Constructor<?>> findConstructors(Class<?> clazz, Predicate<Co
 	 */
 	public static List<Field> findFields(Class<?> clazz, Predicate<Field> predicate,
 			HierarchyTraversalMode traversalMode) {
+
 		return streamFields(clazz, predicate, traversalMode).collect(toUnmodifiableList());
 	}
 
@@ -1252,21 +1253,23 @@ public static Stream<Field> streamFields(Class<?> clazz, Predicate<Field> predic
 		Preconditions.notNull(predicate, "Predicate must not be null");
 		Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null");
 
-		return findAllFieldsInHierarchy(clazz, traversalMode).stream().filter(predicate);
+		return findAllFieldsInHierarchy(clazz, predicate, traversalMode).stream();
 	}
 
-	private static List<Field> findAllFieldsInHierarchy(Class<?> clazz, HierarchyTraversalMode traversalMode) {
+	private static List<Field> findAllFieldsInHierarchy(Class<?> clazz, Predicate<Field> predicate,
+			HierarchyTraversalMode traversalMode) {
+
 		Preconditions.notNull(clazz, "Class must not be null");
 		Preconditions.notNull(traversalMode, "HierarchyTraversalMode must not be null");
 
 		// @formatter:off
-		List<Field> localFields = getDeclaredFields(clazz).stream()
+		List<Field> localFields = getDeclaredFields(clazz, predicate).stream()
 				.filter(field -> !field.isSynthetic())
 				.collect(toList());
-		List<Field> superclassFields = getSuperclassFields(clazz, traversalMode).stream()
+		List<Field> superclassFields = getSuperclassFields(clazz, predicate, traversalMode).stream()
 				.filter(field -> !isFieldShadowedByLocalFields(field, localFields))
 				.collect(toList());
-		List<Field> interfaceFields = getInterfaceFields(clazz, traversalMode).stream()
+		List<Field> interfaceFields = getInterfaceFields(clazz, predicate, traversalMode).stream()
 				.filter(field -> !isFieldShadowedByLocalFields(field, localFields))
 				.collect(toList());
 		// @formatter:on
@@ -1529,18 +1532,20 @@ private static List<Method> findAllMethodsInHierarchy(Class<?> clazz, Predicate<
 
 	/**
 	 * Custom alternative to {@link Class#getFields()} that sorts the fields
-	 * and converts them to a mutable list.
+	 * which match the supplied predicate and converts them to a mutable list.
+	 * @param predicate the field filter; never {@code null}
 	 */
-	private static List<Field> getFields(Class<?> clazz) {
-		return toSortedMutableList(clazz.getFields());
+	private static List<Field> getFields(Class<?> clazz, Predicate<Field> predicate) {
+		return toSortedMutableList(clazz.getFields(), predicate);
 	}
 
 	/**
 	 * Custom alternative to {@link Class#getDeclaredFields()} that sorts the
-	 * fields and converts them to a mutable list.
+	 * fields which match the supplied predicate and converts them to a mutable list.
+	 * @param predicate the field filter; never {@code null}
 	 */
-	private static List<Field> getDeclaredFields(Class<?> clazz) {
-		return toSortedMutableList(clazz.getDeclaredFields());
+	private static List<Field> getDeclaredFields(Class<?> clazz, Predicate<Field> predicate) {
+		return toSortedMutableList(clazz.getDeclaredFields(), predicate);
 	}
 
 	/**
@@ -1602,9 +1607,10 @@ private static List<Method> getDefaultMethods(Class<?> clazz) {
 		// @formatter:on
 	}
 
-	private static List<Field> toSortedMutableList(Field[] fields) {
+	private static List<Field> toSortedMutableList(Field[] fields, Predicate<Field> predicate) {
 		// @formatter:off
 		return Arrays.stream(fields)
+				.filter(predicate)
 				.sorted(ReflectionUtils::defaultFieldSorter)
 				// Use toCollection() instead of toList() to ensure list is mutable.
 				.collect(toCollection(ArrayList::new));
@@ -1672,13 +1678,15 @@ private static List<Method> getInterfaceMethods(Class<?> clazz, Predicate<Method
 		return allInterfaceMethods;
 	}
 
-	private static List<Field> getInterfaceFields(Class<?> clazz, HierarchyTraversalMode traversalMode) {
+	private static List<Field> getInterfaceFields(Class<?> clazz, Predicate<Field> predicate,
+			HierarchyTraversalMode traversalMode) {
+
 		List<Field> allInterfaceFields = new ArrayList<>();
 		for (Class<?> ifc : clazz.getInterfaces()) {
-			List<Field> localInterfaceFields = getFields(ifc);
+			List<Field> localInterfaceFields = getFields(ifc, predicate);
 
 			// @formatter:off
-			List<Field> superinterfaceFields = getInterfaceFields(ifc, traversalMode).stream()
+			List<Field> superinterfaceFields = getInterfaceFields(ifc, predicate, traversalMode).stream()
 					.filter(field -> !isFieldShadowedByLocalFields(field, localInterfaceFields))
 					.collect(toList());
 			// @formatter:on
@@ -1694,12 +1702,14 @@ private static List<Field> getInterfaceFields(Class<?> clazz, HierarchyTraversal
 		return allInterfaceFields;
 	}
 
-	private static List<Field> getSuperclassFields(Class<?> clazz, HierarchyTraversalMode traversalMode) {
+	private static List<Field> getSuperclassFields(Class<?> clazz, Predicate<Field> predicate,
+			HierarchyTraversalMode traversalMode) {
+
 		Class<?> superclass = clazz.getSuperclass();
 		if (!isSearchable(superclass)) {
 			return Collections.emptyList();
 		}
-		return findAllFieldsInHierarchy(superclass, traversalMode);
+		return findAllFieldsInHierarchy(superclass, predicate, traversalMode);
 	}
 
 	private static boolean isFieldShadowedByLocalFields(Field field, List<Field> localFields) {

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

@@ -46,8 +46,12 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.platform.commons.PreconditionViolationException;
+import org.junit.platform.commons.util.pkg1.ClassLevelDir;
+import org.junit.platform.commons.util.pkg1.InstanceLevelDir;
 import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateBeforeMethod;
+import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateTempDirField;
 import org.junit.platform.commons.util.pkg1.subpkg.SubclassWithNonStaticPackagePrivateBeforeMethod;
+import org.junit.platform.commons.util.pkg1.subpkg.SubclassWithNonStaticPackagePrivateTempDirField;
 
 /**
  * Unit tests for {@link AnnotationUtils}.
@@ -504,6 +508,28 @@ private List<Field> findShadowingAnnotatedFields(Class<? extends Annotation> ann
 		return findAnnotatedFields(ClassWithShadowedAnnotatedFields.class, annotationType, isStringField);
 	}
 
+	/**
+	 * @see https://github.com/junit-team/junit5/issues/3532
+	 */
+	@Test
+	void findAnnotatedFieldsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception {
+		final String TEMP_DIR = "tempDir";
+		Class<?> superclass = SuperclassWithStaticPackagePrivateTempDirField.class;
+		Field staticField = superclass.getDeclaredField(TEMP_DIR);
+		Class<?> subclass = SubclassWithNonStaticPackagePrivateTempDirField.class;
+		Field nonStaticField = subclass.getDeclaredField(TEMP_DIR);
+
+		// Prerequisite
+		var fields = findAnnotatedFields(superclass, ClassLevelDir.class, field -> true);
+		assertThat(fields).containsExactly(staticField);
+
+		// Actual use cases for this test
+		fields = findAnnotatedFields(subclass, ClassLevelDir.class, field -> true);
+		assertThat(fields).containsExactly(staticField);
+		fields = findAnnotatedFields(subclass, InstanceLevelDir.class, field -> true);
+		assertThat(fields).containsExactly(nonStaticField);
+	}
+
 	// === findPublicAnnotatedFields() =========================================
 
 	@Test

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

@@ -74,7 +74,9 @@
 import org.junit.platform.commons.util.ReflectionUtilsTests.OuterClassImplementingInterface.InnerClassImplementingInterface;
 import org.junit.platform.commons.util.classes.CustomType;
 import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateBeforeMethod;
+import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateTempDirField;
 import org.junit.platform.commons.util.pkg1.subpkg.SubclassWithNonStaticPackagePrivateBeforeMethod;
+import org.junit.platform.commons.util.pkg1.subpkg.SubclassWithNonStaticPackagePrivateTempDirField;
 
 /**
  * Unit tests for {@link ReflectionUtils}.
@@ -1379,6 +1381,28 @@ void isGeneric() {
 		}
 	}
 
+	/**
+	 * @see https://github.com/junit-team/junit5/issues/3532
+	 */
+	@Test
+	void findFieldsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception {
+		final String TEMP_DIR = "tempDir";
+		Class<?> superclass = SuperclassWithStaticPackagePrivateTempDirField.class;
+		Field staticField = superclass.getDeclaredField(TEMP_DIR);
+		Class<?> subclass = SubclassWithNonStaticPackagePrivateTempDirField.class;
+		Field nonStaticField = subclass.getDeclaredField(TEMP_DIR);
+
+		// Prerequisite
+		var fields = findFields(superclass, ReflectionUtils::isStatic, TOP_DOWN);
+		assertThat(fields).containsExactly(staticField);
+
+		// Actual use cases for this test
+		fields = findFields(subclass, ReflectionUtils::isStatic, TOP_DOWN);
+		assertThat(fields).containsExactly(staticField);
+		fields = findFields(subclass, ReflectionUtils::isNotStatic, TOP_DOWN);
+		assertThat(fields).containsExactly(nonStaticField);
+	}
+
 	@Test
 	void readFieldValuesPreconditions() {
 		assertThrows(PreconditionViolationException.class, () -> ReflectionUtils.readFieldValues(null, new Object()));

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

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2015-2023 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 org.junit.platform.commons.util.pkg1;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Mimics {@code @TempDir}.
+ */
+@Target(ElementType.FIELD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface ClassLevelDir {
+}

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

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2015-2023 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 org.junit.platform.commons.util.pkg1;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Mimics {@code @TempDir}.
+ */
+@Target(ElementType.FIELD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface InstanceLevelDir {
+}

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

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2015-2023 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 org.junit.platform.commons.util.pkg1;
+
+import java.nio.file.Path;
+
+/**
+ * @see https://github.com/junit-team/junit5/issues/3532
+ */
+public class SuperclassWithStaticPackagePrivateTempDirField {
+
+	@ClassLevelDir
+	static Path tempDir;
+
+}

platform-tests/src/test/java/org/junit/platform/commons/util/pkg1/subpkg/SubclassWithNonStaticPackagePrivateTempDirField.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2015-2023 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 org.junit.platform.commons.util.pkg1.subpkg;
+
+import java.nio.file.Path;
+
+import org.junit.platform.commons.util.pkg1.InstanceLevelDir;
+import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateTempDirField;
+
+/**
+ * @see https://github.com/junit-team/junit5/issues/3532
+ */
+public class SubclassWithNonStaticPackagePrivateTempDirField extends SuperclassWithStaticPackagePrivateTempDirField {
+
+	@InstanceLevelDir
+	Path tempDir;
+
+}