Skip to content

Commit eeb92b1

Browse files
graememorganError Prone Team
authored andcommitted
DuplicateAssertion: detect duplicated assertion lines where the argument to assertThat is pure.
This just looks for successive assertion statements with the same source code, and uses our friend `ConstantExpressions` to check whether the argument to `assertThat` seems pure at heart. Flume: unknown commit PiperOrigin-RevId: 821773490
1 parent 82b4f96 commit eeb92b1

File tree

3 files changed

+206
-0
lines changed

3 files changed

+206
-0
lines changed
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
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;
18+
19+
import static com.google.common.collect.Iterables.getOnlyElement;
20+
import static com.google.errorprone.matchers.Description.NO_MATCH;
21+
import static com.google.errorprone.util.ASTHelpers.getReceiver;
22+
import static com.google.errorprone.util.ASTHelpers.getSymbol;
23+
24+
import com.google.common.collect.ImmutableSetMultimap;
25+
import com.google.errorprone.BugPattern;
26+
import com.google.errorprone.BugPattern.SeverityLevel;
27+
import com.google.errorprone.VisitorState;
28+
import com.google.errorprone.bugpatterns.BugChecker.BlockTreeMatcher;
29+
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
30+
import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpression;
31+
import com.google.errorprone.matchers.Description;
32+
import com.sun.source.tree.BlockTree;
33+
import com.sun.source.tree.ExpressionStatementTree;
34+
import com.sun.source.tree.ExpressionTree;
35+
import com.sun.source.tree.MethodInvocationTree;
36+
import javax.inject.Inject;
37+
38+
/** A BugPattern; see the {@code summary}. */
39+
@BugPattern(summary = "This assertion is duplicate.", severity = SeverityLevel.WARNING)
40+
public final class DuplicateAssertion extends BugChecker implements BlockTreeMatcher {
41+
private final ConstantExpressions constantExpressions;
42+
43+
@Inject
44+
DuplicateAssertion(ConstantExpressions constantExpressions) {
45+
this.constantExpressions = constantExpressions;
46+
}
47+
48+
@Override
49+
public Description matchBlock(BlockTree tree, VisitorState state) {
50+
var assertionLines = extractAssertionLines(tree, state);
51+
52+
for (var entry : assertionLines.entries()) {
53+
var source = entry.getKey();
54+
var line = entry.getValue();
55+
if (assertionLines.containsEntry(source, line - 1)) {
56+
state.reportMatch(
57+
buildDescription(tree.getStatements().get(line))
58+
.setMessage("This assertion is duplicated on the line above. Is that a mistake?")
59+
.build());
60+
}
61+
}
62+
63+
return NO_MATCH;
64+
}
65+
66+
private ImmutableSetMultimap<Assertion, Integer> extractAssertionLines(
67+
BlockTree tree, VisitorState state) {
68+
ImmutableSetMultimap.Builder<Assertion, Integer> lines = ImmutableSetMultimap.builder();
69+
for (int i = 0; i < tree.getStatements().size(); i++) {
70+
int finalI = i;
71+
var statement = tree.getStatements().get(i);
72+
if (!(statement instanceof ExpressionStatementTree est)) {
73+
continue;
74+
}
75+
if (!(est.getExpression() instanceof MethodInvocationTree mit)) {
76+
continue;
77+
}
78+
for (ExpressionTree receiver = getReceiver(mit);
79+
receiver != null;
80+
receiver = getReceiver(receiver)) {
81+
var symbol = getSymbol(receiver);
82+
if ((receiver instanceof MethodInvocationTree method)
83+
&& symbol.getSimpleName().contentEquals("assertThat")
84+
&& method.getArguments().size() == 1) {
85+
constantExpressions
86+
.constantExpression(getOnlyElement(method.getArguments()), state)
87+
.ifPresent(
88+
ce -> lines.put(new Assertion(state.getSourceForNode(statement), ce), finalI));
89+
}
90+
}
91+
}
92+
return lines.build();
93+
}
94+
95+
private record Assertion(String line, ConstantExpression assertee) {}
96+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@
122122
import com.google.errorprone.bugpatterns.DoNotMockAutoValue;
123123
import com.google.errorprone.bugpatterns.DoNotMockChecker;
124124
import com.google.errorprone.bugpatterns.DoubleBraceInitialization;
125+
import com.google.errorprone.bugpatterns.DuplicateAssertion;
125126
import com.google.errorprone.bugpatterns.DuplicateBranches;
126127
import com.google.errorprone.bugpatterns.DuplicateDateFormatField;
127128
import com.google.errorprone.bugpatterns.DuplicateMapKeys;
@@ -940,6 +941,7 @@ public static ScannerSupplier warningChecks() {
940941
DoNotClaimAnnotations.class,
941942
DoNotMockAutoValue.class,
942943
DoubleCheckedLocking.class,
944+
DuplicateAssertion.class,
943945
DuplicateBranches.class,
944946
DuplicateDateFormatField.class,
945947
EffectivelyPrivate.class,
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
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;
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 DuplicateAssertionTest {
26+
public final CompilationTestHelper helper =
27+
CompilationTestHelper.newInstance(DuplicateAssertion.class, getClass());
28+
29+
@Test
30+
public void positive() {
31+
helper
32+
.addSourceLines(
33+
"Test.java",
34+
"""
35+
import static com.google.common.truth.Truth.assertThat;
36+
37+
import java.util.List;
38+
39+
class Test {
40+
public void test(List<Integer> list) {
41+
assertThat(list).contains(1);
42+
// BUG: Diagnostic contains:
43+
assertThat(list).contains(1);
44+
}
45+
}
46+
""")
47+
.doTest();
48+
}
49+
50+
@Test
51+
public void negative_notDuplicated() {
52+
helper
53+
.addSourceLines(
54+
"Test.java",
55+
"""
56+
import static com.google.common.truth.Truth.assertThat;
57+
58+
import java.util.List;
59+
60+
class Test {
61+
public void test(List<Integer> list) {
62+
assertThat(list).contains(1);
63+
assertThat(list).contains(2);
64+
}
65+
}
66+
""")
67+
.doTest();
68+
}
69+
70+
@Test
71+
public void negative_notAssertion() {
72+
helper
73+
.addSourceLines(
74+
"Test.java",
75+
"""
76+
class Test {
77+
public void test() {
78+
int x = 1;
79+
x = 2;
80+
x = 2;
81+
}
82+
}
83+
""")
84+
.doTest();
85+
}
86+
87+
@Test
88+
public void negative_impure() {
89+
helper
90+
.addSourceLines(
91+
"Test.java",
92+
"""
93+
import java.util.Iterator;
94+
import static com.google.common.truth.Truth.assertThat;
95+
96+
import java.util.List;
97+
98+
class Test {
99+
public void test(List<Integer> list) {
100+
Iterator<Integer> it = list.iterator();
101+
assertThat(it.next()).isEqualTo(1);
102+
assertThat(it.next()).isEqualTo(1);
103+
}
104+
}
105+
""")
106+
.doTest();
107+
}
108+
}

0 commit comments

Comments
 (0)