Improves error messages around modules

jqno · Oct 30, 2023 · 6891a25 · 6891a25
1 parent 5099ca9
commit 6891a25
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 6 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
+
+-   Improves error message when packages are not "open" to EqualsVerifier. ([Issue 868](https://github.com/jqno/equalsverifier/issues/868))
+
 ## [3.15.2] - 2023-09-23
 
 ### Fixed

diff --git a/...rifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/ModuleException.java b/...rifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/ModuleException.java
@@ -0,0 +1,10 @@
+package nl.jqno.equalsverifier.internal.exceptions;
+
+/** Signals that a class was inaccessible. */
+@SuppressWarnings("serial")
+public class ModuleException extends MessagingException {
+
+    public ModuleException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}
diff --git a/...-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java b/...-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessor.java
@@ -1,7 +1,9 @@
 package nl.jqno.equalsverifier.internal.reflection;
 
 import java.lang.reflect.Field;
+import java.util.function.Function;
 import java.util.function.Predicate;
+import nl.jqno.equalsverifier.internal.exceptions.ModuleException;
 import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues;
 import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag;
 
@@ -48,21 +50,57 @@ private <S> S copyInto(S copy) {
     /** {@inheritDoc} */
     @Override
     public ObjectAccessor<T> scramble(PrefabValues prefabValues, TypeTag enclosingType) {
-        for (Field field : FieldIterable.of(type())) {
-            fieldModifierFor(field).changeField(prefabValues, enclosingType);
-        }
-        return this;
+        return scrambleInternal(prefabValues, enclosingType, FieldIterable::of);
     }
 
     /** {@inheritDoc} */
     @Override
     public ObjectAccessor<T> shallowScramble(PrefabValues prefabValues, TypeTag enclosingType) {
-        for (Field field : FieldIterable.ofIgnoringSuper(type())) {
-            fieldModifierFor(field).changeField(prefabValues, enclosingType);
+        return scrambleInternal(prefabValues, enclosingType, FieldIterable::ofIgnoringSuper);
+    }
+
+    private ObjectAccessor<T> scrambleInternal(
+        PrefabValues prefabValues,
+        TypeTag enclosingType,
+        Function<Class<?>, FieldIterable> it
+    ) {
+        for (Field field : it.apply(type())) {
+            try {
+                fieldModifierFor(field).changeField(prefabValues, enclosingType);
+            } catch (ModuleException e) {
+                handleInaccessibleObjectException(e.getCause(), field);
+            } catch (RuntimeException e) {
+                // InaccessibleObjectException is not yet available in Java 8
+                if (e.getClass().getName().endsWith("InaccessibleObjectException")) {
+                    handleInaccessibleObjectException(e, field);
+                } else {
+                    throw e;
+                }
+            }
         }
         return this;
     }
 
+    private void handleInaccessibleObjectException(Throwable e, Field field) {
+        if (e.getMessage() != null && e.getMessage().contains(type().getCanonicalName())) {
+            throw new ModuleException(
+                "The class is not accessible via the Java Module system. Consider opening the module that contains it.",
+                e
+            );
+        } else {
+            throw new ModuleException(
+                "Field " +
+                field.getName() +
+                " of type " +
+                field.getType().getName() +
+                " is not accessible via the Java Module System.\nConsider opening the module that contains it, or add prefab values for type " +
+                field.getType().getName() +
+                ".",
+                e
+            );
+        }
+    }
+
     /** {@inheritDoc} */
     @Override
     public ObjectAccessor<T> clear(

diff --git a/...-core/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/ModulesTest.java b/...-core/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/ModulesTest.java
@@ -0,0 +1,54 @@
+package nl.jqno.equalsverifier.integration.extended_contract;
+
+import java.text.AttributedString;
+import java.util.Objects;
+import nl.jqno.equalsverifier.EqualsVerifier;
+import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException;
+import org.junit.jupiter.api.Test;
+
+/*
+ * Let's hope nobody needs prefab values for `java.text.AttributedString`,
+ * because we need a class here from je Java APIs that doesn't already
+ * have prefab values.
+ */
+public class ModulesTest {
+
+    @Test
+    public void giveProperErrorMessage_whenClassUnderTestIsInaccessible() {
+        ExpectedException
+            .when(() -> EqualsVerifier.forClass(AttributedString.class).verify())
+            .assertFailure()
+            .assertMessageContains("The class", "Consider opening");
+    }
+
+    @Test
+    public void giveProperErrorMessage_whenFieldIsInaccessible() {
+        ExpectedException
+            .when(() -> EqualsVerifier.forClass(InaccessibleContainer.class).verify())
+            .assertFailure()
+            .assertMessageContains("Field x", "Consider opening", "add prefab values");
+    }
+
+    static final class InaccessibleContainer {
+
+        private final AttributedString x;
+
+        public InaccessibleContainer(AttributedString x) {
+            this.x = x;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof InaccessibleContainer)) {
+                return false;
+            }
+            InaccessibleContainer other = (InaccessibleContainer) obj;
+            return Objects.equals(x, other.x);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(x);
+        }
+    }
+}
diff --git a/.../java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessorScramblingTest.java b/.../java/nl/jqno/equalsverifier/internal/reflection/InPlaceObjectAccessorScramblingTest.java
@@ -3,12 +3,15 @@
 import static nl.jqno.equalsverifier.internal.prefabvalues.factories.Factories.values;
 import static org.junit.jupiter.api.Assertions.*;
 
+import java.text.AttributedString;
 import java.util.ArrayList;
 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.testhelpers.ExpectedException;
 import nl.jqno.equalsverifier.testhelpers.types.Point;
 import nl.jqno.equalsverifier.testhelpers.types.Point3D;
 import nl.jqno.equalsverifier.testhelpers.types.TypeHelper.StaticFinalContainer;
@@ -125,6 +128,26 @@ public void scrambleNestedGenerics() {
         assertEquals(Point.class, foo.points.ts.get(0).getClass());
     }
 
+    @Test
+    public void scrambleSutInaccessible() {
+        AttributedString as = new AttributedString("x");
+
+        ExpectedException
+            .when(() -> doScramble(as))
+            .assertThrows(ModuleException.class)
+            .assertDescriptionContains("The class", "Consider opening");
+    }
+
+    @Test
+    public void scrambleFieldInaccessible() {
+        InaccessibleContainer ic = new InaccessibleContainer(new AttributedString("x"));
+
+        ExpectedException
+            .when(() -> doScramble(ic))
+            .assertThrows(ModuleException.class)
+            .assertDescriptionContains("Field as", "Consider opening");
+    }
+
     @SuppressWarnings("unchecked")
     private <T> InPlaceObjectAccessor<T> create(T object) {
         return new InPlaceObjectAccessor<T>(object, (Class<T>) object.getClass());
@@ -171,4 +194,14 @@ public GenericContainer(List<T> ts) {
             this.ts = ts;
         }
     }
+
+    @SuppressWarnings("unused")
+    static final class InaccessibleContainer {
+
+        private AttributedString as;
+
+        public InaccessibleContainer(AttributedString as) {
+            this.as = as;
+        }
+    }
 }