Skip to content

Match on more types in ReferenceEquality #5821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
copybara-service merged 1 commit into from
May 26, 2026
Merged

Match on more types in ReferenceEquality #5821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 0 additions & 30 deletions core/src/main/java/com/google/errorprone/bugpatterns/ReferenceEquality.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.StandardTags;
import com.google.errorprone.VisitorState;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Scope;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Symtab;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.Name;

/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
@BugPattern(
Expand Down Expand Up @@ -65,9 +62,6 @@ protected boolean matchArgument(ExpressionTree tree, VisitorState state) {
if (ASTHelpers.isSubtype(type, state.getSymtab().classType, state)) {
return false;
}
if (!implementsEquals(type, state)) {
return false;
}
return true;
}

Expand Down Expand Up @@ -113,28 +107,4 @@ private static boolean overridesMethodOnType(
private static Symbol getOnlyMember(VisitorState state, Type type, String name) {
return getOnlyElement(type.tsym.members().getSymbolsByName(state.getName(name)));
}

/** Check if the method declares or inherits an implementation of .equals() */
public static boolean implementsEquals(Type type, VisitorState state) {
Name equalsName = EQUALS.get(state);
Symbol objectEquals = getOnlyMember(state, state.getSymtab().objectType, "equals");
for (Type sup : state.getTypes().closure(type)) {
if (ASTHelpers.isSameType(sup, state.getSymtab().objectType, state)) {
continue;
}
Scope scope = sup.tsym.members();
if (scope == null) {
continue;
}
for (Symbol sym : scope.getSymbolsByName(equalsName)) {
if (sym.overrides(objectEquals, type.tsym, state.getTypes(), /* checkResult= */ false)) {
return true;
}
}
}
return false;
}

private static final Supplier<Name> EQUALS =
VisitorState.memoize(state -> state.getName("equals"));
}
30 changes: 20 additions & 10 deletions core/src/test/java/com/google/errorprone/bugpatterns/ReferenceEqualityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void something(TestProtoMessage f1, TestProtoMessage f2) {
}

@Test
public void negative_const() {
public void positive_const() {
compilationHelper
.addSourceLines(
"Foo.java",
Expand All @@ -70,10 +70,12 @@ class Test {
public static final Foo CONST = new Foo();

boolean f(Foo a) {
// BUG: Diagnostic contains:
return a == CONST;
}

boolean f(Object o, Foo a) {
// BUG: Diagnostic contains:
return o == a;
}
}
Expand All @@ -82,7 +84,7 @@ boolean f(Object o, Foo a) {
}

@Test
public void negative_extends_equalsObject() {
public void extends_equalsObject() {
compilationHelper
.addSourceLines(
"Sup.java",
Expand All @@ -100,6 +102,7 @@ public boolean equals(Object o) {

class Test extends Sup {
boolean f(Object a, Test b) {
// BUG: Diagnostic contains: a.equals(b)
return a == b;
}
}
Expand Down Expand Up @@ -158,7 +161,7 @@ boolean f(Test a, Test b) {
}

@Test
public void negative_noEquals() {
public void positive_noEquals() {
compilationHelper
.addSourceLines(
"Test.java",
Expand All @@ -167,6 +170,7 @@ public void negative_noEquals() {

class Test {
boolean f(Test a, Test b) {
// BUG: Diagnostic contains: a.equals(b)
return a == b;
}
}
Expand Down Expand Up @@ -363,6 +367,7 @@ interface Sup {

class Test implements Sup {
boolean f(Object a, Test b) {
// BUG: Diagnostic contains: a.equals(b)
return a == b;
}
}
Expand Down Expand Up @@ -434,13 +439,17 @@ public void erroneous() {
compilationHelper
.addSourceLines(
"Test.java",
"import " + MayImplementEquals.class.getCanonicalName() + ";",
"abstract class Test {",
" abstract MayImplementEquals getter();",
" boolean f(MayImplementEquals b) {",
" return getter() == b;",
" }",
"}")
"""
import %s;
abstract class Test {
abstract MayImplementEquals getter();
boolean f(MayImplementEquals b) {
// BUG: Diagnostic contains: getter().equals(b)
return getter() == b;
}
}
"""
.formatted(MayImplementEquals.class.getCanonicalName()))
.withClasspath(MayImplementEquals.class, ReferenceEqualityTest.class)
.doTest();
}
Expand All @@ -463,6 +472,7 @@ boolean g(T t1, T t2) {
}

boolean g(X x1, X x2) {
// BUG: Diagnostic contains:
return x1 == x2;
}
}
Expand Down
Loading