Skip to content

Commit dc1279e

Browse files
graememorganError Prone Team
graememorgan
authored and
Error Prone Team
committed
AssertSameIncompatible: flag calls to assertSame/etc where the calls are guaranteed to either succeed or fail.
Flume: http://unknown commit PiperOrigin-RevId: 836216185
1 parent 24387de commit dc1279e

File tree

3 files changed

+264
-0
lines changed

3 files changed

+264
-0
lines changed

‎core/src/main/java/com/google/errorprone/bugpatterns/collectionincompatibletype/AssertSameIncompatible.java‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Copyright 2025 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns.collectionincompatibletype;
18+
19+
import static com.google.common.collect.Iterables.getOnlyElement;
20+
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
21+
import static com.google.errorprone.matchers.Description.NO_MATCH;
22+
import static com.google.errorprone.matchers.Matchers.instanceMethod;
23+
import static com.google.errorprone.matchers.Matchers.staticMethod;
24+
import static com.google.errorprone.util.ASTHelpers.getReceiver;
25+
import static com.google.errorprone.util.ASTHelpers.getSymbol;
26+
import static com.google.errorprone.util.ASTHelpers.getType;
27+
import static com.google.errorprone.util.Signatures.prettyType;
28+
import static java.lang.String.format;
29+
30+
import com.google.common.collect.ImmutableSet;
31+
import com.google.errorprone.BugPattern;
32+
import com.google.errorprone.VisitorState;
33+
import com.google.errorprone.bugpatterns.BugChecker;
34+
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
35+
import com.google.errorprone.matchers.Description;
36+
import com.google.errorprone.matchers.Matcher;
37+
import com.sun.source.tree.ExpressionTree;
38+
import com.sun.source.tree.MethodInvocationTree;
39+
import com.sun.source.tree.Tree;
40+
import com.sun.tools.javac.code.Type;
41+
42+
/** A BugPattern; see the summary. */
43+
@BugPattern(summary = "The types passed to this assertion are incompatible.", severity = WARNING)
44+
public final class AssertSameIncompatible extends BugChecker
45+
implements MethodInvocationTreeMatcher {
46+
private static final Matcher<ExpressionTree> JUNIT_MATCHER =
47+
staticMethod()
48+
.onClassAny("org.junit.Assert", "junit.framework.Assert", "junit.framework.TestCase")
49+
.namedAnyOf("assertSame", "assertNotSame");
50+
51+
private static final Matcher<ExpressionTree> TRUTH_MATCHER =
52+
instanceMethod()
53+
.onDescendantOf("com.google.common.truth.Subject")
54+
.namedAnyOf("isSameInstanceAs", "isNotSameInstanceAs");
55+
56+
private static final ImmutableSet<String> NEGATIVE_ASSERTIONS =
57+
ImmutableSet.of("assertNotSame", "isNotSameInstanceAs");
58+
59+
@Override
60+
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
61+
if (JUNIT_MATCHER.matches(tree, state)) {
62+
return handleJUnit(tree, state);
63+
}
64+
if (TRUTH_MATCHER.matches(tree, state)) {
65+
return handleTruth(tree, state);
66+
}
67+
return NO_MATCH;
68+
}
69+
70+
private Description handleJUnit(MethodInvocationTree tree, VisitorState state) {
71+
var arguments = tree.getArguments();
72+
// Grab the last and penultimate arguments to deal with the message-taking overloads.
73+
var expected = getType(arguments.reversed().get(1));
74+
var actual = getType(arguments.reversed().get(0));
75+
return handle(
76+
tree,
77+
actual,
78+
expected,
79+
NEGATIVE_ASSERTIONS.contains(getSymbol(tree).getSimpleName().toString()),
80+
state);
81+
}
82+
83+
private Description handleTruth(MethodInvocationTree tree, VisitorState state) {
84+
var expected = getType(tree.getArguments().get(0));
85+
var assertThat = getReceiver(tree);
86+
if ((!(assertThat instanceof MethodInvocationTree mit)
87+
|| !getSymbol(mit).getSimpleName().contentEquals("assertThat")
88+
|| mit.getArguments().size() != 1)) {
89+
return NO_MATCH;
90+
}
91+
var actual = getType(getOnlyElement(mit.getArguments()));
92+
return handle(
93+
tree, actual, expected, getSymbol(tree).getSimpleName().toString().contains("Not"), state);
94+
}
95+
96+
private Description handle(
97+
Tree tree, Type actual, Type expected, boolean inverted, VisitorState state) {
98+
if (compatible(actual, expected, state)) {
99+
return NO_MATCH;
100+
}
101+
return buildDescription(tree)
102+
.setMessage(
103+
format(
104+
"The types passed to this assertion are incompatible (this check will always %s):"
105+
+ " type `%s` is not compatible with `%s`",
106+
inverted ? "pass" : "fail", prettyType(actual), prettyType(expected)))
107+
.build();
108+
}
109+
110+
private static boolean compatible(Type typeA, Type typeB, VisitorState state) {
111+
var types = state.getTypes();
112+
Type erasedA = types.erasure(typeA);
113+
Type erasedB = types.erasure(typeB);
114+
return types.isCastable(erasedA, erasedB) || types.isCastable(erasedB, erasedA);
115+
}
116+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@
494494
import com.google.errorprone.bugpatterns.checkreturnvalue.NoCanIgnoreReturnValueOnClasses;
495495
import com.google.errorprone.bugpatterns.checkreturnvalue.UnnecessarilyUsedValue;
496496
import com.google.errorprone.bugpatterns.checkreturnvalue.UsingJsr305CheckReturnValue;
497+
import com.google.errorprone.bugpatterns.collectionincompatibletype.AssertSameIncompatible;
497498
import com.google.errorprone.bugpatterns.collectionincompatibletype.CollectionIncompatibleType;
498499
import com.google.errorprone.bugpatterns.collectionincompatibletype.CollectionUndefinedEquality;
499500
import com.google.errorprone.bugpatterns.collectionincompatibletype.CompatibleWithMisuse;
@@ -894,6 +895,7 @@ public static ScannerSupplier warningChecks() {
894895
ArrayAsKeyOfSetOrMap.class,
895896
ArrayRecordComponent.class,
896897
AssertEqualsArgumentOrderChecker.class,
898+
AssertSameIncompatible.class,
897899
AssertThrowsMultipleStatements.class,
898900
AssertionFailureIgnored.class,
899901
AssignmentExpression.class,

‎core/src/test/java/com/google/errorprone/bugpatterns/collectionincompatibletype/AssertSameIncompatibleTest.java‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
/*
2+
* Copyright 2025 The Error Prone Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.errorprone.bugpatterns.collectionincompatibletype;
18+
19+
import com.google.errorprone.CompilationTestHelper;
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.junit.runners.JUnit4;
23+
24+
@RunWith(JUnit4.class)
25+
public final class AssertSameIncompatibleTest {
26+
private final CompilationTestHelper helper =
27+
CompilationTestHelper.newInstance(AssertSameIncompatible.class, getClass());
28+
29+
@Test
30+
public void assertSame_compatible() {
31+
helper
32+
.addSourceLines(
33+
"Test.java",
34+
"""
35+
import static org.junit.Assert.assertSame;
36+
37+
class Test {
38+
void f() {
39+
assertSame(new Object(), "foo");
40+
}
41+
}
42+
""")
43+
.doTest();
44+
}
45+
46+
@Test
47+
public void assertSame_generics_compatible() {
48+
helper
49+
.addSourceLines(
50+
"Test.java",
51+
"""
52+
import static org.junit.Assert.assertSame;
53+
import java.util.List;
54+
import java.util.ArrayList;
55+
56+
class Test {
57+
void f(List<Object> list, ArrayList<Integer> arrayList) {
58+
assertSame(list, arrayList);
59+
assertSame(arrayList, list);
60+
}
61+
}
62+
""")
63+
.doTest();
64+
}
65+
66+
@Test
67+
public void assertSame_incompatible() {
68+
helper
69+
.addSourceLines(
70+
"Test.java",
71+
"""
72+
import static org.junit.Assert.assertSame;
73+
74+
class Test {
75+
void f() {
76+
// BUG: Diagnostic contains:
77+
assertSame("foo", 1L);
78+
}
79+
}
80+
""")
81+
.doTest();
82+
}
83+
84+
@Test
85+
public void assertThat_compatible() {
86+
helper
87+
.addSourceLines(
88+
"Test.java",
89+
"""
90+
import static com.google.common.truth.Truth.assertThat;
91+
92+
class Test {
93+
void f() {
94+
assertThat(new Object()).isSameInstanceAs("foo");
95+
assertThat(new Object()).isNotSameInstanceAs("bar");
96+
assertThat("foo").isSameInstanceAs(new Object());
97+
assertThat("foo").isNotSameInstanceAs(new Object());
98+
}
99+
}
100+
""")
101+
.doTest();
102+
}
103+
104+
@Test
105+
public void assertThat_incompatible() {
106+
helper
107+
.addSourceLines(
108+
"Test.java",
109+
"""
110+
import static com.google.common.truth.Truth.assertThat;
111+
112+
class Test {
113+
void f() {
114+
// BUG: Diagnostic contains: always fail
115+
assertThat(1L).isSameInstanceAs("foo");
116+
117+
// BUG: Diagnostic contains: always pass
118+
assertThat(1L).isNotSameInstanceAs("foo");
119+
}
120+
}
121+
""")
122+
.doTest();
123+
}
124+
125+
@Test
126+
public void bothInterfaces_alwaysCompatible() {
127+
helper
128+
.addSourceLines(
129+
"Test.java",
130+
"""
131+
import static org.junit.Assert.assertSame;
132+
133+
class Test {
134+
interface A {}
135+
136+
interface B {}
137+
138+
void f(A a, B b) {
139+
assertSame(a, b);
140+
assertSame(b, a);
141+
}
142+
}
143+
""")
144+
.doTest();
145+
}
146+
}

0 commit comments

Comments
 (0)