Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ repository on GitHub.
- `EngineTestKit.execute(String, EngineDiscoveryRequest)`,
`EngineTestKit.execute(TestEngine, EngineDiscoveryRequest)`, and
`EngineTestKit.Builder.filters(...)` methods
* Support for "legacy semantics" for field and method searches that used to be
configurable via the `junit.platform.reflection.search.useLegacySemantics` system
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
Expand Down
27 changes: 3 additions & 24 deletions documentation/src/docs/asciidoc/user-guide/extensions.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <<running-tests-config-params, configuration
parameter>>.
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*
* <p>Value must be either {@code true} or {@code false} (ignoring case);
* defaults to {@code false}.
*
* <p>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() {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -1353,23 +1330,19 @@ private static List<Field> 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<Field> superclassFields = getSuperclassFields(clazz, traversalMode);
List<Field> interfaceFields = getInterfaceFields(clazz, traversalMode);

List<Field> fields = new ArrayList<>(superclassFields.length + interfaceFields.length + localFields.length);
List<Field> 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;
}
Expand Down Expand Up @@ -1780,21 +1753,18 @@ private static List<Method> getInterfaceMethods(Class<?> clazz, HierarchyTravers
private static List<Field> getInterfaceFields(Class<?> clazz, HierarchyTraversalMode traversalMode) {
List<Field> 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<Field> 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;
Expand All @@ -1808,18 +1778,6 @@ private static List<Field> 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<Method> getSuperclassMethods(Class<?> clazz, HierarchyTraversalMode traversalMode) {
Class<?> superclass = clazz.getSuperclass();
if (!isSearchable(superclass)) {
Expand All @@ -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());
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<T> {

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> findShadowingAnnotatedFields(Class<? extends Annotation> annotationType) {
var fields = findAnnotatedFields(ClassWithShadowedAnnotatedFields.class, annotationType, isStringField);
var values = ReflectionUtils.readFieldValues(fields, new ClassWithShadowedAnnotatedFields());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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<Method> isSpecial = method -> method.isAnnotationPresent(Special.class);
Expand Down
Loading