Checks Warning.SURROGATE_KEY also when @id annotation is in super

jqno · Feb 10, 2023 · 0d11a2f · 0d11a2f
1 parent 21e5364
commit 0d11a2f
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,10 +13,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-### Changed
+## Fixed
+
+- `Warning.SURROGATE_KEY` was ignored when `@Id` annotation is in entity's superclass. ([Issue 763](https://github.com/jqno/equalsverifier/issues/763))
 
 ## [3.13] - 2023-02-02
 
+### Changed
+
 - Made it possible to suppress both `Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY` and `Warning.SURROGATE_KEY`. Note that in some cases when suppressing `Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY` on a class with a surrogate key, it is now necessary to also explicitly suppress `Warning.SURROGATE_KEY` ([Issue 755](https://github.com/jqno/equalsverifier/issues/755))
 
 ## [3.12.4] - 2023-01-28

diff --git a/...main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/SignificantFieldCheck.java b/...main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/SignificantFieldCheck.java
@@ -200,7 +200,7 @@ private boolean fieldIsEmptyAndItsOk(
         Object value = fieldAccessor.get(object);
         Class<?> fieldType = fieldAccessor.getFieldType();
         Object zero = PrimitiveMappers.DEFAULT_WRAPPED_VALUE_MAPPER.get(fieldType);
-        boolean fieldIsEmpty = value == null || (value.equals(zero));
+        boolean fieldIsEmpty = value == null || value.equals(zero);
 
         return (
             thisFieldIsMarkedAsId &&

diff --git a/...ain/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java b/...ain/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java
@@ -169,7 +169,7 @@ public boolean validate(
      * equals/hashCode contract, unless {@link Warning#SURROGATE_KEY} is suppressed.
      */
     ID(
-        false,
+        true,
         "javax.persistence.Id",
         "javax.persistence.EmbeddedId",
         "jakarta.persistence.Id",

diff --git a/...r-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaIdTest.java b/...r-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaIdTest.java
@@ -394,6 +394,11 @@ public void fail_whenANaturalIdAnnotationFromAnotherPackageIsUsed() {
             .assertMessageContains("Significant fields");
     }
 
+    @Test
+    public void succeed_whenEqualsIsImplementedInSuperclass_givenWarningSurrogateKeyIsSuppressed() {
+        EqualsVerifier.forClass(SubclassEntity.class).suppress(Warning.SURROGATE_KEY).verify();
+    }
+
     static class JakartaIdBusinessKeyPerson {
 
         @jakarta.persistence.Id
@@ -956,4 +961,38 @@ public final int hashCode() {
             return Objects.hash(socialSecurity);
         }
     }
+
+    static class AbstractEntity {
+
+        @jakarta.persistence.Id
+        private final UUID id;
+
+        public AbstractEntity(UUID id) {
+            this.id = id;
+        }
+
+        @Override
+        public final boolean equals(Object obj) {
+            if (!(obj instanceof AbstractEntity)) {
+                return false;
+            }
+            AbstractEntity other = (AbstractEntity) obj;
+            return Objects.equals(id, other.id);
+        }
+
+        @Override
+        public final int hashCode() {
+            return Objects.hash(id);
+        }
+    }
+
+    static class SubclassEntity extends AbstractEntity {
+
+        private final String name;
+
+        public SubclassEntity(UUID id, String name) {
+            super(id);
+            this.name = name;
+        }
+    }
 }
diff --git a/...ifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaIdTest.java b/...ifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaIdTest.java
@@ -410,6 +410,11 @@ public void fail_whenANaturalIdAnnotationFromAnotherPackageIsUsed() {
             .assertMessageContains("Significant fields");
     }
 
+    @Test
+    public void succeed_whenEqualsIsImplementedInSuperclass_givenWarningSurrogateKeyIsSuppressed() {
+        EqualsVerifier.forClass(SubclassEntity.class).suppress(Warning.SURROGATE_KEY).verify();
+    }
+
     static class JpaIdBusinessKeyPerson {
 
         @Id
@@ -1049,4 +1054,38 @@ public final int hashCode() {
             return Objects.hash(socialSecurity);
         }
     }
+
+    static class AbstractEntity {
+
+        @Id
+        private final UUID id;
+
+        public AbstractEntity(UUID id) {
+            this.id = id;
+        }
+
+        @Override
+        public final boolean equals(Object obj) {
+            if (!(obj instanceof AbstractEntity)) {
+                return false;
+            }
+            AbstractEntity other = (AbstractEntity) obj;
+            return Objects.equals(id, other.id);
+        }
+
+        @Override
+        public final int hashCode() {
+            return Objects.hash(id);
+        }
+    }
+
+    static class SubclassEntity extends AbstractEntity {
+
+        private final String name;
+
+        public SubclassEntity(UUID id, String name) {
+            super(id);
+            this.name = name;
+        }
+    }
 }