This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-lang.git
commit b6ae058bfabc3144c0a19e566c3765f108f9eed1 Author: Gary Gregory <[email protected]> AuthorDate: Fri Jul 17 07:04:33 2026 -0700 Sort members. --- .../commons/lang3/builder/HashCodeBuilder.java | 72 ++++++++--------- .../org/apache/commons/lang3/ArrayUtilsTest.java | 82 ++++++++++---------- .../commons/lang3/builder/HashCodeBuilderTest.java | 90 +++++++++++----------- .../commons/lang3/reflect/FieldUtilsTest.java | 58 +++++++------- 4 files changed, 151 insertions(+), 151 deletions(-) diff --git a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java index 218fdfb21..2b3bb61e2 100644 --- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java @@ -177,12 +177,12 @@ public Builder setMultiplierOddNumber(final int multiplierOddNumber) { private static final ThreadLocal<Set<IDKey>> APPEND_REGISTRY = ThreadLocal.withInitial(HashSet::new); /** - * Constructs a new Builder. + * Registers the given object in the append registry. * - * @return A new Builder. + * @param value The object to register. */ - public static Builder builder() { - return new Builder(); + private static void appendRegister(final Object value) { + APPEND_REGISTRY.get().add(new IDKey(value)); } /* @@ -202,6 +202,28 @@ public static Builder builder() { * to disambiguate the duplicate ids. */ + /** + * Unregisters the given object from the append registry. + * + * @param value The object to unregister. + */ + private static void appendUnregister(final Object value) { + final Set<IDKey> registry = APPEND_REGISTRY.get(); + registry.remove(new IDKey(value)); + if (registry.isEmpty()) { + APPEND_REGISTRY.remove(); + } + } + + /** + * Constructs a new Builder. + * + * @return A new Builder. + */ + public static Builder builder() { + return new Builder(); + } + /** * Gets the registry of objects being traversed by the reflection methods in the current thread. * @@ -211,6 +233,16 @@ static Set<IDKey> getRegistry() { return REGISTRY.get(); } + /** + * Tests whether the append registry contains the given object. Used by {@link #append(Object)} to break its own re-entrant cycles. + * + * @param value The object to look up in the append registry. + * @return {@code true} if the append registry contains the given object. + */ + private static boolean isAppendRegistered(final Object value) { + return APPEND_REGISTRY.get().contains(new IDKey(value)); + } + /** * Tests whether the registry contains the given object. Used by the reflection methods to avoid * infinite loops. @@ -548,38 +580,6 @@ private static void unregister(final Object value) { } } - /** - * Tests whether the append registry contains the given object. Used by {@link #append(Object)} to break its own re-entrant cycles. - * - * @param value The object to look up in the append registry. - * @return {@code true} if the append registry contains the given object. - */ - private static boolean isAppendRegistered(final Object value) { - return APPEND_REGISTRY.get().contains(new IDKey(value)); - } - - /** - * Registers the given object in the append registry. - * - * @param value The object to register. - */ - private static void appendRegister(final Object value) { - APPEND_REGISTRY.get().add(new IDKey(value)); - } - - /** - * Unregisters the given object from the append registry. - * - * @param value The object to unregister. - */ - private static void appendUnregister(final Object value) { - final Set<IDKey> registry = APPEND_REGISTRY.get(); - registry.remove(new IDKey(value)); - if (registry.isEmpty()) { - APPEND_REGISTRY.remove(); - } - } - /** * Constant to use in building the hashCode. */ diff --git a/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java b/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java index dd5f24c7b..61a4bf874 100644 --- a/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/ArrayUtilsTest.java @@ -2809,6 +2809,47 @@ void testReverseObjectRange() { assertNull(array); } + @Test + void testReverseRangeEndIntMinValue() { + // endIndexExclusive == Integer.MIN_VALUE is an undervalue (< start index), documented as no change. + // The unclamped `Math.min(length, end) - 1` underflowed to Integer.MAX_VALUE and indexed out of bounds. + final boolean[] booleans = {true, false, true}; + ArrayUtils.reverse(booleans, 0, Integer.MIN_VALUE); + assertArrayEquals(new boolean[]{true, false, true}, booleans); + + final byte[] bytes = {1, 2, 3}; + ArrayUtils.reverse(bytes, 0, Integer.MIN_VALUE); + assertArrayEquals(new byte[]{1, 2, 3}, bytes); + + final char[] chars = {'a', 'b', 'c'}; + ArrayUtils.reverse(chars, 0, Integer.MIN_VALUE); + assertArrayEquals(new char[]{'a', 'b', 'c'}, chars); + + final double[] doubles = {1, 2, 3}; + ArrayUtils.reverse(doubles, 0, Integer.MIN_VALUE); + assertArrayEquals(new double[]{1, 2, 3}, doubles); + + final float[] floats = {1, 2, 3}; + ArrayUtils.reverse(floats, 0, Integer.MIN_VALUE); + assertArrayEquals(new float[]{1, 2, 3}, floats); + + final int[] ints = {1, 2, 3}; + ArrayUtils.reverse(ints, 0, Integer.MIN_VALUE); + assertArrayEquals(new int[]{1, 2, 3}, ints); + + final long[] longs = {1, 2, 3}; + ArrayUtils.reverse(longs, 0, Integer.MIN_VALUE); + assertArrayEquals(new long[]{1, 2, 3}, longs); + + final Object[] objects = {"a", "b", "c"}; + ArrayUtils.reverse(objects, 0, Integer.MIN_VALUE); + assertArrayEquals(new Object[]{"a", "b", "c"}, objects); + + final short[] shorts = {1, 2, 3}; + ArrayUtils.reverse(shorts, 0, Integer.MIN_VALUE); + assertArrayEquals(new short[]{1, 2, 3}, shorts); + } + @Test void testReverseShort() { short[] array = {1, 2, 3}; @@ -2854,47 +2895,6 @@ void testReverseShortRange() { assertNull(array); } - @Test - void testReverseRangeEndIntMinValue() { - // endIndexExclusive == Integer.MIN_VALUE is an undervalue (< start index), documented as no change. - // The unclamped `Math.min(length, end) - 1` underflowed to Integer.MAX_VALUE and indexed out of bounds. - final boolean[] booleans = {true, false, true}; - ArrayUtils.reverse(booleans, 0, Integer.MIN_VALUE); - assertArrayEquals(new boolean[]{true, false, true}, booleans); - - final byte[] bytes = {1, 2, 3}; - ArrayUtils.reverse(bytes, 0, Integer.MIN_VALUE); - assertArrayEquals(new byte[]{1, 2, 3}, bytes); - - final char[] chars = {'a', 'b', 'c'}; - ArrayUtils.reverse(chars, 0, Integer.MIN_VALUE); - assertArrayEquals(new char[]{'a', 'b', 'c'}, chars); - - final double[] doubles = {1, 2, 3}; - ArrayUtils.reverse(doubles, 0, Integer.MIN_VALUE); - assertArrayEquals(new double[]{1, 2, 3}, doubles); - - final float[] floats = {1, 2, 3}; - ArrayUtils.reverse(floats, 0, Integer.MIN_VALUE); - assertArrayEquals(new float[]{1, 2, 3}, floats); - - final int[] ints = {1, 2, 3}; - ArrayUtils.reverse(ints, 0, Integer.MIN_VALUE); - assertArrayEquals(new int[]{1, 2, 3}, ints); - - final long[] longs = {1, 2, 3}; - ArrayUtils.reverse(longs, 0, Integer.MIN_VALUE); - assertArrayEquals(new long[]{1, 2, 3}, longs); - - final Object[] objects = {"a", "b", "c"}; - ArrayUtils.reverse(objects, 0, Integer.MIN_VALUE); - assertArrayEquals(new Object[]{"a", "b", "c"}, objects); - - final short[] shorts = {1, 2, 3}; - ArrayUtils.reverse(shorts, 0, Integer.MIN_VALUE); - assertArrayEquals(new short[]{1, 2, 3}, shorts); - } - @Test void testSameLength() { final Object[] nullArray = null; diff --git a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java index ae5038248..212e1a32b 100644 --- a/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/HashCodeBuilderTest.java @@ -32,15 +32,18 @@ class HashCodeBuilderTest extends AbstractLangTest { /** - * A reflection test fixture. + * Holds a {@link ReflectionHashCodeMember} and is itself reflection hashed. */ - static class ReflectionTestCycleA { - ReflectionTestCycleB b; + static final class ReflectionHashCodeHolder { + final ReflectionHashCodeMember member; + + ReflectionHashCodeHolder(final ReflectionHashCodeMember member) { + this.member = member; + } @Override public boolean equals(final Object o) { - // Pairs with hashCode() - return super.equals(o); + return EqualsBuilder.reflectionEquals(this, o); } @Override @@ -49,15 +52,18 @@ public int hashCode() { } } /** - * A reflection test fixture. + * A reflection test fixture whose {@code hashCode()} is reflection based. */ - static class ReflectionTestCycleB { - ReflectionTestCycleA a; + static final class ReflectionHashCodeMember { + final int value; + + ReflectionHashCodeMember(final int value) { + this.value = value; + } @Override public boolean equals(final Object o) { - // Pairs with hashCode() - return super.equals(o); + return EqualsBuilder.reflectionEquals(this, o); } @Override @@ -67,18 +73,15 @@ public int hashCode() { } /** - * A reflection test fixture whose {@code hashCode()} is reflection based. + * A reflection test fixture. */ - static final class ReflectionHashCodeMember { - final int value; - - ReflectionHashCodeMember(final int value) { - this.value = value; - } + static class ReflectionTestCycleA { + ReflectionTestCycleB b; @Override public boolean equals(final Object o) { - return EqualsBuilder.reflectionEquals(this, o); + // Pairs with hashCode() + return super.equals(o); } @Override @@ -88,18 +91,15 @@ public int hashCode() { } /** - * Holds a {@link ReflectionHashCodeMember} and is itself reflection hashed. + * A reflection test fixture. */ - static final class ReflectionHashCodeHolder { - final ReflectionHashCodeMember member; - - ReflectionHashCodeHolder(final ReflectionHashCodeMember member) { - this.member = member; - } + static class ReflectionTestCycleB { + ReflectionTestCycleA a; @Override public boolean equals(final Object o) { - return EqualsBuilder.reflectionEquals(this, o); + // Pairs with hashCode() + return super.equals(o); } @Override @@ -236,6 +236,25 @@ public int hashCode() { private static final int CONSTANT = 37; + /** + * {@code append(Object)} must not collapse an object whose own {@code hashCode()} is reflection based to the bare + * initial value. Regression for the cycle guard leaking into the reflection registry. + */ + @Test + void testAppendObjectReflectionHashCodeNotPoisoned() { + final ReflectionHashCodeMember member = new ReflectionHashCodeMember(42); + assertEquals(INITIAL * CONSTANT + member.hashCode(), + new HashCodeBuilder(INITIAL, CONSTANT).append((Object) member).toHashCode()); + + // The same defect reached through a normal, acyclic graph: a nested object's content must still influence + // the enclosing reflection hash. + final int h1 = new ReflectionHashCodeHolder(new ReflectionHashCodeMember(1)).hashCode(); + final int h2 = new ReflectionHashCodeHolder(new ReflectionHashCodeMember(2)).hashCode(); + assertNotEquals(h1, h2); + + assertTrue(HashCodeBuilder.getRegistry().isEmpty()); + } + @Test void testBoolean() { assertEquals(INITIAL * CONSTANT + 0, new HashCodeBuilder(INITIAL, CONSTANT).append(true).toHashCode()); @@ -678,23 +697,4 @@ void testToHashCodeExclude() { assertEquals(INITIAL, HashCodeBuilder.reflectionHashCode(two)); } - /** - * {@code append(Object)} must not collapse an object whose own {@code hashCode()} is reflection based to the bare - * initial value. Regression for the cycle guard leaking into the reflection registry. - */ - @Test - void testAppendObjectReflectionHashCodeNotPoisoned() { - final ReflectionHashCodeMember member = new ReflectionHashCodeMember(42); - assertEquals(INITIAL * CONSTANT + member.hashCode(), - new HashCodeBuilder(INITIAL, CONSTANT).append((Object) member).toHashCode()); - - // The same defect reached through a normal, acyclic graph: a nested object's content must still influence - // the enclosing reflection hash. - final int h1 = new ReflectionHashCodeHolder(new ReflectionHashCodeMember(1)).hashCode(); - final int h2 = new ReflectionHashCodeHolder(new ReflectionHashCodeMember(2)).hashCode(); - assertNotEquals(h1, h2); - - assertTrue(HashCodeBuilder.getRegistry().isEmpty()); - } - } diff --git a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java index 65b3f6507..7a19ec4d2 100644 --- a/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java +++ b/src/test/java/org/apache/commons/lang3/reflect/FieldUtilsTest.java @@ -61,37 +61,37 @@ */ class FieldUtilsTest extends AbstractLangTest { - private static final String JACOCO_DATA_FIELD_NAME = "$jacocoData"; - static final Integer I0 = Integer.valueOf(0); - static final Integer I1 = Integer.valueOf(1); - static final Double D0 = Double.valueOf(0.0); - static final Double D1 = Double.valueOf(1.0); - @Annotated - private PublicChild publicChild; - private PubliclyShadowedChild publiclyShadowedChild; - @Annotated - private PrivatelyShadowedChild privatelyShadowedChild; - private final Class<? super PublicChild> parentClass = PublicChild.class.getSuperclass(); - interface InterfaceWithConstant { int CONSTANT = 42; } - + static class MultiPathToConstant implements SubInterfaceA, SubInterfaceB { + // CONSTANT is reachable through both parents and through the grandparent interface + } + static class SinglePathToConstant implements SubInterfaceA { + // CONSTANT is reachable through the sub-interface and its super-interface + } interface SubInterfaceA extends InterfaceWithConstant { // inherits CONSTANT } - interface SubInterfaceB extends InterfaceWithConstant { // inherits CONSTANT } + private static final String JACOCO_DATA_FIELD_NAME = "$jacocoData"; + static final Integer I0 = Integer.valueOf(0); + static final Integer I1 = Integer.valueOf(1); + static final Double D0 = Double.valueOf(0.0); - static class MultiPathToConstant implements SubInterfaceA, SubInterfaceB { - // CONSTANT is reachable through both parents and through the grandparent interface - } + static final Double D1 = Double.valueOf(1.0); - static class SinglePathToConstant implements SubInterfaceA { - // CONSTANT is reachable through the sub-interface and its super-interface - } + @Annotated + private PublicChild publicChild; + + private PubliclyShadowedChild publiclyShadowedChild; + + @Annotated + private PrivatelyShadowedChild privatelyShadowedChild; + + private final Class<? super PublicChild> parentClass = PublicChild.class.getSuperclass(); /** * Reads the {@code @deprecated} notice on {@link FieldUtils#removeFinalModifier(Field, boolean)}. @@ -129,15 +129,6 @@ void testAmbig() { assertIllegalArgumentException(() -> FieldUtils.getField(Ambig.class, "VALUE")); } - @Test - void testGetFieldInheritedThroughMultipleInterfacePaths() { - // A field reached through more than one interface path resolves to the same Field, so it is not ambiguous. - assertEquals(InterfaceWithConstant.class, FieldUtils.getField(SinglePathToConstant.class, "CONSTANT").getDeclaringClass()); - assertEquals(InterfaceWithConstant.class, FieldUtils.getField(MultiPathToConstant.class, "CONSTANT").getDeclaringClass()); - // A genuine clash of two different fields on unrelated interfaces is still ambiguous. - assertIllegalArgumentException(() -> FieldUtils.getField(Ambig.class, "VALUE")); - } - @Test void testConstructor() { assertNotNull(new FieldUtils()); @@ -347,6 +338,15 @@ void testGetFieldIllegalArgumentException3() { assertIllegalArgumentException(() -> FieldUtils.getField(PublicChild.class, " ")); } + @Test + void testGetFieldInheritedThroughMultipleInterfacePaths() { + // A field reached through more than one interface path resolves to the same Field, so it is not ambiguous. + assertEquals(InterfaceWithConstant.class, FieldUtils.getField(SinglePathToConstant.class, "CONSTANT").getDeclaringClass()); + assertEquals(InterfaceWithConstant.class, FieldUtils.getField(MultiPathToConstant.class, "CONSTANT").getDeclaringClass()); + // A genuine clash of two different fields on unrelated interfaces is still ambiguous. + assertIllegalArgumentException(() -> FieldUtils.getField(Ambig.class, "VALUE")); + } + @Test void testGetFieldNullPointerException() { assertNullPointerException(() -> FieldUtils.getField(null, "none"));
