Add a bugpattern to point out that BigDecimal's equals method does not

compare for just numeric equality.

I've avoided suggesting a change within #equals implementations, as we're pretty likely to give people an inconsistent hashCode if we replace that.

RELNOTES: [BigDecimalEquals] Suggest using compareTo when comparing BigDecimals, not equals

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=211323233

core/src/main/java/com/google/errorprone/bugpatterns/BigDecimalEquals.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright 2018 The Error Prone Authors.
+ *
+ * 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.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import static com.google.common.collect.Iterables.getLast;
+import static com.google.errorprone.BugPattern.Category.JDK;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.equalsMethodDeclaration;
+import static com.google.errorprone.matchers.Matchers.instanceEqualsInvocation;
+import static com.google.errorprone.matchers.Matchers.staticEqualsInvocation;
+import static com.google.errorprone.util.ASTHelpers.getReceiver;
+import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.isSameType;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.BugPattern.ProvidesFix;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Type;
+import java.util.List;
+
+/**
+ * Matches use of {@code BigDecimal#equals}, which compares scale as well (which is not likely to be
+ * intended).
+ *
+ * @author ghm@google.com (Graeme Morgan)
+ */
+@BugPattern(
+    name = "BigDecimalEquals",
+    summary = "BigDecimal#equals has surprising behavior: it also compares scale.",
+    category = JDK,
+    severity = WARNING,
+    providesFix = ProvidesFix.REQUIRES_HUMAN_ATTENTION)
+public final class BigDecimalEquals extends BugChecker implements MethodInvocationTreeMatcher {
+  private static final String BIG_DECIMAL = "java.math.BigDecimal";
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    Tree receiver;
+    Tree argument;
+    List<? extends ExpressionTree> arguments = tree.getArguments();
+    Type bigDecimal = state.getTypeFromString(BIG_DECIMAL);
+    boolean handleNulls;
+
+    if (staticEqualsInvocation().matches(tree, state)) {
+      handleNulls = true;
+      receiver = arguments.get(arguments.size() - 2);
+      argument = getLast(arguments);
+    } else if (instanceEqualsInvocation().matches(tree, state)) {
+      handleNulls = false;
+      receiver = getReceiver(tree);
+      argument = arguments.get(0);
+    } else {
+      return NO_MATCH;
+    }
+    MethodTree enclosingMethod = state.findEnclosing(MethodTree.class);
+    if (enclosingMethod != null && equalsMethodDeclaration().matches(enclosingMethod, state)) {
+      return NO_MATCH;
+    }
+
+    boolean isReceiverBigDecimal = isSameType(getType(receiver), bigDecimal, state);
+    boolean isTargetBigDecimal = isSameType(getType(argument), bigDecimal, state);
+
+    if (!isReceiverBigDecimal && !isTargetBigDecimal) {
+      return NO_MATCH;
+    }
+
+    // One is BigDecimal but the other isn't: report a finding without a fix.
+    if (isReceiverBigDecimal != isTargetBigDecimal) {
+      return describeMatch(tree);
+    }
+    return describe(tree, state, receiver, argument, handleNulls);
+  }
+
+  private Description describe(
+      MethodInvocationTree tree,
+      VisitorState state,
+      Tree receiver,
+      Tree argument,
+      boolean handleNulls) {
+    return describeMatch(tree);
+  }
+}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

@@ -39,6 +39,7 @@
 import com.google.errorprone.bugpatterns.BadImport;
 import com.google.errorprone.bugpatterns.BadInstanceof;
 import com.google.errorprone.bugpatterns.BadShiftAmount;
+import com.google.errorprone.bugpatterns.BigDecimalEquals;
 import com.google.errorprone.bugpatterns.BigDecimalLiteralDouble;
 import com.google.errorprone.bugpatterns.BooleanParameter;
 import com.google.errorprone.bugpatterns.BoxedPrimitiveConstructor;
@@ -517,6 +518,7 @@ public static ScannerSupplier errorChecks() {
           BadComparable.class,
           BadImport.class,
           BadInstanceof.class,
+          BigDecimalEquals.class,
           BigDecimalLiteralDouble.class,
           BoxedPrimitiveConstructor.class,
           ByteBufferBackingArray.class,

core/src/test/java/com/google/errorprone/bugpatterns/BigDecimalEqualsTest.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2018 The Error Prone Authors.
+ *
+ * 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.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for {@link BigDecimalEquals}.
+ *
+ * @author ghm@google.com (Graeme Morgan)
+ */
+@RunWith(JUnit4.class)
+public final class BigDecimalEqualsTest {
+  @Test
+  public void positive() {
+    CompilationTestHelper.newInstance(BigDecimalEquals.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "import com.google.common.base.Objects;",
+            "import java.math.BigDecimal;",
+            "class Test {",
+            "  void test(BigDecimal a, BigDecimal b) {",
+            "    // BUG: Diagnostic contains:",
+            "    boolean foo = a.equals(b);",
+            "    // BUG: Diagnostic contains:",
+            "    boolean bar = !a.equals(b);",
+            "    // BUG: Diagnostic contains:",
+            "    boolean baz = Objects.equal(a, b);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+
+  @Test
+  public void negative() {
+    CompilationTestHelper.newInstance(BigDecimalEquals.class, getClass())
+        .addSourceLines(
+            "Test.java",
+            "import java.math.BigDecimal;",
+            "class Test {",
+            "  BigDecimal a;",
+            "  BigDecimal b;",
+            "  boolean f(String a, String b) {",
+            "    return a.equals(b);",
+            "  }",
+            "  @Override public boolean equals(Object o) {",
+            "    return a.equals(b);",
+            "  }",
+            "}")
+        .doTest();
+  }
+}

docs/bugpattern/BigDecimalEquals.md

@@ -0,0 +1,9 @@
+`BigDecimal`'s equals method compares the scale of the representation as well as
+the numeric value, which may not be expected.
+
+```java {.bad}
+BigDecimal a = new BigDecimal("1.0");
+BigDecimal b = new BigDecimal("1.00");
+a.equals(b); // false!
+```
+