Skip to content

Commit

Permalink
Introduce useLegacySemantics flag for field and method searches
Browse files Browse the repository at this point in the history 
See junit-team#3553
  • Loading branch information
@sbrannen
sbrannen committed Feb 28, 2024
1 parent dbb6e45 commit 211a410
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 69 deletions.
70 changes: 54 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 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 @@ -1719,8 +1742,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 @@ -1737,22 +1761,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 @@ -1881,4 +1906,17 @@ private static Throwable getUnderlyingCause(Throwable t) {
return t;
}

private static boolean getLegacySearchSemanticsFlag() {
String value = System.getProperty(USE_LEGACY_SEARCH_SEMANTICS_PROPERTY_NAME);
if (StringUtils.isBlank(value)) {
return false;
}
String trimmedValue = value.trim();
boolean isTrue = "true".equalsIgnoreCase(trimmedValue);
Preconditions.condition(isTrue || "false".equalsIgnoreCase(trimmedValue),
() -> USE_LEGACY_SEARCH_SEMANTICS_PROPERTY_NAME + " property must be 'true' or 'false' (ignoring case): "
+ value);
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 211a410

Please sign in to comment.