Suggest this == obj instead of super.equals(obj) when the two are…

… equivalent.

(inspired by Marcono1234's google/guava#6780)

PiperOrigin-RevId: 573931267

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

@@ -0,0 +1,98 @@
+/*
+ * Copyright 2023 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.getOnlyElement;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.ASTHelpers.enclosingClass;
+import static com.google.errorprone.util.ASTHelpers.getSymbol;
+import static com.sun.source.tree.Tree.Kind.BLOCK;
+import static com.sun.source.tree.Tree.Kind.IDENTIFIER;
+import static com.sun.source.tree.Tree.Kind.MEMBER_SELECT;
+import static com.sun.source.tree.Tree.Kind.METHOD;
+import static com.sun.source.tree.Tree.Kind.RETURN;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.BlockTree;
+import com.sun.source.tree.IdentifierTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.tree.MethodInvocationTree;
+
+/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
+@BugPattern(
+    summary = "`super.equals(obj)` is equivalent to `this == obj` here",
+    severity = WARNING,
+    tags = LIKELY_ERROR)
+public class SuperEqualsIsObjectEquals extends BugChecker implements MethodInvocationTreeMatcher {
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    var methodSelect = tree.getMethodSelect();
+    if (methodSelect.getKind() != MEMBER_SELECT) {
+      return NO_MATCH;
+    }
+    var memberSelect = (MemberSelectTree) methodSelect;
+    var expression = memberSelect.getExpression();
+    if (expression.getKind() == IDENTIFIER
+        && ((IdentifierTree) expression).getName().equals(state.getNames()._super)
+        // We can't use a Matcher because onExactClass suffers from b/130658266.
+        && enclosingClass(getSymbol(tree)) == state.getSymtab().objectType.tsym
+        && memberSelect.getIdentifier().equals(state.getNames().equals)
+        /*
+         * We ignore an override that is merely `return super.equals(obj)`. Such an override is less
+         * likely to be a bug because it may exist for the purpose of adding Javadoc.
+         *
+         * TODO(cpovirk): Consider flagging even that if the method does *not* have Javadoc.
+         */
+        && !methodBodyIsOnlyReturnSuperEquals(state)) {
+      /*
+       * There will often be better fixes than this, some of which would change behavior. But let's
+       * at least suggest the simple thing that's always equivalent.
+       */
+      return describeMatch(
+          tree,
+          SuggestedFix.replace(
+              tree, "this == " + state.getSourceForNode(getOnlyElement(tree.getArguments()))));
+    }
+    return NO_MATCH;
+  }
+
+  private static boolean methodBodyIsOnlyReturnSuperEquals(VisitorState state) {
+    var parentPath = state.getPath().getParentPath();
+    if (parentPath.getLeaf().getKind() != RETURN) {
+      return false;
+    }
+    var grandparentPath = parentPath.getParentPath();
+    var grandparent = grandparentPath.getLeaf();
+    if (grandparent.getKind() != BLOCK) {
+      return false;
+    }
+    if (((BlockTree) grandparent).getStatements().size() > 1) {
+      return false;
+    }
+    var greatGrandparent = grandparentPath.getParentPath().getLeaf();
+    if (greatGrandparent.getKind() != METHOD) {
+      return false;
+    }
+    return true;
+  }
+}

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

@@ -351,6 +351,7 @@
 import com.google.errorprone.bugpatterns.StringSplitter;
 import com.google.errorprone.bugpatterns.StronglyTypeByteString;
 import com.google.errorprone.bugpatterns.SubstringOfZero;
+import com.google.errorprone.bugpatterns.SuperEqualsIsObjectEquals;
 import com.google.errorprone.bugpatterns.SuppressWarningsDeprecated;
 import com.google.errorprone.bugpatterns.SuppressWarningsWithoutExplanation;
 import com.google.errorprone.bugpatterns.SwigMemoryLeak;
@@ -1039,6 +1040,7 @@ public static ScannerSupplier warningChecks() {
           StringCharset.class,
           StringSplitter.class,
           Suggester.class,
+          SuperEqualsIsObjectEquals.class,
           SwigMemoryLeak.class,
           SynchronizeOnNonFinalField.class,
           ThreadJoinLoop.class,

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

@@ -0,0 +1,136 @@
+/*
+ * Copyright 2023 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.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** {@link SuperEqualsIsObjectEquals}Test */
+@RunWith(JUnit4.class)
+public class SuperEqualsIsObjectEqualsTest {
+  @Test
+  public void positive() {
+    helper()
+        .addSourceLines(
+            "Foo.java",
+            "class Foo {",
+            "  int i;",
+            "  @Override",
+            "  public boolean equals(Object obj) {",
+            "    if (obj instanceof Foo) {",
+            "      return i == ((Foo) obj).i;",
+            "    }",
+            "    // BUG: Diagnostic contains: ",
+            "    return super.equals(obj);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void positiveOtherSupertypeWithoutEquals() {
+    helper()
+        .addSourceLines(
+            "Foo.java",
+            "class Foo extends Exception {",
+            "  int i;",
+            "  @Override",
+            "  public boolean equals(Object obj) {",
+            "    if (obj instanceof Foo) {",
+            "      return i == ((Foo) obj).i;",
+            "    }",
+            "    // BUG: Diagnostic contains: ",
+            "    return super.equals(obj);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void negativeOtherSupertypeWithEquals() {
+    helper()
+        .addSourceLines(
+            "Foo.java",
+            "import java.util.AbstractSet;",
+            "abstract class Foo extends AbstractSet<String> {",
+            "  int i;",
+            "  @Override",
+            "  public boolean equals(Object obj) {",
+            "    if (obj instanceof Foo) {",
+            "      return i == ((Foo) obj).i;",
+            "    }",
+            "    return super.equals(obj);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void negativePureSuperDelegation() {
+    helper()
+        .addSourceLines(
+            "Foo.java",
+            "class Foo {",
+            "  @Override",
+            "  public boolean equals(Object obj) {",
+            "    return super.equals(obj);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void refactoring() {
+    refactoringHelper()
+        .addInputLines(
+            "Foo.java",
+            "class Foo {",
+            "  int i;",
+            "  @Override",
+            "  public boolean equals(Object obj) {",
+            "    if (obj instanceof Foo) {",
+            "      return i == ((Foo) obj).i;",
+            "    }",
+            "    return super.equals(obj);",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Foo.java",
+            "class Foo {",
+            "  int i;",
+            "  @Override",
+            "  public boolean equals(Object obj) {",
+            "    if (obj instanceof Foo) {",
+            "      return i == ((Foo) obj).i;",
+            "    }",
+            "    return this == obj;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  private CompilationTestHelper helper() {
+    return CompilationTestHelper.newInstance(SuperEqualsIsObjectEquals.class, getClass());
+  }
+
+  private BugCheckerRefactoringTestHelper refactoringHelper() {
+    return BugCheckerRefactoringTestHelper.newInstance(SuperEqualsIsObjectEquals.class, getClass());
+  }
+}