TruthIncompatibleType: exhaustiveness check for methods on `Subject…

…`, and include the missing `isIn` and friends.

PiperOrigin-RevId: 564733306

core/src/main/java/com/google/errorprone/bugpatterns/collectionincompatibletype/TruthIncompatibleType.java

@@ -35,6 +35,7 @@
 
 import com.google.common.collect.Streams;
 import com.google.errorprone.BugPattern;
+import com.google.errorprone.ErrorProneFlags;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
@@ -95,6 +96,16 @@ public class TruthIncompatibleType extends BugChecker implements MethodInvocatio
           .namedAnyOf(
               "contains", "containsExactly", "doesNotContain", "containsAnyOf", "containsNoneOf");
 
+  private static final Matcher<ExpressionTree> IS_ANY_OF =
+      instanceMethod()
+          .onDescendantOf("com.google.common.truth.Subject")
+          .namedAnyOf("isAnyOf", "isNoneOf");
+
+  private static final Matcher<ExpressionTree> IS_IN =
+      instanceMethod()
+          .onDescendantOf("com.google.common.truth.Subject")
+          .namedAnyOf("isIn", "isNotIn");
+
   private static final Matcher<ExpressionTree> VECTOR_CONTAINS =
       instanceMethod()
           .onDescendantOfAny(
@@ -144,16 +155,21 @@ public class TruthIncompatibleType extends BugChecker implements MethodInvocatio
       typeFromString("com.google.common.truth.Correspondence");
 
   private final TypeCompatibility typeCompatibility;
+  private final boolean flagMoreCases;
 
   @Inject
-  TruthIncompatibleType(TypeCompatibility typeCompatibility) {
+  TruthIncompatibleType(TypeCompatibility typeCompatibility, ErrorProneFlags flags) {
     this.typeCompatibility = typeCompatibility;
+
+    this.flagMoreCases = flags.getBoolean("TruthIncompatibleType:FlagMoreCases").orElse(true);
   }
 
   @Override
   public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
     Streams.concat(
             matchEquality(tree, state),
+            matchIsAnyOf(tree, state),
+            matchIsIn(tree, state),
             matchVectorContains(tree, state),
             matchArrayContains(tree, state),
             matchScalarContains(tree, state),
@@ -193,6 +209,38 @@ private Stream<Description> matchEquality(MethodInvocationTree tree, VisitorStat
     return checkCompatibility(getOnlyElement(tree.getArguments()), targetType, sourceType, state);
   }
 
+  private Stream<Description> matchIsAnyOf(MethodInvocationTree tree, VisitorState state) {
+    if (!flagMoreCases || !IS_ANY_OF.matches(tree, state)) {
+      return Stream.empty();
+    }
+    ExpressionTree receiver = getReceiver(tree);
+    if (!START_OF_ASSERTION.matches(receiver, state)) {
+      return Stream.empty();
+    }
+    Type targetType =
+        getType(ignoringCasts(getOnlyElement(((MethodInvocationTree) receiver).getArguments())));
+    return matchScalarContains(tree, targetType, state);
+  }
+
+  private Stream<Description> matchIsIn(MethodInvocationTree tree, VisitorState state) {
+    if (!flagMoreCases || !IS_IN.matches(tree, state)) {
+      return Stream.empty();
+    }
+    ExpressionTree receiver = getReceiver(tree);
+    if (!START_OF_ASSERTION.matches(receiver, state)) {
+      return Stream.empty();
+    }
+
+    Type targetType =
+        getType(ignoringCasts(getOnlyElement(((MethodInvocationTree) receiver).getArguments())));
+    Type sourceType =
+        getIterableTypeArg(
+            getType(getOnlyElement(tree.getArguments())),
+            getOnlyElement(tree.getArguments()),
+            state);
+    return checkCompatibility(getOnlyElement(tree.getArguments()), targetType, sourceType, state);
+  }
+
   private Stream<Description> matchVectorContains(MethodInvocationTree tree, VisitorState state) {
     if (!VECTOR_CONTAINS.matches(tree, state)) {
       return Stream.empty();
@@ -241,13 +289,16 @@ private Stream<Description> matchScalarContains(MethodInvocationTree tree, Visit
     if (!START_OF_ASSERTION.matches(receiver, state)) {
       return Stream.empty();
     }
-
-    Tree argument = ignoringCasts(getOnlyElement(((MethodInvocationTree) receiver).getArguments()));
     Type targetType =
         getIterableTypeArg(
             getOnlyElement(getSymbol((MethodInvocationTree) receiver).getParameters()).type,
-            argument,
+            ignoringCasts(getOnlyElement(((MethodInvocationTree) receiver).getArguments())),
             state);
+    return matchScalarContains(tree, targetType, state);
+  }
+
+  private Stream<Description> matchScalarContains(
+      MethodInvocationTree tree, Type targetType, VisitorState state) {
     MethodSymbol methodSymbol = getSymbol(tree);
     return Streams.mapWithIndex(
             tree.getArguments().stream(),

core/src/test/java/com/google/errorprone/bugpatterns/collectionincompatibletype/TruthIncompatibleTypeTest.java

@@ -16,13 +16,24 @@
 
 package com.google.errorprone.bugpatterns.collectionincompatibletype;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.truth.TruthJUnit.assume;
+import static java.lang.String.format;
+import static java.util.Arrays.stream;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.truth.Subject;
 import com.google.errorprone.CompilationTestHelper;
+import com.google.testing.junit.testparameterinjector.TestParameter;
+import com.google.testing.junit.testparameterinjector.TestParameter.TestParameterValuesProvider;
+import com.google.testing.junit.testparameterinjector.TestParameterInjector;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 
 /** {@link TruthIncompatibleType}Test */
-@RunWith(JUnit4.class)
+@RunWith(TestParameterInjector.class)
 public class TruthIncompatibleTypeTest {
 
   private final CompilationTestHelper compilationHelper =
@@ -48,7 +59,7 @@ public void positive() {
   }
 
   @Test
-  public void assume() {
+  public void assumeTypeCheck() {
     compilationHelper
         .addSourceLines(
             "Test.java",
@@ -559,4 +570,56 @@ public void comparingElementsUsingRawCollection_noFinding() {
             "}")
         .doTest();
   }
+
+  @Test
+  public void subjectExhaustiveness(
+      @TestParameter(valuesProvider = SubjectMethods.class) Method method) {
+    // TODO(ghm): isNotSameInstanceAs might be worth flagging, but the check can be even stricter.
+    assume().that(method.getName()).isNoneOf("isSameInstanceAs", "isNotSameInstanceAs");
+
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "import static com.google.common.truth.Truth.assertThat;",
+            "import com.google.common.collect.ImmutableList;",
+            "class Test {",
+            "  public void test(String a, Long b) {",
+            "    // BUG: Diagnostic contains:",
+            getOffensiveLine(method),
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  private static String getOffensiveLine(Method method) {
+    if (stream(method.getParameterTypes()).allMatch(p -> p.equals(Iterable.class))) {
+      return format("    assertThat(a).%s(ImmutableList.of(b));", method.getName());
+    } else if (stream(method.getParameterTypes()).allMatch(p -> p.equals(Object.class))) {
+      return format("    assertThat(a).%s(b);", method.getName());
+    } else if (stream(method.getParameterTypes())
+        .allMatch(p -> p.equals(Object.class) || p.isArray())) {
+      return format("    assertThat(a).%s(b, b, b);", method.getName());
+    } else if (stream(method.getParameterTypes()).allMatch(Class::isArray)) {
+      return format("    assertThat(a).%s(b);", method.getName());
+    } else {
+      throw new AssertionError();
+    }
+  }
+
+  private static final class SubjectMethods implements TestParameterValuesProvider {
+    @Override
+    public ImmutableList<Method> provideValues() {
+      return stream(Subject.class.getDeclaredMethods())
+          .filter(
+              m ->
+                  Modifier.isPublic(m.getModifiers())
+                      && !m.getName().equals("equals")
+                      && m.getParameterCount() > 0
+                      && (stream(m.getParameterTypes()).allMatch(p -> p.equals(Iterable.class))
+                          || stream(m.getParameterTypes())
+                              .allMatch(p -> p.equals(Object.class) || p.isArray())
+                          || stream(m.getParameterTypes()).allMatch(Class::isArray)))
+          .collect(toImmutableList());
+    }
+  }
 }