Merge pull request #1008 from jqno/refactor-valueproviders

Refactor ValueProviders for prefab values
jqno · Oct 22, 2024 · 5196b13 · 5196b13
2 parents 6409a0c + e41d7ff
commit 5196b13
Show file tree

Hide file tree

Showing 53 changed files with 1,105 additions and 294 deletions.
diff --git 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]
 
+### Changed
+
+- The internal instantiation logic has been further refactored, to be more robust and extensible for future enhancements.
+
 ## [3.17.1] - 2024-10-02
 
 ### Fixed

diff --git a/...l/jqno/equalsverifier/internal/reflection/vintage/RecordObjectAccessorScramblingTest.java b/...l/jqno/equalsverifier/internal/reflection/vintage/RecordObjectAccessorScramblingTest.java
@@ -6,10 +6,10 @@
 
 import java.lang.reflect.Constructor;
 import java.util.LinkedHashSet;
-import nl.jqno.equalsverifier.internal.reflection.FactoryCache;
 import nl.jqno.equalsverifier.internal.reflection.JavaApiPrefabValues;
 import nl.jqno.equalsverifier.internal.reflection.TypeTag;
 import nl.jqno.equalsverifier.internal.reflection.instantiation.VintageValueProvider;
+import nl.jqno.equalsverifier.internal.testhelpers.EmptyValueProvider;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.objenesis.ObjenesisStd;
@@ -23,7 +23,8 @@ public class RecordObjectAccessorScramblingTest {
     @BeforeEach
     public void setup() throws Exception {
         factoryCache = JavaApiPrefabValues.build();
-        valueProvider = new VintageValueProvider(factoryCache, new ObjenesisStd());
+        valueProvider =
+            new VintageValueProvider(EmptyValueProvider.INSTANCE, factoryCache, new ObjenesisStd());
     }
 
     @Test

diff --git a/...est/java/nl/jqno/equalsverifier/internal/reflection/vintage/RecordObjectAccessorTest.java b/...est/java/nl/jqno/equalsverifier/internal/reflection/vintage/RecordObjectAccessorTest.java
@@ -11,6 +11,7 @@
 import nl.jqno.equalsverifier.internal.reflection.JavaApiPrefabValues;
 import nl.jqno.equalsverifier.internal.reflection.TypeTag;
 import nl.jqno.equalsverifier.internal.reflection.instantiation.VintageValueProvider;
+import nl.jqno.equalsverifier.internal.testhelpers.EmptyValueProvider;
 import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -74,7 +75,11 @@ public void fail_whenConstructorThrowsOnSomethingElse() {
             .of(OtherThrowingConstructorRecord.class, objenesis)
             .instantiate();
 
-        VintageValueProvider vp = new VintageValueProvider(JavaApiPrefabValues.build(), objenesis);
+        VintageValueProvider vp = new VintageValueProvider(
+            EmptyValueProvider.INSTANCE,
+            JavaApiPrefabValues.build(),
+            objenesis
+        );
         ExpectedException
             .when(() -> accessorFor(instance).scramble(vp, TypeTag.NULL, EMPTY_TYPE_STACK))
             .assertThrows(ReflectionException.class)

diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java
@@ -9,8 +9,9 @@
 import nl.jqno.equalsverifier.api.EqualsVerifierApi;
 import nl.jqno.equalsverifier.api.MultipleTypeEqualsVerifierApi;
 import nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi;
-import nl.jqno.equalsverifier.internal.reflection.FactoryCache;
 import nl.jqno.equalsverifier.internal.reflection.PackageScanner;
+import nl.jqno.equalsverifier.internal.reflection.instantiation.GenericPrefabValueProvider.GenericFactories;
+import nl.jqno.equalsverifier.internal.reflection.instantiation.PrefabValueProvider;
 import nl.jqno.equalsverifier.internal.util.ListBuilders;
 import nl.jqno.equalsverifier.internal.util.PrefabValuesApi;
 import nl.jqno.equalsverifier.internal.util.Validations;
@@ -20,25 +21,34 @@
 public final class ConfiguredEqualsVerifier implements EqualsVerifierApi<Void> {
 
     private final EnumSet<Warning> warningsToSuppress;
-    private final FactoryCache factoryCache;
+    private final PrefabValueProvider prefabValueProvider;
+    private final GenericFactories genericFactories;
     private boolean usingGetClass;
     private Function<String, String> fieldnameToGetter;
     private final Objenesis objenesis = new ObjenesisStd();
 
     /** Constructor. */
     public ConfiguredEqualsVerifier() {
-        this(EnumSet.noneOf(Warning.class), new FactoryCache(), false, null);
+        this(
+            EnumSet.noneOf(Warning.class),
+            new PrefabValueProvider(),
+            new GenericFactories(),
+            false,
+            null
+        );
     }
 
     /** Private constructor. For internal use only. */
     private ConfiguredEqualsVerifier(
         EnumSet<Warning> warningsToSuppress,
-        FactoryCache factoryCache,
+        PrefabValueProvider prefabValueProvider,
+        GenericFactories genericFactories,
         boolean usingGetClass,
         Function<String, String> fieldnameToGetter
     ) {
         this.warningsToSuppress = warningsToSuppress;
-        this.factoryCache = factoryCache;
+        this.prefabValueProvider = prefabValueProvider;
+        this.genericFactories = genericFactories;
         this.usingGetClass = usingGetClass;
         this.fieldnameToGetter = fieldnameToGetter;
     }
@@ -51,7 +61,8 @@ private ConfiguredEqualsVerifier(
     public ConfiguredEqualsVerifier copy() {
         return new ConfiguredEqualsVerifier(
             EnumSet.copyOf(warningsToSuppress),
-            new FactoryCache().merge(factoryCache),
+            prefabValueProvider.copy(),
+            genericFactories.copy(),
             usingGetClass,
             fieldnameToGetter
         );
@@ -67,7 +78,7 @@ public ConfiguredEqualsVerifier suppress(Warning... warnings) {
     /** {@inheritDoc} */
     @Override
     public <S> ConfiguredEqualsVerifier withPrefabValues(Class<S> otherType, S red, S blue) {
-        PrefabValuesApi.addPrefabValues(factoryCache, objenesis, otherType, red, blue);
+        PrefabValuesApi.addPrefabValues(prefabValueProvider, objenesis, otherType, red, blue);
         return this;
     }
 
@@ -77,7 +88,7 @@ public <S> ConfiguredEqualsVerifier withGenericPrefabValues(
         Class<S> otherType,
         Func1<?, S> factory
     ) {
-        PrefabValuesApi.addGenericPrefabValues(factoryCache, otherType, factory);
+        PrefabValuesApi.addGenericPrefabValues(genericFactories, otherType, factory);
         return this;
     }
 
@@ -87,7 +98,7 @@ public <S> ConfiguredEqualsVerifier withGenericPrefabValues(
         Class<S> otherType,
         Func2<?, ?, S> factory
     ) {
-        PrefabValuesApi.addGenericPrefabValues(factoryCache, otherType, factory);
+        PrefabValuesApi.addGenericPrefabValues(genericFactories, otherType, factory);
         return this;
     }
 
@@ -127,7 +138,8 @@ public <T> SingleTypeEqualsVerifierApi<T> forClass(Class<T> type) {
         return new SingleTypeEqualsVerifierApi<>(
             type,
             EnumSet.copyOf(warningsToSuppress),
-            factoryCache,
+            prefabValueProvider.copy(),
+            genericFactories.copy(),
             objenesis,
             usingGetClass,
             fieldnameToGetter

diff --git a/...lsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java b/...lsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java
@@ -9,8 +9,8 @@
 import nl.jqno.equalsverifier.Warning;
 import nl.jqno.equalsverifier.internal.checkers.*;
 import nl.jqno.equalsverifier.internal.exceptions.MessagingException;
-import nl.jqno.equalsverifier.internal.reflection.FactoryCache;
-import nl.jqno.equalsverifier.internal.reflection.FieldCache;
+import nl.jqno.equalsverifier.internal.reflection.instantiation.GenericPrefabValueProvider.GenericFactories;
+import nl.jqno.equalsverifier.internal.reflection.instantiation.PrefabValueProvider;
 import nl.jqno.equalsverifier.internal.util.*;
 import nl.jqno.equalsverifier.internal.util.Formatter;
 import org.objenesis.Objenesis;
@@ -30,8 +30,8 @@ public class SingleTypeEqualsVerifierApi<T> implements EqualsVerifierApi<T> {
     private boolean usingGetClass = false;
     private boolean hasRedefinedSuperclass = false;
     private Class<? extends T> redefinedSubclass = null;
-    private FactoryCache factoryCache = new FactoryCache();
-    private FieldCache fieldCache = new FieldCache();
+    private PrefabValueProvider prefabValueProvider = new PrefabValueProvider();
+    private GenericFactories genericFactories = new GenericFactories();
     private CachedHashCodeInitializer<T> cachedHashCodeInitializer =
         CachedHashCodeInitializer.passthrough();
     private Function<String, String> fieldnameToGetter = null;
@@ -69,7 +69,8 @@ public SingleTypeEqualsVerifierApi(Class<T> type, Objenesis objenesis) {
      *
      * @param type The class for which the {@code equals} method should be tested.
      * @param warningsToSuppress A list of warnings to suppress in {@code EqualsVerifier}.
-     * @param factoryCache Factories that can be used to create values.
+     * @param prefabValueProvider ValueProvider that records prefab values.
+     * @param genericFactories ValueProvider that records generic prefab values.
      * @param objenesis To instantiate non-record classes.
      * @param usingGetClass Whether {@code getClass} is used in the implementation of the {@code
      *     equals} method, instead of an {@code instanceof} check.
@@ -78,14 +79,16 @@ public SingleTypeEqualsVerifierApi(Class<T> type, Objenesis objenesis) {
     public SingleTypeEqualsVerifierApi(
         Class<T> type,
         EnumSet<Warning> warningsToSuppress,
-        FactoryCache factoryCache,
+        PrefabValueProvider prefabValueProvider,
+        GenericFactories genericFactories,
         Objenesis objenesis,
         boolean usingGetClass,
         Function<String, String> converter
     ) {
         this(type, objenesis);
         this.warningsToSuppress = EnumSet.copyOf(warningsToSuppress);
-        this.factoryCache = this.factoryCache.merge(factoryCache);
+        this.prefabValueProvider = prefabValueProvider;
+        this.genericFactories = genericFactories;
         this.usingGetClass = usingGetClass;
         this.fieldnameToGetter = converter;
     }
@@ -121,7 +124,7 @@ public SingleTypeEqualsVerifierApi<T> suppress(Warning... warnings) {
     /** {@inheritDoc} */
     @Override
     public <S> SingleTypeEqualsVerifierApi<T> withPrefabValues(Class<S> otherType, S red, S blue) {
-        PrefabValuesApi.addPrefabValues(factoryCache, objenesis, otherType, red, blue);
+        PrefabValuesApi.addPrefabValues(prefabValueProvider, objenesis, otherType, red, blue);
         return this;
     }
 
@@ -143,7 +146,14 @@ public <S> SingleTypeEqualsVerifierApi<T> withPrefabValuesForField(
         S red,
         S blue
     ) {
-        PrefabValuesApi.addPrefabValuesForField(fieldCache, objenesis, type, fieldName, red, blue);
+        PrefabValuesApi.addPrefabValuesForField(
+            prefabValueProvider,
+            objenesis,
+            type,
+            fieldName,
+            red,
+            blue
+        );
         withNonnullFields(fieldName);
         return this;
     }
@@ -154,7 +164,7 @@ public <S> SingleTypeEqualsVerifierApi<T> withGenericPrefabValues(
         Class<S> otherType,
         Func1<?, S> factory
     ) {
-        PrefabValuesApi.addGenericPrefabValues(factoryCache, otherType, factory);
+        PrefabValuesApi.addGenericPrefabValues(genericFactories, otherType, factory);
         return this;
     }
 
@@ -164,7 +174,7 @@ public <S> SingleTypeEqualsVerifierApi<T> withGenericPrefabValues(
         Class<S> otherType,
         Func2<?, ?, S> factory
     ) {
-        PrefabValuesApi.addGenericPrefabValues(factoryCache, otherType, factory);
+        PrefabValuesApi.addGenericPrefabValues(genericFactories, otherType, factory);
         return this;
     }
 
@@ -431,7 +441,12 @@ private void performVerification() {
         Validations.validateClassCanBeVerified(type);
 
         Configuration<T> config = buildConfig();
-        Context<T> context = new Context<>(config, factoryCache, fieldCache, objenesis);
+        Context<T> context = new Context<>(
+            config,
+            prefabValueProvider,
+            genericFactories,
+            objenesis
+        );
         Validations.validateProcessedAnnotations(
             type,
             config.getAnnotationCache(),
@@ -450,7 +465,7 @@ private Configuration<T> buildConfig() {
             allExcludedFields,
             allIncludedFields,
             nonnullFields,
-            fieldCache.getFieldNames(),
+            prefabValueProvider.getFieldNames(),
             cachedHashCodeInitializer,
             hasRedefinedSuperclass,
             redefinedSubclass,

diff --git a/...in/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/JpaLazyGetterFieldCheck.java b/...in/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/JpaLazyGetterFieldCheck.java
@@ -9,11 +9,7 @@
 import java.util.function.Function;
 import nl.jqno.equalsverifier.Warning;
 import nl.jqno.equalsverifier.internal.exceptions.EqualsVerifierInternalBugException;
-import nl.jqno.equalsverifier.internal.reflection.ClassProbe;
-import nl.jqno.equalsverifier.internal.reflection.FieldProbe;
-import nl.jqno.equalsverifier.internal.reflection.Instantiator;
-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;
@@ -67,8 +63,8 @@ public void execute(FieldProbe fieldProbe) {
             classProbe.hasMethod(getterName)
         );
 
-        Class<T> sub = throwingGetterCreator(getterName);
-        Tuple<T> tuple = valueProvider.<T>provide(new TypeTag(sub));
+        TypeTag sub = new TypeTag(throwingGetterCreator(getterName));
+        Tuple<T> tuple = valueProvider.provide(sub);
         T red1 = tuple.getRed();
         T red2 = tuple.getRedCopy();
 

diff --git a/...main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java b/...main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java
@@ -7,6 +7,7 @@
 import java.util.EnumSet;
 import java.util.Set;
 import nl.jqno.equalsverifier.Warning;
+import nl.jqno.equalsverifier.internal.exceptions.NoValueException;
 import nl.jqno.equalsverifier.internal.reflection.*;
 import nl.jqno.equalsverifier.internal.reflection.annotations.AnnotationCache;
 import nl.jqno.equalsverifier.internal.reflection.annotations.SupportedAnnotations;
@@ -25,7 +26,6 @@ public class ReflexivityFieldCheck<T> implements FieldCheck<T> {
     private final Set<String> nonnullFields;
     private final Set<String> prefabbedFields;
     private final AnnotationCache annotationCache;
-    private final FieldCache fieldCache;
 
     public ReflexivityFieldCheck(Context<T> context) {
         this.subjectCreator = context.getSubjectCreator();
@@ -37,7 +37,6 @@ public ReflexivityFieldCheck(Context<T> context) {
         this.nonnullFields = config.getNonnullFields();
         this.prefabbedFields = config.getPrefabbedFields();
         this.annotationCache = config.getAnnotationCache();
-        this.fieldCache = context.getFieldCache();
     }
 
     @Override
@@ -80,9 +79,9 @@ private void checkValueReflexivity(FieldProbe probe) {
         Field field = probe.getField();
         String fieldName = field.getName();
         TypeTag tag = TypeTag.of(field, typeTag);
-        Tuple<?> tuple = prefabbedFields.contains(fieldName)
-            ? fieldCache.get(fieldName)
-            : valueProvider.provide(tag);
+        Tuple<?> tuple = valueProvider
+            .provide(tag, fieldName)
+            .orElseThrow(() -> new NoValueException(tag, fieldName));
 
         Object left = subjectCreator.withFieldSetTo(field, tuple.getRed());
         Object right = subjectCreator.withFieldSetTo(field, tuple.getRedCopy());

diff --git a/.../src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/StringFieldCheck.java b/.../src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/StringFieldCheck.java
@@ -40,7 +40,8 @@ public StringFieldCheck(
     )
     public void execute(FieldProbe fieldProbe) {
         if (String.class.equals(fieldProbe.getType()) && !fieldProbe.isStatic()) {
-            String red = valueProvider.<String>provide(new TypeTag(String.class)).getRed();
+            TypeTag string = new TypeTag(String.class);
+            String red = valueProvider.<String>provide(string).getRed();
 
             final T reference;
             final T copy;

diff --git a/...ifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/NoValueException.java b/...ifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/NoValueException.java
@@ -0,0 +1,31 @@
+package nl.jqno.equalsverifier.internal.exceptions;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import nl.jqno.equalsverifier.internal.reflection.TypeTag;
+
+@SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "EqualsVerifier doesn't serialize.")
+public class NoValueException extends MessagingException {
+
+    private final TypeTag tag;
+    private final String label;
+
+    public NoValueException(TypeTag tag) {
+        this(tag, null);
+    }
+
+    public NoValueException(TypeTag tag, String label) {
+        super();
+        this.tag = tag;
+        this.label = label;
+    }
+
+    @Override
+    public String getDescription() {
+        return (
+            "Could not find a value for " +
+            tag +
+            (label == null ? "" : " and label " + label) +
+            ". Please add prefab values for this type."
+        );
+    }
+}