From f89a5bc51b500ba433dd7906c12ca7a56f8303a8 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 26 May 2025 09:42:38 +0200 Subject: [PATCH 1/2] Remove support for legacy semantics for field/method search --- .../release-notes/release-notes-6.0.0-M1.adoc | 4 + .../docs/asciidoc/user-guide/extensions.adoc | 27 +---- .../commons/util/ReflectionUtils.java | 108 ++++-------------- .../commons/util/AnnotationUtilsTests.java | 17 --- .../commons/util/ReflectionUtilsTests.java | 70 ------------ 5 files changed, 31 insertions(+), 195 deletions(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc index 28e79fd1e8b7..d5add3e1618d 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc @@ -70,6 +70,10 @@ repository on GitHub. - `EngineTestKit.execute(String, EngineDiscoveryRequest)`, `EngineTestKit.execute(TestEngine, EngineDiscoveryRequest)`, and `EngineTestKit.Builder.filters(...)` methods +* Remove support for "legacy semantics" for field and method search that used to be + configurable via the `junit.platform.reflection.search.useLegacySemantics` system + property. JUnit now always adheres to standard Java semantics regarding whether a given + field or method is visible or overridden according to the rules of the Java language. [[release-notes-6.0.0-M1-junit-platform-new-features-and-improvements]] ==== New Features and Improvements diff --git a/documentation/src/docs/asciidoc/user-guide/extensions.adoc b/documentation/src/docs/asciidoc/user-guide/extensions.adoc index e1a3b351aedb..49ec9920176e 100644 --- a/documentation/src/docs/asciidoc/user-guide/extensions.adoc +++ b/documentation/src/docs/asciidoc/user-guide/extensions.adoc @@ -995,30 +995,9 @@ Various methods in `AnnotationSupport` and `ReflectionSupport` use search algori traverse type hierarchies to locate matching fields and methods – for example, `AnnotationSupport.findAnnotatedFields(...)`, `ReflectionSupport.findMethods(...)`, etc. -As of JUnit 5.11 (JUnit Platform 1.11), field and method search algorithms adhere to -standard Java semantics regarding whether a given field or method is visible or overridden -according to the rules of the Java language. - -Prior to JUnit 5.11, the field and method search algorithms applied what we now refer to -as "legacy semantics". Legacy semantics consider fields and methods to be _hidden_, -_shadowed_, or _superseded_ by fields and methods in super types (superclasses or -interfaces) based solely on the field's name or the method's signature, disregarding the -actual Java language semantics for visibility and the rules that determine if one method -overrides another method. - -Although the JUnit team recommends the use of the standard search semantics, developers -may optionally revert to the legacy semantics via the -`junit.platform.reflection.search.useLegacySemantics` JVM system property. - -For example, to enable legacy search semantics for fields and methods, you can start your -JVM with the following system property. - -`-Djunit.platform.reflection.search.useLegacySemantics=true` - -NOTE: Due to the low-level nature of the feature, the -`junit.platform.reflection.search.useLegacySemantics` flag can only be set via a JVM -system property. It cannot be set via a <>. +The field and method search algorithms adhere to standard Java semantics regarding whether +a given field or method is visible or overridden according to the rules of the Java +language. [[extensions-execution-order]] === Relative Execution Order of User Code and Extensions diff --git a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java index 0e90c574e5a7..7ce780e5e921 100644 --- a/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java +++ b/junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java @@ -83,27 +83,6 @@ @API(status = INTERNAL, since = "1.0") public final class ReflectionUtils { - /** - * Property name used to signal that legacy semantics should be used when - * searching for fields and methods within a type hierarchy: {@value}. - * - *

Value must be either {@code true} or {@code false} (ignoring case); - * defaults to {@code false}. - * - *

When set to {@code false} (either explicitly or implicitly), field and - * method searches will adhere to Java semantics regarding whether a given - * field or method is visible or overridden, where the latter only applies - * to methods. When set to {@code true}, the semantics used in versions prior - * to JUnit 5.11 (JUnit Platform 1.11) will be used, which means that fields - * and methods can hide, shadow, or supersede fields and methods in supertypes - * based solely on the field's name or the method's signature, disregarding - * the actual Java language semantics for visibility and whether a method - * overrides another method. - * - * @since 1.11 - */ - private static final String USE_LEGACY_SEARCH_SEMANTICS_PROPERTY_NAME = "junit.platform.reflection.search.useLegacySemantics"; - private static final Logger logger = LoggerFactory.getLogger(ReflectionUtils.class); private ReflectionUtils() { @@ -250,8 +229,6 @@ public enum HierarchyTraversalMode { primitiveToWrapperMap = Collections.unmodifiableMap(primitivesToWrappers); } - static volatile boolean useLegacySearchSemantics = getLegacySearchSemanticsFlag(); - public static boolean isPublic(Class clazz) { Preconditions.notNull(clazz, "Class must not be null"); return Modifier.isPublic(clazz.getModifiers()); @@ -1353,23 +1330,19 @@ private static List findAllFieldsInHierarchy(Class clazz, HierarchyTra Field[] localFields = getDeclaredFields(clazz).stream() .filter(field -> !field.isSynthetic()) .toArray(Field[]::new); - Field[] superclassFields = getSuperclassFields(clazz, traversalMode).stream() - .filter(field -> isNotShadowedByLocalFields(field, localFields)) - .toArray(Field[]::new); - Field[] interfaceFields = getInterfaceFields(clazz, traversalMode).stream() - .filter(field -> isNotShadowedByLocalFields(field, localFields)) - .toArray(Field[]::new); // @formatter:on + List superclassFields = getSuperclassFields(clazz, traversalMode); + List interfaceFields = getInterfaceFields(clazz, traversalMode); - List fields = new ArrayList<>(superclassFields.length + interfaceFields.length + localFields.length); + List fields = new ArrayList<>(superclassFields.size() + interfaceFields.size() + localFields.length); if (traversalMode == TOP_DOWN) { - Collections.addAll(fields, superclassFields); - Collections.addAll(fields, interfaceFields); + fields.addAll(superclassFields); + fields.addAll(interfaceFields); } Collections.addAll(fields, localFields); if (traversalMode == BOTTOM_UP) { - Collections.addAll(fields, interfaceFields); - Collections.addAll(fields, superclassFields); + fields.addAll(interfaceFields); + fields.addAll(superclassFields); } return fields; } @@ -1780,21 +1753,18 @@ private static List getInterfaceMethods(Class clazz, HierarchyTravers private static List getInterfaceFields(Class clazz, HierarchyTraversalMode traversalMode) { List allInterfaceFields = new ArrayList<>(); for (Class ifc : clazz.getInterfaces()) { + Field[] localInterfaceFields = ifc.getFields(); Arrays.sort(localInterfaceFields, ReflectionUtils::defaultFieldSorter); - // @formatter:off - Field[] superinterfaceFields = getInterfaceFields(ifc, traversalMode).stream() - .filter(field -> isNotShadowedByLocalFields(field, localInterfaceFields)) - .toArray(Field[]::new); - // @formatter:on + List superinterfaceFields = getInterfaceFields(ifc, traversalMode); if (traversalMode == TOP_DOWN) { - Collections.addAll(allInterfaceFields, superinterfaceFields); + allInterfaceFields.addAll(superinterfaceFields); } Collections.addAll(allInterfaceFields, localInterfaceFields); if (traversalMode == BOTTOM_UP) { - Collections.addAll(allInterfaceFields, superinterfaceFields); + allInterfaceFields.addAll(superinterfaceFields); } } return allInterfaceFields; @@ -1808,18 +1778,6 @@ private static List getSuperclassFields(Class clazz, HierarchyTraversa return findAllFieldsInHierarchy(superclass, traversalMode); } - private static boolean isNotShadowedByLocalFields(Field field, Field[] localFields) { - if (useLegacySearchSemantics) { - for (Field local : localFields) { - if (local.getName().equals(field.getName())) { - return false; - } - } - return true; - } - return true; - } - private static List getSuperclassMethods(Class clazz, HierarchyTraversalMode traversalMode) { Class superclass = clazz.getSuperclass(); if (!isSearchable(superclass)) { @@ -1838,23 +1796,20 @@ private static boolean isNotOverriddenByLocalMethods(Method method, Method[] loc } private static boolean isMethodOverriddenBy(Method upper, Method lower) { - // If legacy search semantics are enabled, skip to hasCompatibleSignature() check. - if (!useLegacySearchSemantics) { - // A static method cannot override anything. - if (Modifier.isStatic(lower.getModifiers())) { - return false; - } + // 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 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; - } + // Cannot override a package-private method in another package. + if (isPackagePrivate(upper) && !declaredInSamePackage(upper, lower)) { + return false; } return hasCompatibleSignature(upper, lower.getName(), lower.getParameterTypes()); @@ -1897,10 +1852,7 @@ private static boolean hasCompatibleSignature(Method candidate, String methodNam } } // lower is sub-signature of upper: check for generics in upper method - if (isGeneric(candidate)) { - return true; - } - return false; + return isGeneric(candidate); } static boolean isGeneric(Method method) { @@ -1984,18 +1936,6 @@ private static Throwable getUnderlyingCause(Throwable t) { return t; } - private static boolean getLegacySearchSemanticsFlag() { - String rawValue = System.getProperty(USE_LEGACY_SEARCH_SEMANTICS_PROPERTY_NAME); - if (StringUtils.isBlank(rawValue)) { - return false; - } - String value = rawValue.trim().toLowerCase(); - boolean isTrue = "true".equals(value); - Preconditions.condition(isTrue || "false".equals(value), () -> USE_LEGACY_SEARCH_SEMANTICS_PROPERTY_NAME - + " property must be 'true' or 'false' (ignoring case): " + rawValue); - return isTrue; - } - private interface Visitor { /** diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java b/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java index 31c65511af00..724b500f6d05 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java @@ -504,23 +504,6 @@ void findAnnotatedFieldsFindsAllFieldsInTypeHierarchy() { .containsExactly("interface", "interface-shadow"); } - @Test - void findAnnotatedFieldsForShadowedFieldsInLegacyMode() { - try { - ReflectionUtils.useLegacySearchSemantics = true; - - assertThat(findShadowingAnnotatedFields(Annotation1.class))// - .containsExactly("super-shadow", "foo-shadow", "baz-shadow"); - assertThat(findShadowingAnnotatedFields(Annotation2.class))// - .containsExactly("bar-shadow", "baz-shadow"); - assertThat(findShadowingAnnotatedFields(Annotation3.class))// - .containsExactly("interface-shadow"); - } - finally { - ReflectionUtils.useLegacySearchSemantics = false; - } - } - private List findShadowingAnnotatedFields(Class annotationType) { var fields = findAnnotatedFields(ClassWithShadowedAnnotatedFields.class, annotationType, isStringField); var values = ReflectionUtils.readFieldValues(fields, new ClassWithShadowedAnnotatedFields()); diff --git a/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java b/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java index 9738a86ef7a6..709c6fed82aa 100644 --- a/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java +++ b/platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java @@ -1665,41 +1665,6 @@ void findMethodsWithoutStaticHidingUsingHierarchyUpMode() throws Exception { assertThat(methods.subList(7, 9)).containsOnly(ifcMethod1, ifcMethod2); } - /** - * In legacy mode, "static hiding" occurs. - */ - @Test - void findMethodsWithStaticHidingUsingHierarchyUpModeInLegacyMode() throws Exception { - try { - ReflectionUtils.useLegacySearchSemantics = true; - - Class ifc = StaticMethodHidingInterface.class; - Class parent = StaticMethodHidingParent.class; - Class child = StaticMethodHidingChild.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 parentMethod2 = parent.getDeclaredMethod("method2", int.class, int.class, int.class); - var parentMethod5 = parent.getDeclaredMethod("method5", String.class); - - assertThat(findMethods(child, methodContains1, BOTTOM_UP)).containsExactly(childMethod1); - assertThat(findMethods(child, methodContains2, BOTTOM_UP)).containsExactly(parentMethod2, ifcMethod2); - assertThat(findMethods(child, methodContains4, BOTTOM_UP)).containsExactly(childMethod4); - assertThat(findMethods(child, methodContains5, BOTTOM_UP)).containsExactly(childMethod5, parentMethod5); - - var methods = findMethods(child, method -> true, BOTTOM_UP); - assertEquals(6, methods.size()); - assertThat(methods.subList(0, 3)).containsOnly(childMethod1, childMethod4, childMethod5); - assertThat(methods.subList(3, 5)).containsOnly(parentMethod2, parentMethod5); - assertEquals(ifcMethod2, methods.get(5)); - } - finally { - ReflectionUtils.useLegacySearchSemantics = false; - } - } - /** * In non-legacy mode, "static hiding" does not occur. */ @@ -1732,41 +1697,6 @@ void findMethodsWithoutStaticHidingUsingHierarchyDownMode() throws Exception { assertThat(methods.subList(6, 9)).containsOnly(childMethod1, childMethod4, childMethod5); } - /** - * In legacy mode, "static hiding" occurs. - */ - @Test - void findMethodsWithStaticHidingUsingHierarchyDownModeInLegacyMode() throws Exception { - try { - ReflectionUtils.useLegacySearchSemantics = true; - - Class ifc = StaticMethodHidingInterface.class; - Class parent = StaticMethodHidingParent.class; - Class child = StaticMethodHidingChild.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 parentMethod2 = parent.getDeclaredMethod("method2", int.class, int.class, int.class); - var parentMethod5 = parent.getDeclaredMethod("method5", String.class); - - assertThat(findMethods(child, methodContains1, TOP_DOWN)).containsExactly(childMethod1); - assertThat(findMethods(child, methodContains2, TOP_DOWN)).containsExactly(ifcMethod2, parentMethod2); - assertThat(findMethods(child, methodContains4, TOP_DOWN)).containsExactly(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.getFirst()); - assertThat(methods.subList(1, 3)).containsOnly(parentMethod2, parentMethod5); - assertThat(methods.subList(3, 6)).containsOnly(childMethod1, childMethod4, childMethod5); - } - finally { - ReflectionUtils.useLegacySearchSemantics = false; - } - } - @Test void findMethodsDoesNotReturnOverriddenMethods() { Predicate isSpecial = method -> method.isAnnotationPresent(Special.class); From 4f4d02a6fee51e49c4e340873b504db038b68fa1 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Mon, 26 May 2025 10:58:24 +0200 Subject: [PATCH 2/2] Update documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc Co-authored-by: Sam Brannen <104798+sbrannen@users.noreply.github.com> --- .../asciidoc/release-notes/release-notes-6.0.0-M1.adoc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc index d5add3e1618d..29966c74f2c9 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-M1.adoc @@ -70,10 +70,11 @@ repository on GitHub. - `EngineTestKit.execute(String, EngineDiscoveryRequest)`, `EngineTestKit.execute(TestEngine, EngineDiscoveryRequest)`, and `EngineTestKit.Builder.filters(...)` methods -* Remove support for "legacy semantics" for field and method search that used to be +* Support for "legacy semantics" for field and method searches that used to be configurable via the `junit.platform.reflection.search.useLegacySemantics` system - property. JUnit now always adheres to standard Java semantics regarding whether a given - field or method is visible or overridden according to the rules of the Java language. + property has been removed. JUnit now always adheres to standard Java semantics regarding + whether a given field or method is visible or overridden according to the rules of the + Java language. [[release-notes-6.0.0-M1-junit-platform-new-features-and-improvements]] ==== New Features and Improvements