jqno#540 BigDecimal equality using compareTo

docs/_manual/12-bigdecimal-equality.md

@@ -0,0 +1,30 @@
+---
+title: "`BigDecimal` equality using `compareTo(BigDecimal val)`"
+permalink: /manual/bigdecimal-equality/
+---
+The `Comparable` interface strongly recommends but does not require that implementations consider two objects equal using `compareTo` whenever they are equal using `equals` and vice versa. `BigDecimal` is a class where this is not applied.
+
+{% highlight java %}
+BigDecimal zero = new BigDecimal("0");
+BigDecimal alsoZero = new BigDecimal("0.0");
+
+// prints true - zero is the same as zero
+System.out.println(zero.compareTo(alsoZero) == 0);
+// prints false - zero is not the same as zero
+System.out.println(zero.equals(alsoZero));
+{% endhighlight %}
+
+This is because `BigDecimal` can have multiple representations of the same value. It uses an unscaled value and a scale so, for example, the value of 1 can be represented as unscaled value 1 with scale of 0 (the number of places after the decimal point) or as unscaled value 10 with scale of 1 resolving to 1.0. Its `equals` and `hashCode` methods use both of these attributes in their calculation rather than the resolved value.
+
+If your class contains any `BigDecimal` fields, in order for comparably equal values to be considered the same, then the `equals` method must use `compareTo` for the check and the `hashCode` calculation must derive the same value for all `BigDecimal` instances that are equal using `compareTo` (taking care if it is a nullable field).
+
+EqualsVerifier will check this by default. If you do not have this requirement the check can be disabled by suppressing `Warning.BIGDECIMAL_EQUALITY`.
+
+{% highlight java %}
+EqualsVerifier.forClass(FooWithComparablyEqualBigDecimalFields.class)
+    .suppress(Warning.BIGDECIMAL_EQUALITY)
+    .verify();
+{% endhighlight %}
+
+There is more information on `compareTo` and `equals` in the `Comparable` Javadoc and Effective Java's chapter on implementing `Comparable`.
+There is more information on `BigDecimal` in its Javadoc (and its representation can be seen by printing `unscaledValue()` and `scale()`).

docs/_manual/12-legacy-systems.md → docs/_manual/13-legacy-systems.md

@@ -17,6 +17,7 @@ For these situations, there are several types of warnings that you can suppress:
 * `Warning.STRICT_HASHCODE`: disables the check that all fields used in `equals` must also be used in `hashCode`.
 * `Warning.STRICT_INHERITANCE`: disables the check that classes or their `equals` methods be final, or that inheritance is properly accounted for. Read more about this topic on the [page about inheritance](/equalsverifier/manual/inheritance).
 * `Warning.TRANSIENT_FIELDS`: disables the check that transient fields do not participate in `equals`. This applies both to Java's `transient` keyword, which applies to serialization, and to JPA's `@Transient` annotation, which applies to, well, JPA.
+* `Warning.BIGDECIMAL_EQUALITY`: disables the check that equality of `BigDecimal` fields is implemented using `compareTo` rather than `equals`. Read more about this topic on the [page about BigDecimal equality](/equalsverifier/manual/bigdecimal-equality).
 
 Of course, once you have sufficient test coverage, you _will_ come back and fix these issues, right? 😉
 

src/main/java/nl/jqno/equalsverifier/Warning.java

@@ -166,5 +166,20 @@ public enum Warning {
      * <p>If measures are taken that this will never happen, this warning can be suppressed to
      * disable {@link EqualsVerifier}'s transience test.
      */
-    TRANSIENT_FIELDS
+    TRANSIENT_FIELDS,
+
+    /**
+     * Disables the check that equality of {@code BigDecimal} fields is implemented using
+     * {@code compareTo} rather than {@code equals}.
+     *
+     * <p>{@code BigDecimal} objects that are equal using {@code compareTo} are not necessarily
+     * equal using {@code equals}, for example the values of {@literal 1} and {@literal 1.0}.
+     * For variants of the same value to be considered equal, classes with {@code BigDecimal}
+     * fields should use {@code compareTo} to check equality of non-null {@code BigDecimal}
+     * references and produce the same hashcode for all equal variants.
+     *
+     * <p>{@code EqualsVerifier} checks for this by default but it can be disabled by suppressing
+     * this warning.
+     */
+    BIGDECIMAL_EQUALITY
 }

src/main/java/nl/jqno/equalsverifier/internal/checkers/FieldsChecker.java

