DualValue hash code is not computed properly because it calls actual.hashCode() and expected.hashCode() when equals compares actual and expected references.

davidboden · Joel Costigliola · commit e9ebcc2c1b70 · 2024-01-21T17:40:31.000+01:00
Fix #3340
diff --git a/assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/DualValue.java b/assertj-core/src/main/java/org/assertj/core/api/recursive/comparison/DualValue.java
@@ -13,6 +13,7 @@
 package org.assertj.core.api.recursive.comparison;
 
 import static java.lang.String.format;
+import static java.lang.System.identityHashCode;
 import static java.util.Collections.unmodifiableList;
 import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.recursive.comparison.FieldLocation.rootFieldLocation;
@@ -23,7 +24,6 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalDouble;
 import java.util.OptionalInt;
@@ -61,7 +61,11 @@ public DualValue(FieldLocation fieldLocation, Object actualFieldValue, Object ex
     this.fieldLocation = requireNonNull(fieldLocation, "fieldLocation must not be null");
     actual = actualFieldValue;
     expected = expectedFieldValue;
-    hashCode = Objects.hash(actual, expected);
+    hashCode = computeHashCode();
+  }
+
+  private int computeHashCode() {
+    return identityHashCode(actual) + identityHashCode(expected) + fieldLocation.hashCode();
   }
 
   @Override
diff --git a/assertj-core/src/test/java/org/assertj/core/api/recursive/comparison/DualValue_Test.java b/assertj-core/src/test/java/org/assertj/core/api/recursive/comparison/DualValue_Test.java
@@ -0,0 +1,77 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * Copyright 2012-2024 the original author or authors.
+ */
+package org.assertj.core.api.recursive.comparison;
+
+import org.junit.jupiter.api.Test;
+
+import nl.jqno.equalsverifier.EqualsVerifier;
+
+import java.util.Objects;
+
+import static java.util.Objects.hash;
+import static org.assertj.core.api.BDDAssertions.then;
+import static org.assertj.core.util.Lists.list;
+
+class DualValue_Test {
+
+  @Test
+  void should_honor_equals_contract() {
+    EqualsVerifier.forClass(DualValue.class)
+                  .withNonnullFields("fieldLocation")
+                  .withCachedHashCode("hashCode", "computeHashCode", new DualValue(list(), "foo", "bar"))
+                  .verify();
+  }
+
+  private static final class MutableType {
+    String changeMe;
+
+    private MutableType(String startingValue) {
+      changeMe = startingValue;
+    }
+
+    private void setChangeMe(String nextValue) {
+      changeMe = nextValue;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      MutableType that = (MutableType) o;
+      return Objects.equals(changeMe, that.changeMe);
+    }
+
+    @Override
+    public int hashCode() {
+      return hash(changeMe);
+    }
+  }
+
+  @Test
+  void should_return_same_hashcode_after_mutation() {
+    // GIVEN
+    MutableType actual = new MutableType("One");
+    MutableType expected = new MutableType("One");
+    DualValue dualValue1 = new DualValue(list(), actual, expected);
+    DualValue dualValue2 = new DualValue(list(), actual, expected);
+    int hashCodeBeforeMutation = dualValue1.hashCode();
+    // WHEN
+    actual.setChangeMe("Another value");
+    // THEN
+    then(dualValue1).isEqualTo(dualValue2)
+                    .hasSameHashCodeAs(dualValue2);
+    int hashCodeAfterMutation = dualValue1.hashCode();
+    then(hashCodeAfterMutation).isEqualTo(hashCodeBeforeMutation);
+  }
+
+}
