Improves error message and documentation for versioned entities

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - When `Warning.SURROGATE_OR_BUSINESS_KEY` is suppressed, it is now possible to use `#withOnlyTheseFields`, and the fields may include both `@Id` fields and regular fields. ([Issue 934](https://github.com/jqno/equalsverifier/issues/934))
+- Improved the error message for so called 'versioned entities' (where entities with `0` or `null` ids are always unequal), so it's more discoverable what to do in that situation. ([Issue 932](https://github.com/jqno/equalsverifier/issues/932))
 
 ## [3.15.8] - 2024-03-01
 

docs/_manual/10-jpa-entities.md

@@ -39,21 +39,37 @@ public boolean equals(Object obj) {
     }
     Foo other = (Foo)obj;
     if (id == 0L && other.id == 0L) {
-        return super.equals(obj);
+        return false;
     }
     return id == other.id;
 }
 {% endhighlight %}
 
 You might see an error message such as this one:
 
-    Reflexivity: object does not equal an identical copy of itself:
+    Reflexivity: entity does not equal an identical copy of itself:
       Foo@123456
-    If this is intentional, consider suppressing Warning.IDENTICAL_COPY
+    If this is intentional, consider suppressing Warning.IDENTICAL_COPYFOR_VERSIONED_ENTITY
+
+In that case, you can call `suppress(Warning.SURROGATE_KEY, Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY)`: `SURROGATE_KEY` because the identity is based on the entity's keys, and `IDENTICAL_COPY_FOR_VERSIONED_ENTITY` to allow the (small) breach in reflexivity.
 
-In that case, you can call `suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY)`.
+Similarly, if the entity has a business key, like so:
+
+{% highlight java %}
+@Override
+public boolean equals(Object obj) {
+    if (!(obj instanceof Foo)) {
+        return false;
+    }
+    Foo other = (Foo)obj;
+    if (id == 0L && other.id == 0L) {
+        return false;
+    }
+    return Objects.equals(someField, other.someField);
+}
+{% endhighlight %}
 
-(`Warning.IDENTICAL_COPY`, which the error message suggests, is not appropriate in this case because that is meant for classes which have no state at all.)
+You will get the same error message. In this case you can simply call `suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY)`, without also suppressing `Warning.SURROGATE_KEY`.
 
 
 ### Materialized fields

equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java

@@ -14,6 +14,7 @@
 import nl.jqno.equalsverifier.internal.reflection.ObjectAccessor;
 import nl.jqno.equalsverifier.internal.reflection.annotations.AnnotationCache;
 import nl.jqno.equalsverifier.internal.reflection.annotations.NonnullAnnotationVerifier;
+import nl.jqno.equalsverifier.internal.reflection.annotations.SupportedAnnotations;
 import nl.jqno.equalsverifier.internal.util.Configuration;
 import nl.jqno.equalsverifier.internal.util.Formatter;
 
@@ -125,13 +126,27 @@ private void checkReflexivityFor(T left, T right) {
             );
             assertFalse(f, left.equals(right));
         } else {
-            Formatter f = Formatter.of(
-                "Reflexivity: object does not equal an identical copy of itself:\n  %%" +
-                "\nIf this is intentional, consider suppressing Warning.%%",
-                left,
-                Warning.IDENTICAL_COPY.toString()
+            boolean isEntity = annotationCache.hasClassAnnotation(
+                typeTag.getType(),
+                SupportedAnnotations.ENTITY
             );
-            assertEquals(f, left, right);
+            if (isEntity) {
+                Formatter f = Formatter.of(
+                    "Reflexivity: entity does not equal an identical copy of itself:\n  %%" +
+                    "\nIf this is intentional, consider suppressing Warning.%%.",
+                    left,
+                    Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString()
+                );
+                assertEquals(f, left, right);
+            } else {
+                Formatter f = Formatter.of(
+                    "Reflexivity: object does not equal an identical copy of itself:\n  %%" +
+                    "\nIf this is intentional, consider suppressing Warning.%%.",
+                    left,
+                    Warning.IDENTICAL_COPY.toString()
+                );
+                assertEquals(f, left, right);
+            }
         }
     }
 }

equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/VersionedEntityTest.java

@@ -3,9 +3,11 @@
 import static nl.jqno.equalsverifier.internal.testhelpers.Util.defaultHashCode;
 
 import java.util.Objects;
+import java.util.UUID;
 import nl.jqno.equalsverifier.EqualsVerifier;
 import nl.jqno.equalsverifier.Warning;
 import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException;
+import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.Entity;
 import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.Id;
 import org.junit.jupiter.api.Test;
 
@@ -19,8 +21,8 @@ public void fail_whenInstanceWithAZeroIdDoesNotEqualItself() {
             .when(() -> EqualsVerifier.forClass(OtherwiseStatelessVersionedEntity.class).verify())
             .assertFailure()
             .assertMessageContains(
-                "object does not equal an identical copy of itself",
-                Warning.IDENTICAL_COPY.toString()
+                "entity does not equal an identical copy of itself",
+                Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString()
             );
     }
 
@@ -48,11 +50,11 @@ public void succeed_whenInstanceWithAZeroIdDoesNotEqualItselfAndInstanceWithANon
     @Test
     public void fail_whenInstanceWithAZeroIdDoesNotEqualItself_givenAVersionedEntityWithState() {
         ExpectedException
-            .when(() -> EqualsVerifier.forClass(StringVersionedEntity.class).verify())
+            .when(() -> EqualsVerifier.forClass(LongIdStringFieldVersionedEntity.class).verify())
             .assertFailure()
             .assertMessageContains(
-                "object does not equal an identical copy of itself",
-                Warning.IDENTICAL_COPY.toString()
+                "entity does not equal an identical copy of itself",
+                Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString()
             );
     }
 
