From e1d9301d8719383439891ce8f3e8bd6603bbfb59 Mon Sep 17 00:00:00 2001 From: ghm Date: Tue, 6 Dec 2022 14:48:35 -0800 Subject: [PATCH] Audit EP checks for argumentless mock(). DoNotMock is the only one that uses the arg, so I added a test and handled it. DirectInvocationOnMockTest was sorta implicitly using it, in that it only wanted the one-arg version to exclude stubs with arguments. PiperOrigin-RevId: 493418603 --- .../bugpatterns/AbstractMockChecker.java | 7 +++++-- .../bugpatterns/DirectInvocationOnMock.java | 14 +++++++++++++- .../bugpatterns/MockNotUsedInProduction.java | 7 +------ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMockChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMockChecker.java index d7a827470f58..3cd390b603ce 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMockChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/AbstractMockChecker.java @@ -168,10 +168,13 @@ public static TypeExtractor extractType(Matcher m) { public static TypeExtractor extractFirstArg( Matcher m) { return (tree, state) -> { - if (m.matches(tree, state)) { + if (!m.matches(tree, state)) { + return Optional.empty(); + } + if (tree.getArguments().size() >= 1) { return Optional.ofNullable(ASTHelpers.getType(tree.getArguments().get(0))); } - return Optional.empty(); + return Optional.ofNullable(ASTHelpers.targetType(state)).map(t -> t.type()); }; } diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/DirectInvocationOnMock.java b/core/src/main/java/com/google/errorprone/bugpatterns/DirectInvocationOnMock.java index b8c51c412068..a19b49e13675 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/DirectInvocationOnMock.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/DirectInvocationOnMock.java @@ -22,14 +22,18 @@ import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.matchers.Matchers.allOf; import static com.google.errorprone.matchers.Matchers.anyMethod; +import static com.google.errorprone.matchers.Matchers.anyOf; import static com.google.errorprone.matchers.Matchers.instanceMethod; import static com.google.errorprone.matchers.Matchers.receiverOfInvocation; import static com.google.errorprone.matchers.Matchers.staticMethod; +import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE; +import static com.google.errorprone.suppliers.Suppliers.arrayOf; import static com.google.errorprone.util.ASTHelpers.getReceiver; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.MoreAnnotations.getAnnotationValue; import static java.lang.String.format; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; import com.google.errorprone.VisitorState; @@ -167,7 +171,15 @@ public Void visitAssignment(AssignmentTree tree, Void unused) { } private static final Matcher MOCK = - staticMethod().onClass("org.mockito.Mockito").named("mock").withParameters("java.lang.Class"); + anyOf( + staticMethod() + .onClass("org.mockito.Mockito") + .named("mock") + .withParameters("java.lang.Class"), + staticMethod() + .onClass("org.mockito.Mockito") + .named("mock") + .withParametersOfType(ImmutableList.of(arrayOf(OBJECT_TYPE)))); private static final Matcher DO_CALL_REAL_METHOD = allOf( diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/MockNotUsedInProduction.java b/core/src/main/java/com/google/errorprone/bugpatterns/MockNotUsedInProduction.java index fab61e95b5ce..4b1a8c54fc6e 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/MockNotUsedInProduction.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/MockNotUsedInProduction.java @@ -180,12 +180,7 @@ private boolean annotatedAtMostMock(Symbol symbol) { } private static final Matcher MOCK = - anyOf( - staticMethod() - .onClass("org.mockito.Mockito") - .namedAnyOf("mock") - .withParameters("java.lang.Class"), - staticMethod().onClass("org.mockito.Mockito").namedAnyOf("spy")); + staticMethod().onClass("org.mockito.Mockito").namedAnyOf("mock", "spy"); private static final Matcher MOCK_OR_SPY_ANNOTATED = anyOf(hasAnnotation("org.mockito.Mock"), hasAnnotation("org.mockito.Spy"));