From cf00eef58507730ee3941acc7a8091ab623ce64d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Hoffmann?= Date: Tue, 6 Jul 2021 21:16:29 +0200 Subject: [PATCH] #460: Add support for @EqualsAndHashCode(cacheStrategy = LAZY) --- ...als-relies-on-foo-but-hashcode-does-not.md | 17 +++ .../api/SingleTypeEqualsVerifierApi.java | 14 ++ .../util/CachedHashCodeInitializer.java | 40 ++++-- .../LombokLazyEqualsAndHashcodeTest.java | 127 ++++++++++++++++++ 4 files changed, 190 insertions(+), 8 deletions(-) create mode 100644 src/test/java/nl/jqno/equalsverifier/integration/extra_features/LombokLazyEqualsAndHashcodeTest.java diff --git a/docs/_errormessages/significant-fields-equals-relies-on-foo-but-hashcode-does-not.md b/docs/_errormessages/significant-fields-equals-relies-on-foo-but-hashcode-does-not.md index d80691498..a88c070f5 100644 --- a/docs/_errormessages/significant-fields-equals-relies-on-foo-but-hashcode-does-not.md +++ b/docs/_errormessages/significant-fields-equals-relies-on-foo-but-hashcode-does-not.md @@ -51,4 +51,21 @@ EqualsVerifier.forClass(CachedHashCode.class) .withCachedHashCode("cachedHashCode", "calculateHashCode", new CachedHashCode()); {% endhighlight %} +Another scenario in which you might experience this error message is when using Lombok's `@EqualsAndHashCode` with `cacheStrategy=LAZY`: + +{% highlight java %} +@RequiredArgsConstructor +@EqualsAndHashCode(cacheStrategy = EqualsAndHashCode.CacheStrategy.LAZY) +public class CachedHashCode { + private final String foo; +} +{% endhighlight %} + +Using `.withLombokCachedHashCode` allows to test those classes as well: + +{% highlight java %} +EqualsVerifier.forClass(LazyPojo.class) + .withLombokCachedHashCode(new CachedHashCode("bar")); +{% endhighlight %} + For more help on how to use `withCachedHashCode`, read the [manual page about it](/equalsverifier/manual/caching-hashcodes). diff --git a/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java b/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java index 79a64b835..8be478d8c 100644 --- a/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java +++ b/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java @@ -278,6 +278,20 @@ public SingleTypeEqualsVerifierApi withCachedHashCode( return this; } + /** + * Signals that T uses Lombok to cache its hashCode, instead of re-calculating it each time the + * {@code hashCode()} method is called. + * + * @param example An instance of the class under test, to verify that the hashCode has been + * initialized properly. + * @return {@code this}, for easy method chaining. + * @see #withCachedHashCode(String, String, T) + */ + public SingleTypeEqualsVerifierApi withLombokCachedHashCode(T example) { + cachedHashCodeInitializer = CachedHashCodeInitializer.lombokCachedHashcode(example); + return this; + } + /** * Performs the verification of the contracts for {@code equals} and {@code hashCode} and throws * an {@link AssertionError} if there is a problem. diff --git a/src/main/java/nl/jqno/equalsverifier/internal/util/CachedHashCodeInitializer.java b/src/main/java/nl/jqno/equalsverifier/internal/util/CachedHashCodeInitializer.java index 02f898fc3..d552da3c4 100644 --- a/src/main/java/nl/jqno/equalsverifier/internal/util/CachedHashCodeInitializer.java +++ b/src/main/java/nl/jqno/equalsverifier/internal/util/CachedHashCodeInitializer.java @@ -30,10 +30,19 @@ public class CachedHashCodeInitializer { private final T example; private CachedHashCodeInitializer() { - this.passthrough = true; - this.cachedHashCodeField = null; - this.calculateMethod = null; - this.example = null; + this(true, null, null, null); + } + + private CachedHashCodeInitializer( + boolean passthrough, + Field cachedHashCodeField, + Method calculateMethod, + T example + ) { + this.passthrough = passthrough; + this.cachedHashCodeField = cachedHashCodeField; + this.calculateMethod = calculateMethod; + this.example = example; } public CachedHashCodeInitializer( @@ -44,7 +53,7 @@ public CachedHashCodeInitializer( ) { this.passthrough = false; this.cachedHashCodeField = findCachedHashCodeField(type, cachedHashCodeField); - this.calculateMethod = findCalculateHashCodeMethod(type, calculateHashCodeMethod); + this.calculateMethod = findCalculateHashCodeMethod(type, calculateHashCodeMethod, false); this.example = example; } @@ -52,6 +61,17 @@ public static CachedHashCodeInitializer passthrough() { return new CachedHashCodeInitializer<>(); } + public static CachedHashCodeInitializer lombokCachedHashcode(T example) { + @SuppressWarnings("unchecked") + final Class type = (Class) example.getClass(); + return new CachedHashCodeInitializer<>( + false, + findCachedHashCodeField(type, "$hashCodeCache"), + findCalculateHashCodeMethod(type, "hashCode", true), + example + ); + } + public boolean isPassthrough() { return passthrough; } @@ -85,7 +105,7 @@ private void recomputeCachedHashCode(Object object) { ); } - private Field findCachedHashCodeField(Class type, String cachedHashCodeFieldName) { + private static Field findCachedHashCodeField(Class type, String cachedHashCodeFieldName) { for (Field candidateField : FieldIterable.of(type)) { if (candidateField.getName().equals(cachedHashCodeFieldName)) { if ( @@ -104,12 +124,16 @@ private Field findCachedHashCodeField(Class type, String cachedHashCodeFieldN ); } - private Method findCalculateHashCodeMethod(Class type, String calculateHashCodeMethodName) { + private static Method findCalculateHashCodeMethod( + Class type, + String calculateHashCodeMethodName, + boolean acceptPublicMethod + ) { for (Class currentClass : SuperclassIterable.ofIncludeSelf(type)) { try { Method method = currentClass.getDeclaredMethod(calculateHashCodeMethodName); if ( - !Modifier.isPublic(method.getModifiers()) && + (acceptPublicMethod || !Modifier.isPublic(method.getModifiers())) && method.getReturnType().equals(int.class) ) { method.setAccessible(true); diff --git a/src/test/java/nl/jqno/equalsverifier/integration/extra_features/LombokLazyEqualsAndHashcodeTest.java b/src/test/java/nl/jqno/equalsverifier/integration/extra_features/LombokLazyEqualsAndHashcodeTest.java new file mode 100644 index 000000000..3b23eabb6 --- /dev/null +++ b/src/test/java/nl/jqno/equalsverifier/integration/extra_features/LombokLazyEqualsAndHashcodeTest.java @@ -0,0 +1,127 @@ +package nl.jqno.equalsverifier.integration.extra_features; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.StringContains.containsString; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import nl.jqno.equalsverifier.EqualsVerifier; +import nl.jqno.equalsverifier.Warning; +import org.junit.jupiter.api.Test; + +// CHECKSTYLE OFF: LocalFinalVariableName +// CHECKSTYLE OFF: MemberName +// CHECKSTYLE OFF: NeedBraces + +public class LombokLazyEqualsAndHashcodeTest { + + @Test + void testWithLombokCachedHashCode() { + EqualsVerifier + .forClass(LazyPojo.class) + .withLombokCachedHashCode(new LazyPojo("a", new Object())) + .suppress(Warning.STRICT_INHERITANCE) + .verify(); + } + + @Test + void testDefaultEqualsVerifierFailsForCachedLombokEqualsAndHashcode() { + final AssertionError error = assertThrows( + AssertionError.class, + () -> + EqualsVerifier + .forClass(LazyPojo.class) + .suppress(Warning.STRICT_INHERITANCE) + .verify() + ); + assertThat( + error.getMessage(), + containsString("hashCode relies on $hashCodeCache, but equals does not.") + ); + } + + @Test + void testDefaultEqualsVerifierFailsForCachedLombokEqualsAndHashcodeWhenUsingWithCachedHashCode() { + final IllegalArgumentException error = assertThrows( + IllegalArgumentException.class, + () -> + EqualsVerifier + .forClass(LazyPojo.class) + .suppress(Warning.STRICT_INHERITANCE) + .withCachedHashCode( + "$hashCodeCache", + "hashCode", + new LazyPojo("bar", new Object()) + ) + .verify() + ); + assertThat( + error.getMessage(), + containsString( + "Cached hashCode: Could not find calculateHashCodeMethod: must be 'private int hashCode()'" + ) + ); + } + + /** + * This class has been generated with Lombok (1.18.20). It is equivalent to: + * 
+     *   @RequiredArgsConstructor
+     *   @EqualsAndHashCode(cacheStrategy = EqualsAndHashCode.CacheStrategy.LAZY)
+     *   public class LazyPojo {
+     *
+     *     private final String foo;
+     *
+     *     private final Object bar;
+     *   }
+     *
+ */ + @SuppressWarnings({ "RedundantIfStatement", "EqualsReplaceableByObjectsCall" }) + private static class LazyPojo { + + private transient int $hashCodeCache; + + private final String foo; + private final Object bar; + + public LazyPojo(String foo, Object bar) { + this.foo = foo; + this.bar = bar; + } + + @Override + public boolean equals(final Object o) { + if (o == this) return true; + if (!(o instanceof LazyPojo)) return false; + final LazyPojo other = (LazyPojo) o; + if (!other.canEqual(this)) return false; + final Object this$foo = this.foo; + final Object other$foo = other.foo; + if (this$foo == null ? other$foo != null : !this$foo.equals(other$foo)) return false; + final Object this$bar = this.bar; + final Object other$bar = other.bar; + if (this$bar == null ? other$bar != null : !this$bar.equals(other$bar)) return false; + return true; + } + + protected boolean canEqual(Object other) { + return other instanceof LazyPojo; + } + + @Override + public int hashCode() { + if (this.$hashCodeCache != 0) { + return this.$hashCodeCache; + } else { + final int PRIME = 59; + int result = 1; + final Object $foo = this.foo; + result = result * PRIME + ($foo == null ? 43 : $foo.hashCode()); + final Object $bar = this.bar; + result = result * PRIME + ($bar == null ? 43 : $bar.hashCode()); + + this.$hashCodeCache = result; + return result; + } + } + } +}