Skip to content

Commit 286d9af

Browse files
cushonError Prone Team
cushon
authored and
Error Prone Team
committed
Internal change
PiperOrigin-RevId: 922059417
1 parent a116aaf commit 286d9af

2 files changed

Lines changed: 38 additions & 24 deletions

File tree

‎core/src/main/java/com/google/errorprone/bugpatterns/AssertThrowsMinimizer.java‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,14 @@
6969
import java.util.stream.Stream;
7070
import javax.inject.Inject;
7171
import javax.lang.model.element.ElementKind;
72+
import org.jspecify.annotations.Nullable;
7273

7374
/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
7475
@BugPattern(summary = "Minimize the amount of logic in assertThrows", severity = WARNING)
7576
public class AssertThrowsMinimizer extends BugChecker implements MethodTreeMatcher {
7677

7778
private static final Matcher<ExpressionTree> MATCHER =
78-
staticMethod().onClass("org.junit.Assert").named("assertThrows");
79+
anyOf(staticMethod().onClass("org.junit.Assert").named("assertThrows"));
7980

8081
private final ConstantExpressions constantExpressions;
8182
private final boolean useVarType;
@@ -119,11 +120,10 @@ private Optional<AssertThrows> matchMethodInvocation(
119120
if (!(tree.getArguments().getLast() instanceof LambdaExpressionTree lambdaExpressionTree)) {
120121
return Optional.empty();
121122
}
122-
Type firstArgumentType = getType(tree.getArguments().get(0));
123-
if (firstArgumentType.getTypeArguments().isEmpty()) {
123+
Type exceptionType = getExceptionType(tree, state);
124+
if (exceptionType == null) {
124125
return Optional.empty();
125126
}
126-
Type exceptionType = firstArgumentType.getTypeArguments().get(0);
127127
MethodInvocationTree runnable;
128128
switch (lambdaExpressionTree.getBody()) {
129129
case BlockTree blockTree -> {
@@ -280,7 +280,7 @@ private boolean needsHoisting(ExpressionTree tree, Type exceptionType, VisitorSt
280280
// constant fields and string concatenation.
281281
return false;
282282
}
283-
if (isCheckedException(exceptionType, state) && !throwsSubtypeOf(tree, exceptionType, state)) {
283+
if (isCheckedExceptionType(exceptionType, state) && !maybeThrows(tree, exceptionType, state)) {
284284
return false;
285285
}
286286
boolean needsHoisting =
@@ -327,18 +327,22 @@ private boolean newClassTreeNeedsHoisting(NewClassTree tree) {
327327
return !tree.getClassBody().getMembers().stream().allMatch(m -> m instanceof MethodTree);
328328
}
329329

330-
private static boolean throwsSubtypeOf(
331-
ExpressionTree tree, Type exceptionType, VisitorState state) {
332-
Types types = state.getTypes();
333-
return types.isSubtype(state.getSymtab().runtimeExceptionType, exceptionType)
334-
|| getThrownExceptions(tree, state).stream()
335-
.anyMatch(t -> isCheckedException(t, state) && types.isSubtype(t, exceptionType));
330+
private static @Nullable Type getExceptionType(MethodInvocationTree tree, VisitorState state) {
331+
Type firstArgumentType = getType(tree.getArguments().get(0));
332+
if (firstArgumentType.getTypeArguments().isEmpty()) {
333+
return null;
334+
}
335+
return firstArgumentType.getTypeArguments().get(0);
336336
}
337337

338-
private static boolean isCheckedException(Type exception, VisitorState state) {
338+
private static boolean maybeThrows(ExpressionTree tree, Type exceptionType, VisitorState state) {
339339
Types types = state.getTypes();
340-
return !types.isSubtype(exception, state.getSymtab().runtimeExceptionType)
341-
&& !types.isSubtype(exception, state.getSymtab().errorType);
340+
if (types.isSubtype(state.getSymtab().runtimeExceptionType, exceptionType)) {
341+
// The exception is Exception or Throwable, assume anything could throw it
342+
return true;
343+
}
344+
return getThrownExceptions(tree, state).stream()
345+
.anyMatch(t -> types.isAssignable(exceptionType, t));
342346
}
343347

344348
private static final Matcher<ExpressionTree> KNOWN_SAFE =

‎core/src/test/java/com/google/errorprone/bugpatterns/AssertThrowsMinimizerTest.java‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ interface Builder {
4141
Builder setBar(Bar bar);
4242
4343
Builder setBar(Supplier<Bar> bar);
44+
4445
Foo build();
4546
}
4647
}
@@ -514,7 +515,7 @@ public static Object getThing() throws Exception {
514515
throw new Exception();
515516
}
516517
517-
public static Object getThingUnchecked() {
518+
public static Object getThingUnchecked() {
518519
throw new RuntimeException();
519520
}
520521
@@ -679,10 +680,11 @@ public void lambda() {
679680
.addInputLines(
680681
"Test.java",
681682
"""
683+
import static org.junit.Assert.assertThrows;
684+
682685
import java.util.ArrayList;
683686
import java.util.List;
684687
import java.util.function.Supplier;
685-
import static org.junit.Assert.assertThrows;
686688
687689
class Test {
688690
void f() {
@@ -706,10 +708,11 @@ public void methodReference() {
706708
.addInputLines(
707709
"Test.java",
708710
"""
711+
import static org.junit.Assert.assertThrows;
712+
709713
import java.util.ArrayList;
710714
import java.util.List;
711715
import java.util.function.Supplier;
712-
import static org.junit.Assert.assertThrows;
713716
714717
class Test {
715718
void f() {
@@ -725,10 +728,11 @@ Bar m() {
725728
.addOutputLines(
726729
"Test.java",
727730
"""
731+
import static org.junit.Assert.assertThrows;
732+
728733
import java.util.ArrayList;
729734
import java.util.List;
730735
import java.util.function.Supplier;
731-
import static org.junit.Assert.assertThrows;
732736
733737
class Test {
734738
void f() {
@@ -797,10 +801,11 @@ public void anonymousClass() {
797801
.addInputLines(
798802
"Test.java",
799803
"""
804+
import static org.junit.Assert.assertThrows;
805+
800806
import java.util.ArrayList;
801807
import java.util.List;
802808
import java.util.function.Supplier;
803-
import static org.junit.Assert.assertThrows;
804809
805810
class Test {
806811
void f() {
@@ -867,10 +872,11 @@ abstract class InstanceBarSupplier extends BarSupplier {
867872
.addOutputLines(
868873
"Test.java",
869874
"""
875+
import static org.junit.Assert.assertThrows;
876+
870877
import java.util.ArrayList;
871878
import java.util.List;
872879
import java.util.function.Supplier;
873-
import static org.junit.Assert.assertThrows;
874880
875881
class Test {
876882
void f() {
@@ -984,10 +990,11 @@ public void varArgs() {
984990
.addInputLines(
985991
"Test.java",
986992
"""
993+
import static org.junit.Assert.assertThrows;
994+
987995
import java.util.ArrayList;
988996
import java.util.List;
989997
import java.util.function.Supplier;
990-
import static org.junit.Assert.assertThrows;
991998
992999
abstract class Test {
9931000
void f() {
@@ -1002,10 +1009,11 @@ void f() {
10021009
.addOutputLines(
10031010
"Test.java",
10041011
"""
1012+
import static org.junit.Assert.assertThrows;
1013+
10051014
import java.util.ArrayList;
10061015
import java.util.List;
10071016
import java.util.function.Supplier;
1008-
import static org.junit.Assert.assertThrows;
10091017
10101018
abstract class Test {
10111019
void f() {
@@ -1028,10 +1036,11 @@ public void cast() {
10281036
.addInputLines(
10291037
"Test.java",
10301038
"""
1039+
import static org.junit.Assert.assertThrows;
1040+
10311041
import java.util.ArrayList;
10321042
import java.util.List;
10331043
import java.util.function.Supplier;
1034-
import static org.junit.Assert.assertThrows;
10351044
10361045
abstract class Test {
10371046
void f(String s, Object o) {
@@ -1055,10 +1064,11 @@ void f(String s, Object o) {
10551064
.addOutputLines(
10561065
"Test.java",
10571066
"""
1067+
import static org.junit.Assert.assertThrows;
1068+
10581069
import java.util.ArrayList;
10591070
import java.util.List;
10601071
import java.util.function.Supplier;
1061-
import static org.junit.Assert.assertThrows;
10621072
10631073
abstract class Test {
10641074
void f(String s, Object o) {

0 commit comments

Comments
 (0)