Add new ReferenceComparison check (#11)

Author: jtraglia

src/main/java/tech/pegasys/tools/epchecks/ReferenceComparison.java

@@ -0,0 +1,106 @@
+/*
+ * Copyright 2022 ConsenSys AG.
+ *
+ * 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 tech.pegasys.tools.epchecks;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.sun.source.tree.Tree.Kind.EQUAL_TO;
+import static com.sun.source.tree.Tree.Kind.IDENTIFIER;
+import static com.sun.source.tree.Tree.Kind.NOT_EQUAL_TO;
+import static com.sun.source.tree.Tree.Kind.NULL_LITERAL;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.BinaryTreeMatcher;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Matchers;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.BinaryTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.Tree;
+import com.sun.tools.javac.code.Type;
+
+/**
+ * This check is very similar to the <a
+ * href=http://errorprone.info/bugpattern/ReferenceEquality>ReferenceEquality</a> check in Error
+ * Prone. But this one identifies <code>Object</code> comparisons for types without an explicitly
+ * declared <code>equals()</code> method.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "ReferenceComparison",
+    summary = "Reference comparison should be value comparison.",
+    severity = SUGGESTION,
+    linkType = BugPattern.LinkType.NONE)
+public class ReferenceComparison extends BugChecker implements BinaryTreeMatcher {
+  private static final Matcher<BinaryTree> REFERENCE_COMPARISON =
+      Matchers.anyOf(Matchers.kindIs(EQUAL_TO), Matchers.kindIs(NOT_EQUAL_TO));
+  private static final Matcher<Tree> NULL = Matchers.kindIs(NULL_LITERAL);
+  private static final Matcher<Tree> ENUM = Matchers.isSubtypeOf("java.lang.Enum");
+  private static final Matcher<Tree> CLASS = Matchers.isSubtypeOf("java.lang.Class");
+
+  @Override
+  public Description matchBinary(BinaryTree tree, VisitorState state) {
+    // We're looking for reference comparisons (== and !=).
+    if (!REFERENCE_COMPARISON.matches(tree, state)) {
+      return NO_MATCH;
+    }
+
+    final Tree left = tree.getLeftOperand();
+    final Tree right = tree.getRightOperand();
+    final Type leftType = ASTHelpers.getType(left);
+    final Type rightType = ASTHelpers.getType(right);
+
+    if (leftType == null || rightType == null) {
+      return NO_MATCH;
+    }
+
+    // When a boxed type is compared to a primitive, it'll be unboxed.
+    if (leftType.isPrimitive() || rightType.isPrimitive()) {
+      return NO_MATCH;
+    }
+
+    // Ignore comparisons with "this" identifier.
+    if (isThis(left) || isThis(right)) {
+      return NO_MATCH;
+    }
+
+    // Ignore null comparisons.
+    if (NULL.matches(left, state) || NULL.matches(right, state)) {
+      return NO_MATCH;
+    }
+
+    // Ignore reference comparisons with enums. Those are a special case.
+    if (ENUM.matches(left, state) || ENUM.matches(right, state)) {
+      return NO_MATCH;
+    }
+
+    // Ignore class comparisons, those are generally fine.
+    if (CLASS.matches(left, state) || CLASS.matches(right, state)) {
+      return NO_MATCH;
+    }
+
+    return describeMatch(tree);
+  }
+
+  private static boolean isThis(Tree tree) {
+    if (tree.getKind().equals(IDENTIFIER)) {
+      final IdentifierTree identifier = (IdentifierTree) tree;
+      return identifier.getName().contentEquals("this");
+    }
+    return false;
+  }
+}

src/test/java/tech/pegasys/tools/epchecks/ReferenceComparisonTest.java

@@ -0,0 +1,37 @@
+/*
+ * Copyright 2022 ConsenSys AG.
+ *
+ * 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 tech.pegasys.tools.epchecks;
+
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class ReferenceComparisonTest {
+
+  private CompilationTestHelper compilationHelper;
+
+  @BeforeEach
+  public void setup() {
+    compilationHelper = CompilationTestHelper.newInstance(ReferenceComparison.class, getClass());
+  }
+
+  @Test
+  public void referenceComparisonPositiveCases() {
+    compilationHelper.addSourceFile("ReferenceComparisonPositiveCases.java").doTest();
+  }
+
+  @Test
+  public void referenceComparisonNegativeCases() {
+    compilationHelper.addSourceFile("ReferenceComparisonNegativeCases.java").doTest();
+  }
+}

src/test/resources/tech/pegasys/tools/epchecks/ReferenceComparisonNegativeCases.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package tech.pegasys.tools.epchecks;
+
+import com.google.common.primitives.Bytes;
+import org.junit.Test;
+
+public class ReferenceComparisonNegativeCases {
+  public boolean comparisonWithEquals(final Bytes a, final Bytes b) {
+    return a.equals(b);
+  }
+
+  public boolean comparisonWithEnums() {
+    return TestEnum.A == TestEnum.B;
+  }
+
+  public boolean comparisonWithThis() {
+    Object o = null;
+    return this == o;
+  }
+
+  public boolean comparisonWithNull(final Bytes a) {
+    return a == null;
+  }
+
+  public boolean comparisonBoxedPrimitiveAndPrimitive(final Integer a) {
+    return a == 27;
+  }
+
+  public boolean comparisonWithGetClass(final Bytes a, final Bytes b) {
+    return a.getClass() == b.getClass();
+  }
+
+  public boolean comparisonWithEnumVariable(final TestEnum a) {
+    return a == TestEnum.B;
+  }
+
+  public boolean comparisonWithEnumVariables(final TestEnum a, final TestEnum b) {
+    return a == b;
+  }
+
+  private enum TestEnum {
+    A,
+    B,
+    C;
+  }
+}

src/test/resources/tech/pegasys/tools/epchecks/ReferenceComparisonPositiveCases.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright ConsenSys AG.
+ *
+ * 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.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package tech.pegasys.tools.epchecks;
+
+import java.util.List;
+
+public class ReferenceComparisonPositiveCases {
+  public void comparisonWithIdentifiers(final List<Integer> a, final List<Integer> b) {
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (a == b) {
+      return;
+    }
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (a != b) {
+      return;
+    }
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (b == a) {
+      return;
+    }
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (b != a) {
+      return;
+    }
+  }
+
+  public boolean comparisonReturn(final List<Integer> a, final List<Integer> b) {
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    return a == b;
+  }
+
+  public boolean comparisonReturnWithOr(final List<Integer> a, final List<Integer> b) {
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    return false || a == b;
+  }
+
+  public void comparisonWithIdentifierAndMethodInvocationr(final List<Integer> b) {
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (List.of(1, 2, 3) == b) {
+      return;
+    }
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (b == List.of(1, 2, 3)) {
+      return;
+    }
+  }
+
+  public void comparisonWithMethodInvocations(final List<Integer> b) {
+    // BUG: Diagnostic contains: Reference comparison should be value comparison
+    if (List.of(1, 2, 3) == List.of(1, 2, 3, 4)) {
+      return;
+    }
+  }
+}