Skip to content

Commit 62086b7

Browse files
graememorganError Prone Team
authored and
Error Prone Team
committed
Handle multiple arguments in thenThrow.
PiperOrigin-RevId: 738343494
1 parent 7440ff1 commit 62086b7

File tree

2 files changed

+79
-37
lines changed

2 files changed

+79
-37
lines changed

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

+40-37
Original file line numberDiff line numberDiff line change
@@ -52,43 +52,46 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
5252
if (!THEN_THROW.matches(tree, state)) {
5353
return NO_MATCH;
5454
}
55-
ExpressionTree exceptionTree = tree.getArguments().get(0);
56-
var thrownType = getType(exceptionTree);
57-
if (!isCheckedExceptionType(thrownType, state)) {
58-
return NO_MATCH;
59-
}
60-
// Heuristic: if the type being thrown is Exception/Throwable, but doesn't come directly from a
61-
// constructor, it might be a parameter, and we can't know that it's not always given sensible
62-
// types.
63-
if (!(exceptionTree instanceof NewClassTree)
64-
&& (isSameType(thrownType, state.getSymtab().exceptionType, state)
65-
|| isSameType(thrownType, state.getSymtab().throwableType, state))) {
66-
return NO_MATCH;
67-
}
68-
for (var receiver = getReceiver(tree);
69-
receiver instanceof MethodInvocationTree whenMit;
70-
receiver = getReceiver(receiver)) {
71-
if (WHEN.matches(receiver, state)
72-
&& whenMit.getArguments().get(0) instanceof MethodInvocationTree mit
73-
&& getType(mit.getMethodSelect()).getThrownTypes().stream()
74-
.noneMatch(
75-
throwableType -> state.getTypes().isAssignable(thrownType, throwableType))) {
76-
var thrownTypes = getType(mit.getMethodSelect()).getThrownTypes();
77-
return buildDescription(whenMit.getArguments().get(0))
78-
.setMessage(
79-
thrownTypes.isEmpty()
80-
? format(
81-
"%s is not throwable by this method; only unchecked exceptions can be"
82-
+ " thrown.",
83-
thrownType.tsym.getSimpleName())
84-
: format(
85-
"%s is not throwable by this method; possible exception types are %s, or"
86-
+ " any unchecked exception.",
87-
thrownType.tsym.getSimpleName(),
88-
thrownTypes.stream()
89-
.map(t -> t.tsym.getSimpleName().toString())
90-
.collect(joining(", "))))
91-
.build();
55+
for (ExpressionTree exceptionTree : tree.getArguments()) {
56+
var thrownType = getType(exceptionTree);
57+
if (!isCheckedExceptionType(thrownType, state)) {
58+
continue;
59+
}
60+
// Heuristic: if the type being thrown is Exception/Throwable, but doesn't come directly from
61+
// a constructor, it might be a parameter, and we can't know that it's not always given
62+
// sensible types.
63+
if (!(exceptionTree instanceof NewClassTree)
64+
&& (isSameType(thrownType, state.getSymtab().exceptionType, state)
65+
|| isSameType(thrownType, state.getSymtab().throwableType, state))) {
66+
continue;
67+
}
68+
for (var receiver = getReceiver(tree);
69+
receiver instanceof MethodInvocationTree whenMit;
70+
receiver = getReceiver(receiver)) {
71+
if (WHEN.matches(receiver, state)
72+
&& whenMit.getArguments().get(0) instanceof MethodInvocationTree mit
73+
&& getType(mit.getMethodSelect()).getThrownTypes().stream()
74+
.noneMatch(
75+
throwableType -> state.getTypes().isAssignable(thrownType, throwableType))) {
76+
var thrownTypes = getType(mit.getMethodSelect()).getThrownTypes();
77+
state.reportMatch(
78+
buildDescription(whenMit.getArguments().get(0))
79+
.setMessage(
80+
thrownTypes.isEmpty()
81+
? format(
82+
"%s is not throwable by this method; only unchecked exceptions can be"
83+
+ " thrown.",
84+
thrownType.tsym.getSimpleName())
85+
: format(
86+
"%s is not throwable by this method; possible exception types are %s,"
87+
+ " or any unchecked exception.",
88+
thrownType.tsym.getSimpleName(),
89+
thrownTypes.stream()
90+
.map(t -> t.tsym.getSimpleName().toString())
91+
.collect(joining(", "))))
92+
.build());
93+
break;
94+
}
9295
}
9396
}
9497
return NO_MATCH;

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

+39
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,26 @@ void test(Test t) {
4646
.doTest();
4747
}
4848

49+
@Test
50+
public void positive_multipleThrows() {
51+
compilationHelper
52+
.addSourceLines(
53+
"Test.java",
54+
"""
55+
import static org.mockito.Mockito.when;
56+
57+
abstract class Test {
58+
abstract Object foo();
59+
60+
void test(Test t) {
61+
// BUG: Diagnostic contains: only unchecked
62+
when(t.foo()).thenThrow(new IllegalStateException(), new Exception());
63+
}
64+
}
65+
""")
66+
.doTest();
67+
}
68+
4969
@Test
5070
public void positiveWithSpecificType() {
5171
compilationHelper
@@ -108,6 +128,25 @@ void test(Test t) throws Exception {
108128
.doTest();
109129
}
110130

131+
@Test
132+
public void nothingThrown_noFinding() {
133+
compilationHelper
134+
.addSourceLines(
135+
"Test.java",
136+
"""
137+
import static org.mockito.Mockito.when;
138+
139+
abstract class Test {
140+
abstract Object foo() throws Exception;
141+
142+
void test(Test t) throws Exception {
143+
when(t.foo()).thenThrow();
144+
}
145+
}
146+
""")
147+
.doTest();
148+
}
149+
111150
@Test
112151
public void genericException() {
113152
compilationHelper

0 commit comments

Comments
 (0)