From 7d41453ea342209823dfe460aa956cd57065a812 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 19 Sep 2024 08:25:18 +0200 Subject: [PATCH] Refines #withPrefabValuesForField --- .../PrefabValuesForFieldInRecordTest.java | 6 +- .../api/SingleTypeEqualsVerifierApi.java | 2 + .../fieldchecks/ReflexivityFieldCheck.java | 13 +- .../internal/reflection/FieldCache.java | 8 + .../internal/util/Configuration.java | 9 + .../equalsverifier/internal/util/Context.java | 12 + .../internal/util/Validations.java | 8 +- .../WithPrefabValuesForFieldTest.java | 227 ++++++++++++++++++ .../internal/reflection/FieldCacheTest.java | 13 + .../internal/util/ConfigurationHelper.java | 2 + .../testhelpers/types/FinalPoint.java | 8 + 11 files changed, 298 insertions(+), 10 deletions(-) create mode 100644 equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/operational/WithPrefabValuesForFieldTest.java diff --git a/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/PrefabValuesForFieldInRecordTest.java b/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/PrefabValuesForFieldInRecordTest.java index 778b741df..c0f29a826 100644 --- a/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/PrefabValuesForFieldInRecordTest.java +++ b/equalsverifier-16/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/PrefabValuesForFieldInRecordTest.java @@ -55,7 +55,7 @@ public void succeed_whenRecordHasDualPrecondition_givenPrefabValueForBothFields( record SinglePrecondition(int i) { public SinglePrecondition { if (i < 100 || i > 200) { - throw new IllegalArgumentException("i must be between 100 and 200!"); + throw new IllegalArgumentException("i must be between 100 and 200! But was " + i); } } } @@ -63,10 +63,10 @@ record SinglePrecondition(int i) { record DualPrecondition(int x, int y) { public DualPrecondition { if (x < 100 || x > 200) { - throw new IllegalArgumentException("x must be between 100 and 200!"); + throw new IllegalArgumentException("x must be between 100 and 200! But was " + x); } if (y < 500 || y > 600) { - throw new IllegalArgumentException("y must be between 500 and 600!"); + throw new IllegalArgumentException("y must be between 500 and 600! But was " + y); } } } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java index 664713aa3..3999234dd 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java @@ -132,6 +132,7 @@ public SingleTypeEqualsVerifierApi withPrefabValuesForField( ) { Validations.validateFieldNameExists(type, fieldName, actualFields); PrefabValuesApi.addPrefabValuesForField(fieldCache, type, fieldName, red, blue); + withNonnullFields(fieldName); suppress(Warning.ZERO_FIELDS); return this; } @@ -439,6 +440,7 @@ private Configuration buildConfig() { allExcludedFields, allIncludedFields, nonnullFields, + fieldCache.getFieldNames(), cachedHashCodeInitializer, hasRedefinedSuperclass, redefinedSubclass, diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java index 43eb1bac7..19bf4f5c3 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java @@ -7,10 +7,7 @@ import java.util.EnumSet; import java.util.Set; import nl.jqno.equalsverifier.Warning; -import nl.jqno.equalsverifier.internal.reflection.ClassProbe; -import nl.jqno.equalsverifier.internal.reflection.FieldProbe; -import nl.jqno.equalsverifier.internal.reflection.Tuple; -import nl.jqno.equalsverifier.internal.reflection.TypeTag; +import nl.jqno.equalsverifier.internal.reflection.*; import nl.jqno.equalsverifier.internal.reflection.annotations.AnnotationCache; import nl.jqno.equalsverifier.internal.reflection.annotations.SupportedAnnotations; import nl.jqno.equalsverifier.internal.reflection.instantiation.SubjectCreator; @@ -27,6 +24,7 @@ public class ReflexivityFieldCheck implements FieldCheck { private final EnumSet warningsToSuppress; private final Set nonnullFields; private final AnnotationCache annotationCache; + private final FieldCache fieldCache; public ReflexivityFieldCheck(Context context) { this.subjectCreator = context.getSubjectCreator(); @@ -37,6 +35,7 @@ public ReflexivityFieldCheck(Context context) { this.warningsToSuppress = config.getWarningsToSuppress(); this.nonnullFields = config.getNonnullFields(); this.annotationCache = config.getAnnotationCache(); + this.fieldCache = context.getFieldCache(); } @Override @@ -77,8 +76,12 @@ private void checkValueReflexivity(FieldProbe probe) { } Field field = probe.getField(); + String fieldName = field.getName(); TypeTag tag = TypeTag.of(field, typeTag); - Tuple tuple = valueProvider.provide(tag); + Tuple tuple = fieldCache.contains(fieldName) + ? fieldCache.get(fieldName) + : valueProvider.provide(tag); + Object left = subjectCreator.withFieldSetTo(field, tuple.getRed()); Object right = subjectCreator.withFieldSetTo(field, tuple.getRedCopy()); diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldCache.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldCache.java index 57a0355e2..ba59e4c99 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldCache.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/FieldCache.java @@ -2,6 +2,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.Set; import nl.jqno.equalsverifier.internal.reflection.instantiation.SubjectCreator; /** Contains a cache for values connected to specific fields, for {@link SubjectCreator}. */ @@ -51,4 +52,11 @@ public Tuple get(String fieldName) { public boolean contains(String fieldName) { return cache.containsKey(fieldName); } + + /** + * @return The fields preset in the cache. + */ + public Set getFieldNames() { + return cache.keySet(); + } } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java index 6b9be50a5..72c06ef98 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java @@ -16,6 +16,7 @@ public final class Configuration { private final Class type; private final Set nonnullFields; + private final Set prefabbedFields; private final CachedHashCodeInitializer cachedHashCodeInitializer; private final boolean hasRedefinedSuperclass; private final Class redefinedSubclass; @@ -36,6 +37,7 @@ private Configuration( TypeTag typeTag, Set ignoredFields, Set nonnullFields, + Set prefabbedFields, AnnotationCache annotationCache, CachedHashCodeInitializer cachedHashCodeInitializer, boolean hasRedefinedSuperclass, @@ -50,6 +52,7 @@ private Configuration( this.typeTag = typeTag; this.ignoredFields = ignoredFields; this.nonnullFields = nonnullFields; + this.prefabbedFields = prefabbedFields; this.annotationCache = annotationCache; this.cachedHashCodeInitializer = cachedHashCodeInitializer; this.hasRedefinedSuperclass = hasRedefinedSuperclass; @@ -66,6 +69,7 @@ public static Configuration build( Set excludedFields, Set includedFields, Set nonnullFields, + Set prefabbedFields, CachedHashCodeInitializer cachedHashCodeInitializer, boolean hasRedefinedSuperclass, Class redefinedSubclass, @@ -96,6 +100,7 @@ public static Configuration build( typeTag, ignoredFields, nonnullFields, + prefabbedFields, annotationCache, cachedHashCodeInitializer, hasRedefinedSuperclass, @@ -190,6 +195,10 @@ public Set getNonnullFields() { return Collections.unmodifiableSet(nonnullFields); } + public Set getPrefabbedFields() { + return Collections.unmodifiableSet(prefabbedFields); + } + public CachedHashCodeInitializer getCachedHashCodeInitializer() { return cachedHashCodeInitializer; } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Context.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Context.java index d3303c78b..456b45704 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Context.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Context.java @@ -11,9 +11,15 @@ public final class Context { private final Class type; private final Configuration configuration; private final ClassProbe classProbe; + private final FieldCache fieldCache; + private final SubjectCreator subjectCreator; private final ValueProvider valueProvider; + @SuppressFBWarnings( + value = "EI_EXPOSE_REP2", + justification = "FieldCache is inherently mutable" + ) public Context( Configuration configuration, FactoryCache factoryCache, @@ -22,6 +28,7 @@ public Context( this.type = configuration.getType(); this.configuration = configuration; this.classProbe = new ClassProbe<>(configuration.getType()); + this.fieldCache = fieldCache; FactoryCache cache = JavaApiPrefabValues.build().merge(factoryCache); this.valueProvider = new VintageValueProvider(cache); @@ -40,6 +47,11 @@ public ClassProbe getClassProbe() { return classProbe; } + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "A cache is inherently mutable") + public FieldCache getFieldCache() { + return fieldCache; + } + @SuppressFBWarnings( value = "EI_EXPOSE_REP", justification = "VintageValueProvider can use a mutable cache." diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java index cdcb3ec8c..0adb4f04d 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java @@ -96,11 +96,15 @@ public static void validateFieldTypeMatches( ) { try { Field f = container.getDeclaredField(fieldName); + boolean sameFields = f.getType().equals(fieldType); + boolean compatibleFields = fieldType.equals( + PrimitiveMappers.PRIMITIVE_OBJECT_MAPPER.get(f.getType()) + ); validate( - f.getType().equals(fieldType), + !sameFields && !compatibleFields, "Prefab values for field " + fieldName + - " should be of type " + + " should be of type " + f.getType().getSimpleName() + " but are " + fieldType.getSimpleName() + diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/operational/WithPrefabValuesForFieldTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/operational/WithPrefabValuesForFieldTest.java new file mode 100644 index 000000000..2ade8be58 --- /dev/null +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/operational/WithPrefabValuesForFieldTest.java @@ -0,0 +1,227 @@ +package nl.jqno.equalsverifier.integration.operational; + +import java.time.LocalDate; +import java.time.Month; +import java.util.Objects; +import nl.jqno.equalsverifier.EqualsVerifier; +import nl.jqno.equalsverifier.Warning; +import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException; +import nl.jqno.equalsverifier.testhelpers.types.FinalPoint; +import org.junit.jupiter.api.Test; + +public class WithPrefabValuesForFieldTest { + + private final FinalPoint pRed = new FinalPoint(3, 42); + private final FinalPoint pBlue = new FinalPoint(3, 1337); + private final int iRed = 111; + private final int iBlue = 142; + + @Test + public void fail_whenRecordHasSinglePrecondition() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(SinglePrecondition.class) + .suppress(Warning.NULL_FIELDS) + .verify() + ) + .assertFailure() + .assertMessageContains("x coordinate must be"); + } + + @Test + public void succeed_whenRecordHasSinglePrecondition_givenPrefabValuesForField() { + EqualsVerifier + .forClass(SinglePrecondition.class) + .withPrefabValuesForField("point", pRed, pBlue) + .verify(); + } + + @Test + public void fail_whenRecordHasDualPrecondition() { + ExpectedException + .when(() -> EqualsVerifier.forClass(DualPrecondition.class).verify()) + .assertFailure() + .assertMessageContains("x must be between"); + } + + @Test + public void fail_whenRecordHasDualPrecondition_givenPrefabValuesForOnlyOneField() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(DualPrecondition.class) + .withPrefabValuesForField("x", iRed, iBlue) + .verify() + ) + .assertFailure() + .assertMessageContains("y must be between"); + } + + @Test + public void succeed_whenRecordHasDualPrecondition_givenPrefabValueForBothFields() { + EqualsVerifier + .forClass(DualPrecondition.class) + .withPrefabValuesForField("x", iRed, iBlue) + .withPrefabValuesForField("y", 505, 555) + .verify(); + } + + @Test + public void throw_whenFieldDoesNotExistInClass() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(SinglePrecondition.class) + .withPrefabValuesForField("doesnt_exist", 1, 2) + ) + .assertThrows(IllegalStateException.class) + .assertMessageContains("Precondition:", "does not contain field doesnt_exist"); + } + + @Test + public void throw_whenFirstPrefabValueIsNull() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(SinglePrecondition.class) + .withPrefabValuesForField("point", null, pBlue) + ) + .assertThrows(NullPointerException.class); + } + + @Test + public void throw_whenSecondPrefabValueIsNull() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(SinglePrecondition.class) + .withPrefabValuesForField("point", pRed, null) + ) + .assertThrows(NullPointerException.class); + } + + @Test + public void throw_whenThePrefabValuesAreTheSame() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(SinglePrecondition.class) + .withPrefabValuesForField("point", pRed, pRed) + ) + .assertThrows(IllegalStateException.class) + .assertMessageContains( + "Precondition", + "both prefab values of type FinalPoint are equal" + ); + } + + @Test + public void throw_whenThePrefabValuesAreEqual() { + FinalPoint red1 = new FinalPoint(3, 4); + FinalPoint red2 = new FinalPoint(3, 4); + + ExpectedException + .when(() -> + EqualsVerifier + .forClass(SinglePrecondition.class) + .withPrefabValuesForField("point", red1, red2) + ) + .assertThrows(IllegalStateException.class) + .assertMessageContains( + "Precondition", + "both prefab values of type FinalPoint are equal" + ); + } + + @Test + public void dontThrow_whenAddingPrefabValuesFromAnotherModuleAndThereforeARedCopyCantBeMade() { + EqualsVerifier + .forClass(OtherModuleContainer.class) + .withPrefabValuesForField("date", LocalDate.of(2024, 9, 18), LocalDate.of(2024, 9, 19)) + .verify(); + } + + static final class SinglePrecondition { + + private final FinalPoint point; + + public SinglePrecondition(FinalPoint point) { + this.point = point; + } + + @Override + public boolean equals(Object obj) { + if (point == null || point.getX() != 3) { + throw new IllegalArgumentException("x coordinate must be 3! But was " + point); + } + if (!(obj instanceof SinglePrecondition)) { + return false; + } + SinglePrecondition other = (SinglePrecondition) obj; + return Objects.equals(point, other.point); + } + + @Override + public int hashCode() { + return Objects.hash(point); + } + } + + static final class DualPrecondition { + + private final int x; + private final int y; + + public DualPrecondition(int x, int y) { + this.x = x; + this.y = y; + } + + @Override + public boolean equals(Object obj) { + if (x < 100 || x > 200) { + throw new IllegalArgumentException("x must be between 100 and 200! But was " + x); + } + if (y < 500 || y > 600) { + throw new IllegalArgumentException("y must be between 500 and 600! But was " + y); + } + if (!(obj instanceof DualPrecondition)) { + return false; + } + DualPrecondition other = (DualPrecondition) obj; + return Objects.equals(x, other.x) && Objects.equals(y, other.y); + } + + @Override + public int hashCode() { + return Objects.hash(x, y); + } + } + + static final class OtherModuleContainer { + + private final LocalDate date; + + public OtherModuleContainer(LocalDate date) { + this.date = date; + } + + @Override + public boolean equals(Object obj) { + if (date == null || date.getMonth() != Month.SEPTEMBER) { + throw new IllegalArgumentException("date must be in September! But was " + date); + } + if (!(obj instanceof OtherModuleContainer)) { + return false; + } + OtherModuleContainer other = (OtherModuleContainer) obj; + return Objects.equals(date, other.date); + } + + @Override + public int hashCode() { + return Objects.hash(date); + } + } +} diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldCacheTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldCacheTest.java index 19b7bd8fb..5d09df19a 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldCacheTest.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldCacheTest.java @@ -2,6 +2,9 @@ import static org.junit.jupiter.api.Assertions.*; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; import org.junit.jupiter.api.Test; public class FieldCacheTest { @@ -45,4 +48,14 @@ public void contains() { public void doesntContain() { assertFalse(cache.contains(stringField)); } + + @Test + public void getFieldNames() { + assertEquals(Collections.emptySet(), cache.getFieldNames()); + + cache.put(stringField, stringValues); + Set expected = new HashSet<>(); + expected.add(stringField); + assertEquals(expected, cache.getFieldNames()); + } } diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/util/ConfigurationHelper.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/util/ConfigurationHelper.java index d23c139c2..d3e92c825 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/util/ConfigurationHelper.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/util/ConfigurationHelper.java @@ -16,6 +16,7 @@ public static final Configuration emptyConfiguration( Collections.emptySet(), Collections.emptySet(), Collections.emptySet(), + Collections.emptySet(), null, false, null, @@ -40,6 +41,7 @@ public static final Configuration emptyConfigurationWithNonnullFields( Collections.emptySet(), Collections.emptySet(), new HashSet<>(Arrays.asList(fieldNames)), + Collections.emptySet(), null, false, null, diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/types/FinalPoint.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/types/FinalPoint.java index ac3cb73de..41e081c18 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/types/FinalPoint.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/types/FinalPoint.java @@ -10,6 +10,14 @@ public FinalPoint(int x, int y) { this.y = y; } + public int getX() { + return x; + } + + public int getY() { + return y; + } + @Override public boolean equals(Object obj) { if (!(obj instanceof FinalPoint)) {