Skip to content

Commit

Permalink
Revert "Apply field predicate before searching type hierarchy"
Browse files Browse the repository at this point in the history
This commit reverts the functional changes from commit f30a8d5 and
disables the associated tests for the time being.

See #3532
See #3553
Closes #3638
  • Loading branch information
sbrannen committed Jan 15, 2024
1 parent f3d0710 commit 7789d32
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,52 @@

*Date of Release:* ❓

*Scope:* minor bug fixes since 5.10.1.
*Scope:* minor bug fixes and changes since 5.10.1.

For a complete list of all _closed_ issues and pull requests for this release, consult the
link:{junit5-repo}+/milestone/73?closed=1+[5.10.2] milestone page in the
JUnit repository on GitHub.
link:{junit5-repo}+/milestone/73?closed=1+[5.10.2] milestone page in the JUnit repository
on GitHub.


[[release-notes-5.10.2-junit-platform]]
=== JUnit Platform

==== Bug Fixes

* Allow `junit-platform-launcher` to be used as a Java module when
* The `junit-platform-launcher` may now be used as a Java module when
`junit.platform.launcher.interceptors.enabled` is set to `true`.
- See link:https://github.com/junit-team/junit5/issues/3561[issue 3561] for details.
- See issue link:https://github.com/junit-team/junit5/issues/3561[#3561] for details.

==== Deprecations and Breaking Changes

* Field predicates are no longer applied eagerly while searching the type hierarchy. This
reverts changes made in 5.10.1 that affected `findFields(...)` and `streamFields(...)`
in `ReflectionSupport` as well as `findAnnotatedFields(...)` and
`findAnnotatedFieldValues(...)` in `AnnotationSupport`.
- See issues link:https://github.com/junit-team/junit5/issues/3638[#3638] and
link:https://github.com/junit-team/junit5/issues/3553[#3553] for details.


[[release-notes-5.10.2-junit-jupiter]]
=== JUnit Jupiter

==== Bug Fixes

* _none so far_
* ❓

==== Deprecations and Breaking Changes

* A package-private static field annotated with `@TempDir` is once again _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. This reverts changes made
in 5.10.1.
- See issues link:https://github.com/junit-team/junit5/issues/3638[#3638] and
link:https://github.com/junit-team/junit5/issues/3553[#3553] for details.


[[release-notes-5.10.2-junit-vintage]]
=== JUnit Vintage

==== Bug Fixes

* _none so far_
*
Original file line number Diff line number Diff line change
Expand Up @@ -1202,23 +1202,21 @@ 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, predicate, traversalMode).stream();
return findAllFieldsInHierarchy(clazz, traversalMode).stream().filter(predicate);
}