@@ -61,7 +63,7 @@ public void fail_whenInstanceWithANonzeroIdEqualsItself_givenAVersionedEntityWit
         ExpectedException
             .when(() ->
                 EqualsVerifier
-                    .forClass(StringVersionedEntity.class)
+                    .forClass(LongIdStringFieldVersionedEntity.class)
                     .suppress(Warning.IDENTICAL_COPY)
                     .verify()
             )
@@ -72,7 +74,39 @@ public void fail_whenInstanceWithANonzeroIdEqualsItself_givenAVersionedEntityWit
     @Test
     public void succeed_whenInstanceWithAZeroIdDoesNotEqualItselfAndInstanceWithANonzeroIdDoes_givenAVersionedEntityWithStateAndVersionedEntityWarningIsSuppressed() {
         EqualsVerifier
-            .forClass(StringVersionedEntity.class)
+            .forClass(LongIdStringFieldVersionedEntity.class)
+            .suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY, Warning.SURROGATE_KEY)
+            .verify();
+    }
+
+    @Test
+    public void fail_whenInstanceWithANullIdDoesNotEqualItself_givenAVersionedEntityWithState() {
+        ExpectedException
+            .when(() -> EqualsVerifier.forClass(UuidIdStringFieldVersionedEntity.class).verify())
+            .assertFailure()
+            .assertMessageContains(
+                "entity does not equal an identical copy of itself",
+                Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString()
+            );
+    }
+
+    @Test
+    public void fail_whenInstanceWithANonnullIdEqualsItself_givenAVersionedEntityWithStateAndIdenticalCopyWarningIsSuppressed() {
+        ExpectedException
+            .when(() ->
+                EqualsVerifier
+                    .forClass(UuidIdStringFieldVersionedEntity.class)
+                    .suppress(Warning.IDENTICAL_COPY)
+                    .verify()
+            )
+            .assertFailure()
+            .assertMessageContains("Unnecessary suppression", Warning.IDENTICAL_COPY.toString());
+    }
+
+    @Test
+    public void succeed_whenInstanceWithANullIdDoesNotEqualItselfAndInstanceWithANonnullIdDoes_givenAVersionedEntityWithStateAndVersionedEntityWarningIsSuppressed() {
+        EqualsVerifier
+            .forClass(UuidIdStringFieldVersionedEntity.class)
             .suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY, Warning.SURROGATE_KEY)
             .verify();
     }
@@ -143,6 +177,7 @@ public void fail_whenTheExceptionIsThrownInADifficultToReachPartOfTheSubclassOfA
             .assertMessageContains("catch me if you can");
     }
 
+    @Entity
     public static final class OtherwiseStatelessVersionedEntity {
 
         @Id
@@ -170,25 +205,26 @@ public int hashCode() {
         }
     }
 
-    public static final class StringVersionedEntity {
+    @Entity
+    public static final class LongIdStringFieldVersionedEntity {
 
         @Id
         private final long id;
 
         @SuppressWarnings("unused")
         private final String s;
 
-        public StringVersionedEntity(long id, String s) {
+        public LongIdStringFieldVersionedEntity(long id, String s) {
             this.id = id;
             this.s = s;
         }
 
         @Override
         public boolean equals(Object obj) {
-            if (!(obj instanceof StringVersionedEntity)) {
+            if (!(obj instanceof LongIdStringFieldVersionedEntity)) {
                 return false;
             }
-            StringVersionedEntity other = (StringVersionedEntity) obj;
+            LongIdStringFieldVersionedEntity other = (LongIdStringFieldVersionedEntity) obj;
             if (id == 0L && other.id == 0L) {
                 return false;
             }
@@ -201,6 +237,39 @@ public int hashCode() {
         }
     }
 
+    @Entity
+    public static final class UuidIdStringFieldVersionedEntity {
+
+        @Id
+        private final UUID id;
+
+        @SuppressWarnings("unused")
+        private final String s;
+
+        public UuidIdStringFieldVersionedEntity(UUID id, String s) {
+            this.id = id;
+            this.s = s;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof UuidIdStringFieldVersionedEntity)) {
+                return false;
+            }
+            UuidIdStringFieldVersionedEntity other = (UuidIdStringFieldVersionedEntity) obj;
+            if (id == null && other.id == null) {
+                return false;
+            }
+            return Objects.equals(id, other.id);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(id);
+        }
+    }
+
+    @Entity
     public static final class WeakStringVersionedEntity {
 
         @Id
@@ -231,6 +300,7 @@ public int hashCode() {
         }
     }
 
+    @Entity
     public static final class NullCheckStringVersionedEntity {
 
         @Id
@@ -259,6 +329,7 @@ public int hashCode() {
         }
     }
 
+    @Entity
     public static final class BusinessKeyStringVersionedEntity {
 
         @Id
@@ -286,6 +357,7 @@ public int hashCode() {
         }
     }
 
+    @Entity
     private static class CanEqualVersionedEntity {
 
         private final Long id;
@@ -318,6 +390,7 @@ public final int hashCode() {
         }
     }
 
+    @Entity
     private static class NonReflexiveCanEqualVersionedEntity extends CanEqualVersionedEntity {
 
         public NonReflexiveCanEqualVersionedEntity(Long id) {