Suggestion to restructure the check

PicnicSupermarket · Jan 27, 2023 · ef8f41d · ef8f41d
1 parent d8bffbb
commit ef8f41d
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 18 deletions.
diff --git a/...e-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java b/...e-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReference.java
@@ -4,13 +4,13 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
 import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
 import static com.google.errorprone.matchers.Matchers.allOf;
-import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.Matchers.argument;
 import static com.google.errorprone.matchers.Matchers.argumentCount;
 import static com.google.errorprone.matchers.Matchers.isSameType;
 import static com.google.errorprone.matchers.Matchers.isVariable;
 import static com.google.errorprone.matchers.Matchers.not;
 import static com.google.errorprone.matchers.Matchers.staticMethod;
+import static com.sun.source.tree.Tree.Kind.ASSIGNMENT;
 import static com.sun.source.tree.Tree.Kind.RETURN;
 import static com.sun.source.tree.Tree.Kind.VARIABLE;
 import static java.util.Objects.requireNonNull;
@@ -26,10 +26,12 @@
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.MethodTree;
 import com.sun.source.tree.ReturnTree;
 import com.sun.source.tree.Tree;
+import com.sun.source.tree.Tree.Kind;
 import com.sun.source.tree.VariableTree;
 
 /**
@@ -53,11 +55,6 @@ public final class MockitoMockClassReference extends BugChecker
           argumentCount(1),
           argument(0, allOf(isSameType(Class.class.getName()), not(isVariable()))),
           staticMethod().onClass("org.mockito.Mockito").namedAnyOf("mock", "spy"));
-  // XXX: Replace `var` usage with explicit type instead.
-  private static final Matcher<VariableTree> INCOMPATIBLE_VARIABLE =
-      anyOf(ASTHelpers::hasNoExplicitType, MockitoMockClassReference::hasTypeDifference);
-  private static final Matcher<ReturnTree> INCOMPATIBLE_RETURN =
-      MockitoMockClassReference::hasTypeDifference;
 
   /** Instantiates a new {@link MockitoMockClassReference} instance. */
   public MockitoMockClassReference() {}
@@ -69,24 +66,25 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     }
 
     Tree parent = state.getPath().getParentPath().getLeaf();
-    if (parent.getKind() == VARIABLE
-        && INCOMPATIBLE_VARIABLE.matches((VariableTree) parent, state)) {
-      return Description.NO_MATCH;
-    } else if (parent.getKind() == RETURN
-        && INCOMPATIBLE_RETURN.matches((ReturnTree) parent, state)) {
+    Kind parentKind = parent.getKind();
+    if (parentKind != VARIABLE && parentKind != ASSIGNMENT && parentKind != RETURN) {
       return Description.NO_MATCH;
     }
 
-    return describeMatch(tree, SuggestedFix.delete(Iterables.getOnlyElement(tree.getArguments())));
-  }
-
-  private static boolean hasTypeDifference(VariableTree tree, VisitorState state) {
-    return hasTypeDifference(tree, tree.getInitializer(), state);
+    return isParentIncompatible(parent, state)
+        ? Description.NO_MATCH
+        : describeMatch(tree, SuggestedFix.delete(Iterables.getOnlyElement(tree.getArguments())));
   }
 
-  private static boolean hasTypeDifference(ReturnTree tree, VisitorState state) {
+  private static boolean isParentIncompatible(Tree parent, VisitorState state) {
+    if (parent instanceof VariableTree) {
+      return ASTHelpers.hasNoExplicitType((VariableTree) parent, state)
+          || hasTypeDifference(parent, ((VariableTree) parent).getInitializer(), state);
+    } else if (parent instanceof AssignmentTree) {
+      return hasTypeDifference(parent, ((AssignmentTree) parent).getExpression(), state);
+    }
     Tree returnTypeTree = requireNonNull(state.findEnclosing(MethodTree.class)).getReturnType();
-    return hasTypeDifference(returnTypeTree, tree.getExpression(), state);
+    return hasTypeDifference(returnTypeTree, ((ReturnTree) parent).getExpression(), state);
   }
 
   private static boolean hasTypeDifference(Tree treeA, Tree treeB, VisitorState state) {

diff --git a/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReferenceTest.java b/...ntrib/src/test/java/tech/picnic/errorprone/bugpatterns/MockitoMockClassReferenceTest.java
@@ -15,6 +15,8 @@ void identification() {
             "import static org.mockito.Mockito.spy;",
             "",
             "import java.util.List;",
+            "import java.util.Objects;",
+            "import org.mockito.invocation.InvocationOnMock;",
             "",
             "class A {",
             "  // BUG: Diagnostic contains:",
@@ -45,6 +47,11 @@ void identification() {
             "    var nonExplicitlyTypedSpy = spy(Integer.class);",
             "    Number subtypeSpy = spy(Integer.class);",
             "    Object objectSpy = spy(new Object());",
+            "",
+            "    Objects.hash(mock(Integer.class));",
+            "    Integer i = mock(mock(Integer.class));",
+            "    Integer i2 = mock(Integer.class, InvocationOnMock::callRealMethod);",
+            "    String s = new String(mock(String.class));",
             "  }",
             "",
             "  Integer getIntegerMock() {",