private static List<Field> findAllFieldsInHierarchy(Class<?> clazz, Predicate<Field> predicate,
HierarchyTraversalMode traversalMode) {

private static List<Field> findAllFieldsInHierarchy(Class<?> clazz, 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, predicate).stream()
List<Field> localFields = getDeclaredFields(clazz).stream()
.filter(field -> !field.isSynthetic())
.collect(toList());
List<Field> superclassFields = getSuperclassFields(clazz, predicate, traversalMode).stream()
List<Field> superclassFields = getSuperclassFields(clazz, traversalMode).stream()
.filter(field -> !isFieldShadowedByLocalFields(field, localFields))
.collect(toList());
List<Field> interfaceFields = getInterfaceFields(clazz, predicate, traversalMode).stream()
List<Field> interfaceFields = getInterfaceFields(clazz, traversalMode).stream()
.filter(field -> !isFieldShadowedByLocalFields(field, localFields))
.collect(toList());
// @formatter:on
Expand Down Expand Up @@ -1481,18 +1479,18 @@ private static List<Method> findAllMethodsInHierarchy(Class<?> clazz, Predicate<

/**
* Custom alternative to {@link Class#getFields()} that sorts the fields
* which match the supplied predicate and converts them to a mutable list.
* and converts them to a mutable list.
*/
private static List<Field> getFields(Class<?> clazz, Predicate<Field> predicate) {
return toSortedMutableList(clazz.getFields(), predicate);
private static List<Field> getFields(Class<?> clazz) {
return toSortedMutableList(clazz.getFields());
}

/**
* Custom alternative to {@link Class#getDeclaredFields()} that sorts the
* fields which match the supplied predicate and converts them to a mutable list.
* fields and converts them to a mutable list.
*/
private static List<Field> getDeclaredFields(Class<?> clazz, Predicate<Field> predicate) {
return toSortedMutableList(clazz.getDeclaredFields(), predicate);
private static List<Field> getDeclaredFields(Class<?> clazz) {
return toSortedMutableList(clazz.getDeclaredFields());
}

/**
Expand Down Expand Up @@ -1556,10 +1554,9 @@ private static List<Method> getDefaultMethods(Class<?> clazz, Predicate<Method>
// @formatter:on
}

private static List<Field> toSortedMutableList(Field[] fields, Predicate<Field> predicate) {
private static List<Field> toSortedMutableList(Field[] fields) {
// @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));
Expand Down Expand Up @@ -1628,15 +1625,13 @@ private static List<Method> getInterfaceMethods(Class<?> clazz, Predicate<Method
return allInterfaceMethods;
}

private static List<Field> getInterfaceFields(Class<?> clazz, Predicate<Field> predicate,
HierarchyTraversalMode traversalMode) {

private static List<Field> getInterfaceFields(Class<?> clazz, HierarchyTraversalMode traversalMode) {
List<Field> allInterfaceFields = new ArrayList<>();
for (Class<?> ifc : clazz.getInterfaces()) {
List<Field> localInterfaceFields = getFields(ifc, predicate);
List<Field> localInterfaceFields = getFields(ifc);

// @formatter:off
List<Field> superinterfaceFields = getInterfaceFields(ifc, predicate, traversalMode).stream()
List<Field> superinterfaceFields = getInterfaceFields(ifc, traversalMode).stream()
.filter(field -> !isFieldShadowedByLocalFields(field, localInterfaceFields))
.collect(toList());
// @formatter:on
Expand All @@ -1652,14 +1647,12 @@ private static List<Field> getInterfaceFields(Class<?> clazz, Predicate<Field> p
return allInterfaceFields;
}

private static List<Field> getSuperclassFields(Class<?> clazz, Predicate<Field> predicate,
HierarchyTraversalMode traversalMode) {

private static List<Field> getSuperclassFields(Class<?> clazz, HierarchyTraversalMode traversalMode) {
Class<?> superclass = clazz.getSuperclass();
if (!isSearchable(superclass)) {
return Collections.emptyList();
}
return findAllFieldsInHierarchy(superclass, predicate, traversalMode);
return findAllFieldsInHierarchy(superclass, traversalMode);
}

private static boolean isFieldShadowedByLocalFields(Field field, List<Field> localFields) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@

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 @@ -509,10 +510,11 @@ private List<Field> findShadowingAnnotatedFields(Class<? extends Annotation> ann
}

/**
* @see https://github.com/junit-team/junit5/issues/3532
* @see https://github.com/junit-team/junit5/issues/3553
*/
@Disabled("Until #3553 is resolved")
@Test
void findAnnotatedFieldsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception {
void findAnnotatedFieldsDoesNotAllowInstanceFieldToHideStaticField() throws Exception {
final String TEMP_DIR = "tempDir";
Class<?> superclass = SuperclassWithStaticPackagePrivateTempDirField.class;
Field staticField = superclass.getDeclaredField(TEMP_DIR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
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 @@ -1357,10 +1358,11 @@ void isGeneric() {
}

/**
* @see https://github.com/junit-team/junit5/issues/3532
* @see https://github.com/junit-team/junit5/issues/3553
*/
@Disabled("Until #3553 is resolved")
@Test
void findFieldsAppliesPredicateBeforeSearchingTypeHierarchy() throws Exception {
void findFieldsDoesNotAllowInstanceFieldToHideStaticField() throws Exception {
final String TEMP_DIR = "tempDir";
Class<?> superclass = SuperclassWithStaticPackagePrivateTempDirField.class;
Field staticField = superclass.getDeclaredField(TEMP_DIR);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import java.nio.file.Path;

/**
* @see https://github.com/junit-team/junit5/issues/3532
* @see https://github.com/junit-team/junit5/issues/3553
*/
public class SuperclassWithStaticPackagePrivateTempDirField {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import org.junit.platform.commons.util.pkg1.SuperclassWithStaticPackagePrivateTempDirField;

/**
* @see https://github.com/junit-team/junit5/issues/3532
* @see https://github.com/junit-team/junit5/issues/3553
*/
public class SubclassWithNonStaticPackagePrivateTempDirField extends SuperclassWithStaticPackagePrivateTempDirField {

Expand Down

0 comments on commit 7789d32

Please sign in to comment.