Add matchers for incompatible type matchers

subprojects/errorprone/src/main/java/org/mockito/errorprone/bugpatterns/AbstractMockitoAnyForPrimitiveType.java

@@ -0,0 +1,129 @@
+/*
+ * Copyright (c) 2017 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.errorprone.bugpatterns;
+
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MemberSelectTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
+import com.sun.source.util.TreePath;
+import com.sun.tools.javac.code.Symbol.MethodSymbol;
+import com.sun.tools.javac.code.Symbol.VarSymbol;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Type.ArrayType;
+import java.util.List;
+import javax.lang.model.type.TypeKind;
+
+/** Base for {@link BugChecker}s that detect issues with any() matchers and primitive types. */
+public abstract class AbstractMockitoAnyForPrimitiveType extends BugChecker
+    implements MethodInvocationTreeMatcher {
+
+  protected abstract Matcher<? super MethodInvocationTree> matcher();
+
+  protected abstract String formatMessage(
+      String expectedTypeAsString, Type matcherType, String replacementName);
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (!matcher().matches(tree, state)) {
+      return NO_MATCH;
+    }
+
+    MethodSymbol method = ASTHelpers.getSymbol(tree);
+    Type matcherType = method.getReturnType();
+
+    // It is expected that the call to anyX() is itself the argument to another call which is
+    // the one being mocked, e.g. something like this:
+    //   when(mock.call(..., anyInt(), ...))...
+    TreePath path = state.getPath();
+    Tree parentTree = path.getParentPath().getLeaf();
+    if (!(parentTree instanceof MethodInvocationTree)) {
+      // Ignore calls that are not arguments to another method call.
+      // TODO: Report this as a problem because it makes little sense.
+      // TODO: Support casting.
+      return NO_MATCH;
+    }
+
+    MethodInvocationTree parentCall = (MethodInvocationTree) parentTree;
+    MethodSymbol parentMethod = ASTHelpers.getSymbol(parentCall);
+
+    // Find the index of the argument in the parent call.
+    int argumentIndex = -1;
+    List<? extends ExpressionTree> parentArguments = parentCall.getArguments();
+    for (int i = 0; i < parentArguments.size(); i++) {
+      ExpressionTree argumentTree = parentArguments.get(i);
+      if (argumentTree == tree) {
+        argumentIndex = i;
+        break;
+      }
+    }
+    if (argumentIndex == -1) {
+      throw new IllegalStateException(
+          "Cannot find argument " + tree + " in argument list from " + parentTree);
+    }
+
+    Type parameterType = getParameterType(parentMethod, argumentIndex);
+
+    TypeKind parameterTypeKind = parameterType.getKind();
+    if (parameterTypeKind.isPrimitive() && parameterTypeKind != matcherType.getKind()) {
+      String expectedTypeAsString = parameterType.toString();
+      String replacementName =
+          "any"
+              + Character.toUpperCase(expectedTypeAsString.charAt(0))
+              + expectedTypeAsString.substring(1);
+
+      String message = formatMessage(expectedTypeAsString, matcherType, replacementName);
+
+      SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
+
+      ExpressionTree methodSelect = tree.getMethodSelect();
+      String replacement;
+      if (methodSelect instanceof MemberSelectTree) {
+        MemberSelectTree qualifier = (MemberSelectTree) methodSelect;
+        replacement = state.getSourceForNode(qualifier.getExpression()) + "." + replacementName;
+      } else {
+        replacement = replacementName;
+        String staticImport = method.owner + "." + replacementName;
+        fixBuilder.addStaticImport(staticImport);
+      }
+
+      SuggestedFix fix = fixBuilder.replace(tree, replacement + "()").build();
+
+      return buildDescription(tree).setMessage(message).addFix(fix).build();
+    }
+
+    return NO_MATCH;
+  }
+
+  /**
+   * Get the type of the parameter for a supplied argument.
+   *
+   * @param method the method symbol that is being called.
+   * @param argumentIndex the index of the argument, can be greater than the number of parameters
+   *     for a var arg method.
+   * @return the type of the associated parameter.
+   */
+  private Type getParameterType(MethodSymbol method, int argumentIndex) {
+    List<VarSymbol> parameters = method.getParameters();
+    Type parameterType;
+    int parameterCount = parameters.size();
+    if (argumentIndex >= parameterCount && method.isVarArgs()) {
+      VarSymbol varArgParameter = parameters.get(parameterCount - 1);
+      parameterType = ((ArrayType) varArgParameter.asType()).getComponentType();
+    } else {
+      parameterType = parameters.get(argumentIndex).asType();
+    }
+    return parameterType;
+  }
+}

