From 6891a25d414a2fd1e34fd3efb62acb297f8d5cf8 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Mon, 30 Oct 2023 19:48:45 +0100 Subject: [PATCH] Improves error messages around modules --- CHANGELOG.md | 4 ++ .../internal/exceptions/ModuleException.java | 10 ++++ .../reflection/InPlaceObjectAccessor.java | 50 ++++++++++++++--- .../extended_contract/ModulesTest.java | 54 +++++++++++++++++++ .../InPlaceObjectAccessorScramblingTest.java | 33 ++++++++++++ 5 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/ModuleException.java create mode 100644 equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/ModulesTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 633331ddd..de7fd6652 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] +### 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/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/ModuleException.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/exceptions/ModuleException.java new file mode 100644 index 000000000..68c6adf3f --- /dev/null +++ b/equalsverifier-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/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 0140b644f..baae1c349 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,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 copyInto(S copy) { /** {@inheritDoc} */ @Override public ObjectAccessor 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 shallowScramble(PrefabValues prefabValues, TypeTag enclosingType) { - for (Field field : FieldIterable.ofIgnoringSuper(type())) { - fieldModifierFor(field).changeField(prefabValues, enclosingType); + return scrambleInternal(prefabValues, enclosingType, FieldIterable::ofIgnoringSuper); + } + + private ObjectAccessor scrambleInternal( + PrefabValues prefabValues, + TypeTag enclosingType, + Function, 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 clear( diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/ModulesTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extended_contract/ModulesTest.java new file mode 100644 index 000000000..491a3e5a5 --- /dev/null +++ b/equalsverifier-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/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 4a61d79f2..e400f289d 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 @@ -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 InPlaceObjectAccessor create(T object) { return new InPlaceObjectAccessor(object, (Class) object.getClass()); @@ -171,4 +194,14 @@ public GenericContainer(List ts) { this.ts = ts; } } + + @SuppressWarnings("unused") + static final class InaccessibleContainer { + + private AttributedString as; + + public InaccessibleContainer(AttributedString as) { + this.as = as; + } + } }