From e958d173ac487a6e531479ce6c05d828724d1895 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Tue, 20 Feb 2024 14:08:17 +0100 Subject: [PATCH 1/5] Adds test case for sealed interface recursion --- .../SealedTypesRecursionTest.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java diff --git a/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java b/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java new file mode 100644 index 000000000..45e9974e0 --- /dev/null +++ b/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java @@ -0,0 +1,63 @@ +package nl.jqno.equalsverifier.integration.extended_contract; + +import java.util.Objects; +import nl.jqno.equalsverifier.EqualsVerifier; +import nl.jqno.equalsverifier.Warning; +import org.junit.jupiter.api.Test; + +class SealedTypesRecursionTest { + + @Test + public void testEV() { + EqualsVerifier + .forClass(A.class) + .suppress(Warning.STRICT_INHERITANCE, Warning.NONFINAL_FIELDS) + .verify(); + } + + class A { + + public I sealedClassField; + + @Override + public int hashCode() { + return Objects.hash(sealedClassField); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof A)) { + return false; + } + A other = (A) obj; + return Objects.equals(sealedClassField, other.sealedClassField); + } + } + + public sealed interface I permits E {} + + public final class E implements I { + + public A referenceToA; + + @Override + public int hashCode() { + return Objects.hash(referenceToA); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof E)) { + return false; + } + E other = (E) obj; + return Objects.equals(referenceToA, other.referenceToA); + } + } +} From f5a42d24123142d4a8dc847588674f21abc7444c Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Tue, 20 Feb 2024 14:08:48 +0100 Subject: [PATCH 2/5] Initial (dirty) fix for sealed interface recursion issue --- .../internal/prefabvalues/PrefabValues.java | 12 ++++++++-- .../factories/FallbackFactory.java | 10 +++++--- .../internal/reflection/ClassAccessor.java | 14 ++++++++++- .../internal/reflection/FieldModifier.java | 8 ++++++- .../reflection/InPlaceObjectAccessor.java | 23 ++++++++++++++++--- .../internal/reflection/ObjectAccessor.java | 7 ++++++ .../reflection/RecordObjectAccessor.java | 13 ++++++++++- 7 files changed, 76 insertions(+), 11 deletions(-) diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java index e75cac3ea..fbcc7571b 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java @@ -78,7 +78,11 @@ public T giveRedCopy(TypeTag tag) { * @return A tuple of two different values of the given type. */ public Tuple giveTuple(TypeTag tag) { - realizeCacheFor(tag, emptyStack()); + return giveTuple(tag, new LinkedHashSet<>()); + } + + public Tuple giveTuple(TypeTag tag, LinkedHashSet typeStack) { + realizeCacheFor(tag, typeStack); return cache.getTuple(tag); } @@ -92,6 +96,10 @@ public Tuple giveTuple(TypeTag tag) { * @return A value that is different from {@code value}. */ public T giveOther(TypeTag tag, T value) { + return giveOther(tag, value, new LinkedHashSet<>()); + } + + public T giveOther(TypeTag tag, T value, LinkedHashSet typeStack) { Class type = tag.getType(); if ( value != null && @@ -101,7 +109,7 @@ public T giveOther(TypeTag tag, T value) { throw new ReflectionException("TypeTag does not match value."); } - Tuple tuple = giveTuple(tag); + Tuple tuple = giveTuple(tag, typeStack); if (tuple.getRed() == null) { return null; } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java index a49f3728f..3ee0b1b59 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java @@ -37,7 +37,7 @@ public Tuple createValues( } traverseFields(tag, prefabValues, clone); - return giveInstances(tag, prefabValues); + return giveInstances(tag, prefabValues, clone); } private Tuple giveEnumInstances(TypeTag tag) { @@ -90,9 +90,13 @@ private void traverseFields( } } - private Tuple giveInstances(TypeTag tag, PrefabValues prefabValues) { + private Tuple giveInstances( + TypeTag tag, + PrefabValues prefabValues, + LinkedHashSet typeStack + ) { ClassAccessor accessor = ClassAccessor.of(tag.getType(), prefabValues); - T red = accessor.getRedObject(tag); + T red = accessor.getRedObject(tag, typeStack); T blue = accessor.getBlueObject(tag); T redCopy = accessor.getRedObject(tag); return new Tuple<>(red, blue, redCopy); diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java index d78a11179..d9314477c 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java @@ -4,6 +4,7 @@ import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.util.LinkedHashSet; import java.util.Set; import java.util.function.Predicate; import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; @@ -185,6 +186,10 @@ public T getRedObject(TypeTag enclosingType) { return getRedAccessor(enclosingType).get(); } + public T getRedObject(TypeTag enclosingType, LinkedHashSet typeStack) { + return getRedAccessor(enclosingType, typeStack).get(); + } + /** * Returns an {@link ObjectAccessor} for {@link #getRedObject(TypeTag)}. * @@ -193,7 +198,14 @@ public T getRedObject(TypeTag enclosingType) { * @return An {@link ObjectAccessor} for {@link #getRedObject(TypeTag)}. */ public ObjectAccessor getRedAccessor(TypeTag enclosingType) { - return buildObjectAccessor().scramble(prefabValues, enclosingType); + return getRedAccessor(enclosingType, new LinkedHashSet<>()); + } + + public ObjectAccessor getRedAccessor( + TypeTag enclosingType, + LinkedHashSet typeStack + ) { + return buildObjectAccessor().scramble(prefabValues, enclosingType, typeStack); } /** diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java index ffb20997d..fac40f174 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java @@ -3,6 +3,8 @@ import static nl.jqno.equalsverifier.internal.util.Rethrow.rethrow; import java.lang.reflect.Field; +import java.util.LinkedHashSet; + import nl.jqno.equalsverifier.internal.exceptions.ReflectionException; import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag; @@ -92,9 +94,13 @@ public void copyTo(Object to) { * @throws ReflectionException If the operation fails. */ public void changeField(PrefabValues prefabValues, TypeTag enclosingType) { + changeField(prefabValues, enclosingType, new LinkedHashSet<>()); + } + + public void changeField(PrefabValues prefabValues, TypeTag enclosingType, LinkedHashSet typeStack) { FieldChanger fm = () -> { TypeTag tag = TypeTag.of(field, enclosingType); - Object newValue = prefabValues.giveOther(tag, field.get(object)); + Object newValue = prefabValues.giveOther(tag, field.get(object), typeStack); field.set(object, newValue); }; change(fm, false); diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java index baae1c349..0a0e75cad 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java @@ -1,6 +1,7 @@ package nl.jqno.equalsverifier.internal.reflection; import java.lang.reflect.Field; +import java.util.LinkedHashSet; import java.util.function.Function; import java.util.function.Predicate; import nl.jqno.equalsverifier.internal.exceptions.ModuleException; @@ -50,23 +51,39 @@ private S copyInto(S copy) { /** {@inheritDoc} */ @Override public ObjectAccessor scramble(PrefabValues prefabValues, TypeTag enclosingType) { - return scrambleInternal(prefabValues, enclosingType, FieldIterable::of); + return scramble(prefabValues, enclosingType, new LinkedHashSet<>()); + } + + /** {@inheritDoc} */ + @Override + public ObjectAccessor scramble( + PrefabValues prefabValues, + TypeTag enclosingType, + LinkedHashSet typeStack + ) { + return scrambleInternal(prefabValues, enclosingType, typeStack, FieldIterable::of); } /** {@inheritDoc} */ @Override public ObjectAccessor shallowScramble(PrefabValues prefabValues, TypeTag enclosingType) { - return scrambleInternal(prefabValues, enclosingType, FieldIterable::ofIgnoringSuper); + return scrambleInternal( + prefabValues, + enclosingType, + new LinkedHashSet<>(), + FieldIterable::ofIgnoringSuper + ); } private ObjectAccessor scrambleInternal( PrefabValues prefabValues, TypeTag enclosingType, + LinkedHashSet typeStack, Function, FieldIterable> it ) { for (Field field : it.apply(type())) { try { - fieldModifierFor(field).changeField(prefabValues, enclosingType); + fieldModifierFor(field).changeField(prefabValues, enclosingType, typeStack); } catch (ModuleException e) { handleInaccessibleObjectException(e.getCause(), field); } catch (RuntimeException e) { diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java index 3ba603b0d..d431ddd43 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java @@ -1,6 +1,7 @@ package nl.jqno.equalsverifier.internal.reflection; import java.lang.reflect.Field; +import java.util.LinkedHashSet; import java.util.function.Predicate; import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag; @@ -130,6 +131,12 @@ public T getField(Field field) { */ public abstract ObjectAccessor scramble(PrefabValues prefabValues, TypeTag enclosingType); + public abstract ObjectAccessor scramble( + PrefabValues prefabValues, + TypeTag enclosingType, + LinkedHashSet typeStack + ); + /** * Modifies all fields of the wrapped object that are declared in T, but not those inherited * from superclasses. It may or may not mutate the object of the current ObjectAccessor. Either diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java index 57c1f8bb0..adede8803 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java @@ -4,6 +4,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.util.LinkedHashSet; import java.util.List; import java.util.function.Function; import java.util.function.Predicate; @@ -58,10 +59,20 @@ public T copyIntoAnonymousSubclass() { /** {@inheritDoc} */ @Override public ObjectAccessor scramble(PrefabValues prefabValues, TypeTag enclosingType) { + return scramble(prefabValues, enclosingType, new LinkedHashSet<>()); + } + + /** {@inheritDoc} */ + @Override + public ObjectAccessor scramble( + PrefabValues prefabValues, + TypeTag enclosingType, + LinkedHashSet typeStack + ) { return makeAccessor(f -> { Object value = getField(f); TypeTag tag = TypeTag.of(f, enclosingType); - return prefabValues.giveOther(tag, value); + return prefabValues.giveOther(tag, value, typeStack); }); } From 548fdef7efbcc5a5815695ca27376cc6047aa49b Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Fri, 23 Feb 2024 08:26:35 +0100 Subject: [PATCH 3/5] Cleans up the sealed types recursion fix --- .../RecordObjectAccessorScramblingTest.java | 15 ++---- .../reflection/RecordObjectAccessorTest.java | 4 +- .../SealedTypesRecursionTest.java | 51 ++++++++++++------- .../internal/prefabvalues/PrefabValues.java | 22 ++++++-- .../factories/FallbackFactory.java | 4 +- .../internal/reflection/ClassAccessor.java | 49 +++++++++++++++++- .../internal/reflection/FieldModifier.java | 20 +++++++- .../reflection/InPlaceObjectAccessor.java | 6 --- .../internal/reflection/ObjectAccessor.java | 3 +- .../reflection/RecordObjectAccessor.java | 6 --- .../InPlaceObjectAccessorScramblingTest.java | 14 ++--- 11 files changed, 132 insertions(+), 62 deletions(-) diff --git a/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorScramblingTest.java b/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorScramblingTest.java index 49ff6d2a9..ce1af09c6 100644 --- a/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorScramblingTest.java +++ b/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorScramblingTest.java @@ -1,27 +1,22 @@ package nl.jqno.equalsverifier.internal.reflection; import static nl.jqno.equalsverifier.internal.prefabvalues.factories.Factories.values; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotEquals; -import static org.junit.jupiter.api.Assertions.assertNotSame; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import nl.jqno.equalsverifier.internal.exceptions.EqualsVerifierInternalBugException; -import nl.jqno.equalsverifier.internal.prefabvalues.FactoryCache; -import nl.jqno.equalsverifier.internal.prefabvalues.JavaApiPrefabValues; -import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; -import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag; +import nl.jqno.equalsverifier.internal.prefabvalues.*; import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class RecordObjectAccessorScramblingTest { + private static final LinkedHashSet EMPTY_TYPE_STACK = new LinkedHashSet<>(); private FactoryCache factoryCache; private PrefabValues prefabValues; @@ -137,7 +132,7 @@ private Object fieldValue(ObjectAccessor accessor, String fieldName) } private ObjectAccessor doScramble(Object object) { - return create(object).scramble(prefabValues, TypeTag.NULL); + return create(object).scramble(prefabValues, TypeTag.NULL, EMPTY_TYPE_STACK); } record Point(int x, int y) {} diff --git a/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorTest.java b/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorTest.java index 3e3948b54..beb9eb68e 100644 --- a/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorTest.java +++ b/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessorTest.java @@ -6,6 +6,7 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.util.LinkedHashSet; import java.util.Objects; import nl.jqno.equalsverifier.internal.exceptions.ReflectionException; import nl.jqno.equalsverifier.internal.prefabvalues.JavaApiPrefabValues; @@ -17,6 +18,7 @@ public class RecordObjectAccessorTest { + private static final LinkedHashSet EMPTY_TYPE_STACK = new LinkedHashSet<>(); private Object recordInstance; @BeforeEach @@ -72,7 +74,7 @@ public void fail_whenConstructorThrowsOnSomethingElse() { PrefabValues pv = new PrefabValues(JavaApiPrefabValues.build()); ExpectedException - .when(() -> accessorFor(instance).scramble(pv, TypeTag.NULL)) + .when(() -> accessorFor(instance).scramble(pv, TypeTag.NULL, EMPTY_TYPE_STACK)) .assertThrows(ReflectionException.class) .assertMessageContains("Record:", "failed to run constructor", "prefab values"); } diff --git a/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java b/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java index 45e9974e0..0dcc334a2 100644 --- a/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java +++ b/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java @@ -2,26 +2,37 @@ import java.util.Objects; import nl.jqno.equalsverifier.EqualsVerifier; -import nl.jqno.equalsverifier.Warning; +import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException; import org.junit.jupiter.api.Test; class SealedTypesRecursionTest { @Test public void testEV() { - EqualsVerifier - .forClass(A.class) - .suppress(Warning.STRICT_INHERITANCE, Warning.NONFINAL_FIELDS) - .verify(); + // A container with a field of a sealed interface. + // The sealed interface permits only 1 type, which refers back to the container. + ExpectedException + .when(() -> EqualsVerifier.forClass(SealedContainer.class).verify()) + .assertFailure() + .assertMessageContains( + "Recursive datastructure", + "Add prefab values for one of the following types", + "SealedContainer", + "SealedInterface" + ); } - class A { + static final class SealedContainer { - public I sealedClassField; + public final SealedInterface sealed; + + public SealedContainer(SealedInterface sealed) { + this.sealed = sealed; + } @Override public int hashCode() { - return Objects.hash(sealedClassField); + return Objects.hash(sealed); } @Override @@ -29,23 +40,27 @@ public boolean equals(Object obj) { if (this == obj) { return true; } - if (!(obj instanceof A)) { + if (!(obj instanceof SealedContainer)) { return false; } - A other = (A) obj; - return Objects.equals(sealedClassField, other.sealedClassField); + SealedContainer other = (SealedContainer) obj; + return Objects.equals(sealed, other.sealed); } } - public sealed interface I permits E {} + sealed interface SealedInterface permits OnlyPermittedImplementation {} - public final class E implements I { + static final class OnlyPermittedImplementation implements SealedInterface { - public A referenceToA; + public final SealedContainer container; + + public OnlyPermittedImplementation(SealedContainer container) { + this.container = container; + } @Override public int hashCode() { - return Objects.hash(referenceToA); + return Objects.hash(container); } @Override @@ -53,11 +68,11 @@ public boolean equals(Object obj) { if (this == obj) { return true; } - if (!(obj instanceof E)) { + if (!(obj instanceof OnlyPermittedImplementation)) { return false; } - E other = (E) obj; - return Objects.equals(referenceToA, other.referenceToA); + OnlyPermittedImplementation other = (OnlyPermittedImplementation) obj; + return Objects.equals(container, other.container); } } } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java index fbcc7571b..799e7fd0d 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/PrefabValues.java @@ -81,6 +81,14 @@ public Tuple giveTuple(TypeTag tag) { return giveTuple(tag, new LinkedHashSet<>()); } + /** + * Returns a tuple of two different prefabricated values of the specified type. + * + * @param The returned tuple will have this generic type. + * @param tag A description of the desired type, including generic parameters. + * @param typeStack Keeps track of recursion in the type. + * @return A tuple of two different values of the given type. + */ public Tuple giveTuple(TypeTag tag, LinkedHashSet typeStack) { realizeCacheFor(tag, typeStack); return cache.getTuple(tag); @@ -99,6 +107,16 @@ public T giveOther(TypeTag tag, T value) { return giveOther(tag, value, new LinkedHashSet<>()); } + /** + * Returns a prefabricated value of the specified type, that is different from the specified + * value. + * + * @param The type of the value. + * @param tag A description of the desired type, including generic parameters. + * @param value A value that is different from the value that will be returned. + * @param typeStack Keeps track of recursion in the type. + * @return A value that is different from {@code value}. + */ public T giveOther(TypeTag tag, T value, LinkedHashSet typeStack) { Class type = tag.getType(); if ( @@ -131,10 +149,6 @@ private boolean arraysAreDeeplyEqual(Object x, Object y) { return Arrays.deepEquals(new Object[] { x }, new Object[] { y }); } - private LinkedHashSet emptyStack() { - return new LinkedHashSet<>(); - } - /** * Makes sure that values for the specified type are present in the cache, but doesn't return * them. diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java index 3ee0b1b59..5ee82eed2 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/prefabvalues/factories/FallbackFactory.java @@ -97,8 +97,8 @@ private Tuple giveInstances( ) { ClassAccessor accessor = ClassAccessor.of(tag.getType(), prefabValues); T red = accessor.getRedObject(tag, typeStack); - T blue = accessor.getBlueObject(tag); - T redCopy = accessor.getRedObject(tag); + T blue = accessor.getBlueObject(tag, typeStack); + T redCopy = accessor.getRedObject(tag, typeStack); return new Tuple<>(red, blue, redCopy); } } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java index d9314477c..247645456 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java @@ -186,6 +186,15 @@ public T getRedObject(TypeTag enclosingType) { return getRedAccessor(enclosingType).get(); } + /** + * Returns an instance of T that is not equal to the instance of T returned by {@link + * #getBlueObject(TypeTag)}. + * + * @param enclosingType Describes the type that contains this object as a field, to determine + * any generic parameters it may contain. + * @param typeStack Keeps track of recursion in the type. + * @return An instance of T. + */ public T getRedObject(TypeTag enclosingType, LinkedHashSet typeStack) { return getRedAccessor(enclosingType, typeStack).get(); } @@ -201,6 +210,14 @@ public ObjectAccessor getRedAccessor(TypeTag enclosingType) { return getRedAccessor(enclosingType, new LinkedHashSet<>()); } + /** + * Returns an {@link ObjectAccessor} for {@link #getRedObject(TypeTag)}. + * + * @param enclosingType Describes the type that contains this object as a field, to determine + * any generic parameters it may contain. + * @param typeStack Keeps track of recursion in the type. + * @return An {@link ObjectAccessor} for {@link #getRedObject(TypeTag)}. + */ public ObjectAccessor getRedAccessor( TypeTag enclosingType, LinkedHashSet typeStack @@ -220,6 +237,19 @@ public T getBlueObject(TypeTag enclosingType) { return getBlueAccessor(enclosingType).get(); } + /** + * Returns an instance of T that is not equal to the instance of T returned by {@link + * #getRedObject(TypeTag)}. + * + * @param enclosingType Describes the type that contains this object as a field, to determine + * any generic parameters it may contain. + * @param typeStack Keeps track of recursion in the type. + * @return An instance of T. + */ + public T getBlueObject(TypeTag enclosingType, LinkedHashSet typeStack) { + return getBlueAccessor(enclosingType, typeStack).get(); + } + /** * Returns an {@link ObjectAccessor} for {@link #getBlueObject(TypeTag)}. * @@ -228,9 +258,24 @@ public T getBlueObject(TypeTag enclosingType) { * @return An {@link ObjectAccessor} for {@link #getBlueObject(TypeTag)}. */ public ObjectAccessor getBlueAccessor(TypeTag enclosingType) { + return getBlueAccessor(enclosingType, new LinkedHashSet<>()); + } + + /** + * Returns an {@link ObjectAccessor} for {@link #getBlueObject(TypeTag)}. + * + * @param enclosingType Describes the type that contains this object as a field, to determine + * any generic parameters it may contain. + * @param typeStack Keeps track of recursion in the type. + * @return An {@link ObjectAccessor} for {@link #getBlueObject(TypeTag)}. + */ + public ObjectAccessor getBlueAccessor( + TypeTag enclosingType, + LinkedHashSet typeStack + ) { return buildObjectAccessor() - .scramble(prefabValues, enclosingType) - .scramble(prefabValues, enclosingType); + .scramble(prefabValues, enclosingType, typeStack) + .scramble(prefabValues, enclosingType, typeStack); } /** diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java index fac40f174..51ba705c9 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldModifier.java @@ -4,7 +4,6 @@ import java.lang.reflect.Field; import java.util.LinkedHashSet; - import nl.jqno.equalsverifier.internal.exceptions.ReflectionException; import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag; @@ -97,7 +96,24 @@ public void changeField(PrefabValues prefabValues, TypeTag enclosingType) { changeField(prefabValues, enclosingType, new LinkedHashSet<>()); } - public void changeField(PrefabValues prefabValues, TypeTag enclosingType, LinkedHashSet typeStack) { + /** + * Changes the field's value to something else. The new value will never be null. Other than + * that, the precise value is undefined. + * + *

Ignores static fields and fields that can't be modified reflectively. + * + * @param prefabValues If the field is of a type contained within prefabValues, the new value + * will be taken from it. + * @param enclosingType A tag for the type that contains the field. Needed to determine a + * generic type, if it has one.. + * @param typeStack Keeps track of recursion in the type. + * @throws ReflectionException If the operation fails. + */ + public void changeField( + PrefabValues prefabValues, + TypeTag enclosingType, + LinkedHashSet typeStack + ) { FieldChanger fm = () -> { TypeTag tag = TypeTag.of(field, enclosingType); Object newValue = prefabValues.giveOther(tag, field.get(object), typeStack); diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java index 0a0e75cad..63ce34a7e 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java @@ -48,12 +48,6 @@ private S copyInto(S copy) { return copy; } - /** {@inheritDoc} */ - @Override - public ObjectAccessor scramble(PrefabValues prefabValues, TypeTag enclosingType) { - return scramble(prefabValues, enclosingType, new LinkedHashSet<>()); - } - /** {@inheritDoc} */ @Override public ObjectAccessor scramble( diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java index d431ddd43..e3ec543fa 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ObjectAccessor.java @@ -127,10 +127,9 @@ public T getField(Field field) { * @param prefabValues Prefabricated values to take values from. * @param enclosingType Describes the type that contains this object as a field, to determine * any generic parameters it may contain. + * @param typeStack Keeps track of recursion in the type. * @return An accessor to the scrambled object. */ - public abstract ObjectAccessor scramble(PrefabValues prefabValues, TypeTag enclosingType); - public abstract ObjectAccessor scramble( PrefabValues prefabValues, TypeTag enclosingType, diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java index adede8803..600586fad 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/RecordObjectAccessor.java @@ -56,12 +56,6 @@ public T copyIntoAnonymousSubclass() { ); } - /** {@inheritDoc} */ - @Override - public ObjectAccessor scramble(PrefabValues prefabValues, TypeTag enclosingType) { - return scramble(prefabValues, enclosingType, new LinkedHashSet<>()); - } - /** {@inheritDoc} */ @Override public ObjectAccessor scramble( diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessorScramblingTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessorScramblingTest.java index ebeb78f77..67a3487f4 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessorScramblingTest.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessorScramblingTest.java @@ -1,19 +1,14 @@ package nl.jqno.equalsverifier.internal.reflection; import static nl.jqno.equalsverifier.internal.prefabvalues.factories.Factories.values; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import java.text.AttributedString; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import nl.jqno.equalsverifier.internal.exceptions.ModuleException; -import nl.jqno.equalsverifier.internal.prefabvalues.FactoryCache; -import nl.jqno.equalsverifier.internal.prefabvalues.JavaApiPrefabValues; -import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; -import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag; +import nl.jqno.equalsverifier.internal.prefabvalues.*; import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException; import nl.jqno.equalsverifier.testhelpers.types.Point; import nl.jqno.equalsverifier.testhelpers.types.Point3D; @@ -25,6 +20,7 @@ public class InPlaceObjectAccessorScramblingTest { + private static final LinkedHashSet EMPTY_TYPE_STACK = new LinkedHashSet<>(); private PrefabValues prefabValues; @BeforeEach @@ -165,7 +161,7 @@ private T copy(T object) { } private ObjectAccessor doScramble(Object object) { - return create(object).scramble(prefabValues, TypeTag.NULL); + return create(object).scramble(prefabValues, TypeTag.NULL, EMPTY_TYPE_STACK); } static final class StringContainer { From 20f4a799435a18ad610aea0f113a0676371285aa Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Fri, 23 Feb 2024 08:27:55 +0100 Subject: [PATCH 4/5] Updates CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a4af6a3f..14bb563cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- StackOverflowError when a class has a field of a sealed type whose only permitted subtype has a reference to the original class. ([Issue 920](https://github.com/jqno/equalsverifier/issues/920)) + ## [3.15.6] - 2024-01-09 ### Fixed From f319cacefb515283df15641c46fb25e642fc7e0b Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Fri, 23 Feb 2024 08:47:32 +0100 Subject: [PATCH 5/5] Adds test for record with sealed interface recursion --- .../SealedTypesRecursionTest.java | 48 ++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java b/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java index 0dcc334a2..e1adf31da 100644 --- a/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java +++ b/equalsverifier-17/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/SealedTypesRecursionTest.java @@ -8,7 +8,7 @@ class SealedTypesRecursionTest { @Test - public void testEV() { + public void dontThrowStackOverflowError_whenOnlyPermittedSubclassInSealedInterfaceRefersBackToContainer() { // A container with a field of a sealed interface. // The sealed interface permits only 1 type, which refers back to the container. ExpectedException @@ -22,6 +22,21 @@ public void testEV() { ); } + @Test + public void dontThrowStackOverflowError_whenOnlyPermittedRecordInSealedInterfaceRefersBackToContainer() { + // A container with a field of a sealed interface. + // The sealed interface permits only 1 type, which is a record that refers back to the container. + ExpectedException + .when(() -> EqualsVerifier.forClass(SealedRecordContainer.class).verify()) + .assertFailure() + .assertMessageContains( + "Recursive datastructure", + "Add prefab values for one of the following types", + "SealedRecordContainer", + "SealedRecordInterface" + ); + } + static final class SealedContainer { public final SealedInterface sealed; @@ -75,4 +90,35 @@ public boolean equals(Object obj) { return Objects.equals(container, other.container); } } + + static final class SealedRecordContainer { + + public final SealedRecordInterface sealed; + + public SealedRecordContainer(SealedRecordInterface sealed) { + this.sealed = sealed; + } + + @Override + public int hashCode() { + return Objects.hash(sealed); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof SealedRecordContainer)) { + return false; + } + SealedRecordContainer other = (SealedRecordContainer) obj; + return Objects.equals(sealed, other.sealed); + } + } + + sealed interface SealedRecordInterface permits OnlyPermittedRecordImplementation {} + + static final record OnlyPermittedRecordImplementation(SealedRecordContainer container) + implements SealedRecordInterface {} }