Skip to content

Commit

Permalink
Introduce useLegacySemantics flag for field and method searches
Browse files Browse the repository at this point in the history 
This commit introduces support for the following JVM system property
which can be set to "true" to revert to legacy search semantics
for fields and methods.

junit.platform.reflection.search.useLegacySemantics

See #3553
  • Loading branch information
@sbrannen
sbrannen committed Apr 7, 2024
1 parent 65501fc commit 64b697f
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 69 deletions.
69 changes: 53 additions & 16 deletions junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,27 @@
@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 overridable, where the latter only applies
* to methods. When set to {@code true}, the semantics used in JUnit 5 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 @@ -238,6 +259,8 @@ 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 @@ -1718,8 +1741,9 @@ private static List<Field> getSuperclassFields(Class<?> clazz, HierarchyTraversa
}

private static boolean isFieldShadowedByLocalFields(Field field, List<Field> localFields) {
// TODO Enable if legacy field search semantics are enabled.
// return localFields.stream().anyMatch(local -> local.getName().equals(field.getName()));
if (useLegacySearchSemantics) {
return localFields.stream().anyMatch(local -> local.getName().equals(field.getName()));
}
return false;
}

Expand All @@ -1736,22 +1760,23 @@ private static boolean isMethodOverriddenByLocalMethods(Method method, List<Meth
}

private static boolean isMethodOverriddenBy(Method upper, Method lower) {
// TODO Skip to hasCompatibleSignature() if legacy method search semantics are enabled.

// A static method cannot override anything.
if (Modifier.isStatic(lower.getModifiers())) {
return false;
}
// 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;
}

// 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 @@ -1880,4 +1905,16 @@ 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;
}

}
21 changes: 13 additions & 8 deletions platform-tests/src/test/java/org/junit/platform/commons/util/AnnotationUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.platform.commons.PreconditionViolationException;
import org.junit.platform.commons.util.pkg1.ClassLevelDir;
Expand Down Expand Up @@ -501,15 +500,21 @@ void findAnnotatedFieldsFindsAllFieldsInTypeHierarchy() {
.containsExactly("interface", "interface-shadow");
}

@Disabled("Disabled until legacy search mode is supported")
@Test
void findAnnotatedFieldsForShadowedFieldsInLegacyMode() {
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");
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) {
Expand Down
101 changes: 56 additions & 45 deletions platform-tests/src/test/java/org/junit/platform/commons/util/ReflectionUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import java.util.stream.IntStream;
import java.util.stream.Stream;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.fixtures.TrackLogRecords;
Expand Down Expand Up @@ -1491,30 +1490,36 @@ void findMethodsWithoutStaticHidingUsingHierarchyUpMode() throws Exception {
/**
* In legacy mode, "static hiding" occurs.
*/
@Disabled("Disabled until legacy search mode is supported")
@Test
void findMethodsWithStaticHidingUsingHierarchyUpModeInLegacyMode() throws Exception {
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));
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;
}
}

/**
Expand Down Expand Up @@ -1553,30 +1558,36 @@ void findMethodsWithoutStaticHidingUsingHierarchyDownMode() throws Exception {
/**
* In legacy mode, "static hiding" occurs.
*/
@Disabled("Disabled until legacy search mode is supported")
@Test
void findMethodsWithStaticHidingUsingHierarchyDownModeInLegacyMode() throws Exception {
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.get(0));
assertThat(methods.subList(1, 3)).containsOnly(parentMethod2, parentMethod5);
assertThat(methods.subList(3, 6)).containsOnly(childMethod1, childMethod4, childMethod5);
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.get(0));
assertThat(methods.subList(1, 3)).containsOnly(parentMethod2, parentMethod5);
assertThat(methods.subList(3, 6)).containsOnly(childMethod1, childMethod4, childMethod5);
}
finally {
ReflectionUtils.useLegacySearchSemantics = false;
}
}

@Test
Expand Down

0 comments on commit 64b697f

Please sign in to comment.