subprojects/errorprone/src/main/java/org/mockito/errorprone/bugpatterns/MockitoAnyClassWithPrimitiveType.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2017 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.matchers.Matchers;
+import com.google.errorprone.matchers.method.MethodMatchers.MethodNameMatcher;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Type;
+
+/**
+ * Matches on usage of {@code Mockito.argThat(Matcher)} with a matcher that does not extend
+ * ArgumentMatcher.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "MockitoAnyClassWithPrimitiveType",
+    summary = "Matcher inconsistency: use of any(Class) to match a primitive argument",
+    severity = WARNING,
+    linkType = CUSTOM,
+    link = "site.mockito.org/usage/bugpattern/MockitoAnyClassWithPrimitiveType",
+    explanation =
+        "Mockito relies on Java type checking to ensure that parameter matchers are"
+            + " type safe but there are some discrepancies between what the Java type checker"
+            + " allows and what Mockito expects. e.g. Java will allow anyInt() to be used as a"
+            + " matcher for a long parameter because an int can be widened to a long. This"
+            + " checker highlights those incorrect usages and suggests replacements. In Mockito"
+            + " 1.x this was not really an issue because the anyX() methods did not do runtime"
+            + " type checking of the arguments but in Mockito 2.x they do.")
+public class MockitoAnyClassWithPrimitiveType extends AbstractMockitoAnyForPrimitiveType {
+
+  private static final String[] CLASS_NAMES = {
+    "org.mockito.Mockito", "org.mockito.ArgumentMatchers", "org.mockito.Matchers"
+  };
+
+  // Match against the any() or any(Class) methods.
+  private static final MethodNameMatcher GENERIC_ANY =
+      Matchers.staticMethod().onClassAny(CLASS_NAMES).named("any");
+
+  @Override
+  protected Matcher<? super MethodInvocationTree> matcher() {
+    return GENERIC_ANY;
+  }
+
+  @Override
+  protected String formatMessage(
+      String expectedTypeAsString, Type matcherType, String replacementName) {
+    return String.format(
+        "Matcher mismatch: use %s() matcher to match primitive %s arguments",
+        replacementName, expectedTypeAsString);
+  }
+}

subprojects/errorprone/src/main/java/org/mockito/errorprone/bugpatterns/MockitoAnyIncorrectPrimitiveType.java

@@ -0,0 +1,67 @@
+/*
+ * Copyright (c) 2017 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
+import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
+import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+
+import com.google.auto.service.AutoService;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.bugpatterns.BugChecker;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Type;
+import java.util.regex.Pattern;
+
+/**
+ * Matches on usage of {@code Mockito.argThat(Matcher)} with a matcher that does not extend
+ * ArgumentMatcher.
+ */
+@AutoService(BugChecker.class)
+@BugPattern(
+    name = "MockitoAnyIncorrectPrimitiveType",
+    summary = "Matcher mismatch: incorrect use of any() or anyX() to match a primitive argument",
+    severity = ERROR,
+    linkType = CUSTOM,
+    link = "site.mockito.org/usage/bugpattern/MockitoAnyIncorrectPrimitiveType",
+    explanation =
+        "Mockito relies on Java type checking to ensure that parameter matchers are"
+            + " type safe but there are some discrepancies between what the Java type checker"
+            + " allows and what Mockito expects. e.g. Java will allow anyInt() to be used as a"
+            + " matcher for a long parameter because an int can be widened to a long. This"
+            + " checker highlights those incorrect usages and suggests replacements. In Mockito"
+            + " 1.x this was not really an issue because the anyX() methods did not do runtime"
+            + " type checking of the arguments but in Mockito 2.x they do."
+            + " Java will also allow any() to be used within a primitive but any() returns null and"
+            + " the compiler wraps that call in unboxing which leads to a NPE.")
+public class MockitoAnyIncorrectPrimitiveType extends AbstractMockitoAnyForPrimitiveType {
+
+  // Match against any() or any of the any<x>() methods.
+  private static final Pattern METHOD_NAME_PATTERN =
+      Pattern.compile("any(Boolean|Byte|Char|Double|Float|Int|Long|Short)?");
+
+  private static final String[] CLASS_NAMES = {
+    "org.mockito.ArgumentMatchers", "org.mockito.Mockito", "org.mockito.Matchers"
+  };
+
+  private static final Matcher<ExpressionTree> METHOD_MATCHER =
+      staticMethod().onClassAny(CLASS_NAMES).withNameMatching(METHOD_NAME_PATTERN).withParameters();
+
+  @Override
+  protected Matcher<? super MethodInvocationTree> matcher() {
+    return METHOD_MATCHER;
+  }
+
+  @Override
+  protected String formatMessage(
+      String expectedTypeAsString, Type matcherType, String replacementName) {
+    return String.format(
+        "Matcher mismatch: expected matcher for parameter of type '%s',"
+            + " found matcher for parameter of type '%s'",
+        expectedTypeAsString, matcherType);
+  }
+}