@@ -23,6 +23,7 @@ public class FieldsChecker<T> implements Checker {
     private final SymmetryFieldCheck<T> symmetryFieldCheck;
     private final TransientFieldsCheck<T> transientFieldsCheck;
     private final TransitivityFieldCheck<T> transitivityFieldCheck;
+    private final BigDecimalFieldCheck<T> bigDecimalFieldCheck;
 
     public FieldsChecker(Configuration<T> config) {
         this.config = config;
@@ -48,6 +49,8 @@ public FieldsChecker(Configuration<T> config) {
         this.symmetryFieldCheck = new SymmetryFieldCheck<>(prefabValues, typeTag);
         this.transientFieldsCheck = new TransientFieldsCheck<>(config);
         this.transitivityFieldCheck = new TransitivityFieldCheck<>(prefabValues, typeTag);
+        this.bigDecimalFieldCheck =
+            new BigDecimalFieldCheck<>(config.getCachedHashCodeInitializer());
     }
 
     @Override
@@ -80,6 +83,10 @@ public void check() {
                 skippingSignificantFieldCheck
             );
         }
+
+        if (!config.getWarningsToSuppress().contains(Warning.BIGDECIMAL_EQUALITY)) {
+            inspector.check(bigDecimalFieldCheck);
+        }
     }
 
     private boolean ignoreMutability(Class<?> type) {

src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/BigDecimalFieldCheck.java

@@ -0,0 +1,87 @@
+package nl.jqno.equalsverifier.internal.checkers.fieldchecks;
+
+import static nl.jqno.equalsverifier.internal.util.Assert.assertEquals;
+
+import java.lang.reflect.Field;
+import java.math.BigDecimal;
+import java.math.RoundingMode;
+import nl.jqno.equalsverifier.Warning;
+import nl.jqno.equalsverifier.internal.reflection.FieldAccessor;
+import nl.jqno.equalsverifier.internal.reflection.ObjectAccessor;
+import nl.jqno.equalsverifier.internal.util.CachedHashCodeInitializer;
+import nl.jqno.equalsverifier.internal.util.Formatter;
+
+public class BigDecimalFieldCheck<T> implements FieldCheck<T> {
+
+    private final CachedHashCodeInitializer<T> cachedHashCodeInitializer;
+
+    public BigDecimalFieldCheck(CachedHashCodeInitializer<T> cachedHashCodeInitializer) {
+        this.cachedHashCodeInitializer = cachedHashCodeInitializer;
+    }
+
+    @Override
+    public void execute(
+        ObjectAccessor<T> referenceAccessor,
+        ObjectAccessor<T> copyAccessor,
+        FieldAccessor fieldAccessor
+    ) {
+        if (BigDecimal.class.equals(fieldAccessor.getFieldType())) {
+            Field field = fieldAccessor.getField();
+            BigDecimal referenceField = (BigDecimal) referenceAccessor.getField(field);
+            BigDecimal changedField = referenceField.setScale(
+                referenceField.scale() + 1,
+                RoundingMode.UNNECESSARY
+            );
+            ObjectAccessor<T> changed = copyAccessor.withFieldSetTo(field, changedField);
+
+            T left = referenceAccessor.get();
+            T right = changed.get();
+
+            checkEquals(field, referenceField, changedField, left, right);
+            checkHashCode(field, referenceField, changedField, left, right);
+        }
+    }
+
+    private void checkEquals(
+        Field field,
+        BigDecimal referenceField,
+        BigDecimal changedField,
+        T left,
+        T right
+    ) {
+        Formatter f = Formatter.of(
+            "BigDecimal equality by comparison: object does not equal a copy of itself where BigDecimal field %%" +
+            " has a value that is equal using compareTo: %% compared to %%" +
+            "\nIf these values should be considered equal then use compareTo rather than equals for this field." +
+            "\nIf these values should not be considered equal, suppress Warning.%% to disable this check.",
+            field.getName(),
+            referenceField,
+            changedField,
+            Warning.BIGDECIMAL_EQUALITY
+        );
+        assertEquals(f, left, right);
+    }
+
+    private void checkHashCode(
+        Field field,
+        BigDecimal referenceField,
+        BigDecimal changedField,
+        T left,
+        T right
+    ) {
+        Formatter f = Formatter.of(
+            "BigDecimal equality by comparison: hashCode of object does not equal hashCode of a copy of itself" +
+            " where BigDecimal field %%" +
+            " has a value that is equal using compareTo: %% compared to %%" +
+            "\nIf these values should be considered equal then make sure to derive the same constituent hashCode from this field." +
+            "\nIf these values should not be considered equal, suppress Warning.%% to disable this check.",
+            field.getName(),
+            referenceField,
+            changedField,
+            Warning.BIGDECIMAL_EQUALITY
+        );
+        int leftHashCode = cachedHashCodeInitializer.getInitializedHashCode(left);
+        int rightHashCode = cachedHashCodeInitializer.getInitializedHashCode(right);
+        assertEquals(f, leftHashCode, rightHashCode);
+    }
+}

src/test/java/nl/jqno/equalsverifier/integration/extended_contract/BigDecimalTest.java

@@ -0,0 +1,151 @@
+package nl.jqno.equalsverifier.integration.extended_contract;
+
+import static nl.jqno.equalsverifier.testhelpers.Util.defaultHashCode;
+
+import java.math.BigDecimal;
+import java.util.Objects;
+import nl.jqno.equalsverifier.EqualsVerifier;
+import nl.jqno.equalsverifier.Warning;
+import nl.jqno.equalsverifier.testhelpers.ExpectedException;
+import org.junit.jupiter.api.Test;
+
+public class BigDecimalTest {
+
+    @Test
+    public void fail_whenBigDecimalsComparedUsingEquals() {
+        ExpectedException
+            .when(() -> EqualsVerifier.forClass(BigDecimalEquals.class).verify())
+            .assertFailure()
+            .assertMessageContains(
+                "BigDecimal",
+                "equals",
+                "compareTo",
+                "bd",
+                Warning.BIGDECIMAL_EQUALITY.toString()
+            );
+    }
+
+    @Test
+    public void succeed_whenBigDecimalsComparedUsingEquals_givenBigDecimalEqualsWarningIsSuppressed() {
+        EqualsVerifier
+            .forClass(BigDecimalEquals.class)
+            .suppress(Warning.BIGDECIMAL_EQUALITY)
+            .verify();
+    }
+
+    @Test
+    public void succeed_whenBigDecimalsComparedUsingCompareTo() {
+        EqualsVerifier.forClass(BigDecimalCompareTo.class).verify();
+    }
+
+    @Test
+    public void fail_whenBigDecimalsComparedUsingCompareToWithInconsistentHashCode() {
+        ExpectedException
+            .when(() -> EqualsVerifier.forClass(BigDecimalInconsistentHashCode.class).verify())
+            .assertFailure()
+            .assertMessageContains(
+                "BigDecimal",
+                "hashCode",
+                "compareTo",
+                "bd",
+                Warning.BIGDECIMAL_EQUALITY.toString()
+            );
+    }
+
+    /**
+     * Uses standard equals and hashCode for objects.
+     * 0 and 0.0 are not equal.
+     */
+    private static final class BigDecimalEquals {
+
+        private final BigDecimal bd;
+
+        public BigDecimalEquals(BigDecimal bd) {
+            this.bd = bd;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof BigDecimalEquals)) {
+                return false;
+            }
+            BigDecimalEquals other = (BigDecimalEquals) obj;
+            return Objects.equals(bd, other.bd);
+        }
+
+        @Override
+        public int hashCode() {
+            return defaultHashCode(this);
+        }
+    }
+
+    /**
+     * Uses compareTo for BigDecimal equality and ensures hashCode is equal for equal BigDecimal instances.
+     * 0 and 0.0 are equal and produce the same hashCode.
+     */
+    private static final class BigDecimalCompareTo {
+
+        private final BigDecimal bd;
+
+        public BigDecimalCompareTo(BigDecimal bd) {
+            this.bd = bd;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof BigDecimalCompareTo)) {
+                return false;
+            }
+            BigDecimalCompareTo other = (BigDecimalCompareTo) obj;
+            return comparablyEquals(bd, other.bd);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hashCode(comparablyHashed(bd));
+        }
+    }
+
+    /**
+     * Uses compareTo for BigDecimal equality but has hashCode that may be inconsistent.
+     * 0 and 0.0 are equal but may produce a different hashCode.
+     */
+    private static final class BigDecimalInconsistentHashCode {
+
+        private final BigDecimal bd;
+
+        public BigDecimalInconsistentHashCode(BigDecimal bd) {
+            this.bd = bd;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof BigDecimalInconsistentHashCode)) {
+                return false;
+            }
+            BigDecimalInconsistentHashCode other = (BigDecimalInconsistentHashCode) obj;
+            return comparablyEquals(bd, other.bd);
+        }
+
+        @Override
+        public int hashCode() {
+            return defaultHashCode(this);
+        }
+    }
+
+    /**
+     * Checks equality using compareTo rather than equals.
+     */
+    private static boolean comparablyEquals(BigDecimal left, BigDecimal right) {
+        boolean bothNull = left == null && right == null;
+        boolean bothNonNullAndEqual = left != null && right != null && left.compareTo(right) == 0;
+        return bothNull || bothNonNullAndEqual;
+    }
+
+    /**
+     * Returns a instance (or null) that produces the same hashCode as any other instance that is equal using compareTo.
+     */
+    private static BigDecimal comparablyHashed(BigDecimal bd) {
+        return bd != null ? bd.stripTrailingZeros() : null;
+    }
+}