FormatStringShouldUsePlaceholders shouldn't rewrite calls with a pass-through varargs Object[] argument

cushon · Error Prone Team · commit 10f54242b4a6 · 2025-11-20T08:37:31.000-08:00
PiperOrigin-RevId: 834775942
diff --git a/check_api/src/main/java/com/google/errorprone/suppliers/Suppliers.java b/check_api/src/main/java/com/google/errorprone/suppliers/Suppliers.java
@@ -266,6 +266,15 @@ public Type get(VisitorState state) {
     };
   }
 
+  public static final Supplier<Type> OBJECT_TYPE_ARRAY =
+      arrayOf(
+          new Supplier<Type>() {
+            @Override
+            public Type get(VisitorState state) {
+              return state.getSymtab().objectType;
+            }
+          });
+
   public static ImmutableList<Supplier<Type>> fromStrings(Iterable<String> types) {
     return ImmutableList.copyOf(
         Iterables.transform(
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/DirectInvocationOnMock.java b/core/src/main/java/com/google/errorprone/bugpatterns/DirectInvocationOnMock.java
@@ -26,8 +26,7 @@
 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.suppliers.Suppliers.OBJECT_TYPE_ARRAY;
 import static com.google.errorprone.util.ASTHelpers.getReceiver;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.MoreAnnotations.getAnnotationValue;
@@ -179,7 +178,7 @@ public Void visitAssignment(AssignmentTree tree, Void unused) {
           staticMethod()
               .onClass("org.mockito.Mockito")
               .named("mock")
-              .withParametersOfType(ImmutableList.of(arrayOf(OBJECT_TYPE))));
+              .withParametersOfType(ImmutableList.of(OBJECT_TYPE_ARRAY)));
 
   private static final Matcher<MethodInvocationTree> DO_CALL_REAL_METHOD =
       anyOf(
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/DistinctVarargsChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/DistinctVarargsChecker.java
@@ -20,6 +20,7 @@
 import static com.google.errorprone.matchers.Matchers.anyOf;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
 import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE;
+import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE_ARRAY;
 import static com.google.errorprone.suppliers.Suppliers.arrayOf;
 import static com.google.errorprone.suppliers.Suppliers.typeFromString;
 
@@ -63,7 +64,7 @@ public final class DistinctVarargsChecker extends BugChecker
           staticMethod()
               .onClass("com.google.common.collect.Ordering")
               .named("explicit")
-              .withParametersOfType(ImmutableList.of(OBJECT_TYPE, arrayOf(OBJECT_TYPE))));
+              .withParametersOfType(ImmutableList.of(OBJECT_TYPE, OBJECT_TYPE_ARRAY)));
   private static final Matcher<ExpressionTree> EVEN_PARITY_DISTINCT_ARG_MATCHER =
       anyOf(
           // ImmutableMap.of is covered by AlwaysThrows.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/NullNeedsCastForVarargs.java b/core/src/main/java/com/google/errorprone/bugpatterns/NullNeedsCastForVarargs.java
@@ -24,7 +24,7 @@
 import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
 import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE;
-import static com.google.errorprone.suppliers.Suppliers.arrayOf;
+import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE_ARRAY;
 import static com.google.errorprone.util.ASTHelpers.getType;
 import static com.google.errorprone.util.TargetType.targetType;
 import static com.sun.source.tree.Tree.Kind.NULL_LITERAL;
@@ -137,54 +137,54 @@ private static Type targetTypElementType(VisitorState state) {
    * A single parameter whose erasure is type {@code Object[]}. This includes {@code Object...} and
    * (when {@code <T>} is an unbounded type parameter) {@code T[]}.
    */
-  private static final ImmutableList<Supplier<Type>> JUST_OBJECT_ARRAY =
-      ImmutableList.of(arrayOf(OBJECT_TYPE));
+  private static final ImmutableList<Supplier<Type>> JUST_OBJECT_TYPE_ARRAY =
+      ImmutableList.of(OBJECT_TYPE_ARRAY);
 
   private static final Matcher<ExpressionTree> METHOD_WITH_SOLE_PARAMETER_OBJECT_VARARGS =
       instanceMethod()
           .onDescendantOf("com.google.common.truth.Subject")
           .named("containsExactly")
-          .withParametersOfType(JUST_OBJECT_ARRAY);
+          .withParametersOfType(JUST_OBJECT_TYPE_ARRAY);
 
   private static final Matcher<ExpressionTree> METHOD_WITH_SECOND_PARAMETER_OBJECT_VARARGS =
       instanceMethod()
           .onDescendantOf("com.google.common.reflect.Invokable")
           .named("invoke")
-          .withParametersOfType(ImmutableList.of(OBJECT_TYPE, arrayOf(OBJECT_TYPE)));
+          .withParametersOfType(ImmutableList.of(OBJECT_TYPE, OBJECT_TYPE_ARRAY));
 
   // TODO: b/429160687 - Also cover the methods in UsingCorrespondence.
 
   private static final Matcher<ExpressionTree> METHOD_WITH_THIRD_PARAMETER_OBJECT_VARARGS =
       instanceMethod()
           .onDescendantOf("com.google.common.truth.Subject")
           .namedAnyOf("containsAnyOf", "containsAtLeast", "containsNoneOf", "isAnyOf", "isNoneOf")
-          .withParametersOfType(ImmutableList.of(OBJECT_TYPE, OBJECT_TYPE, arrayOf(OBJECT_TYPE)));
+          .withParametersOfType(ImmutableList.of(OBJECT_TYPE, OBJECT_TYPE, OBJECT_TYPE_ARRAY));
 
   private static final Matcher<ExpressionTree> METHOD_WITH_SOLE_PARAMETER_GENERIC_VARARGS =
       anyOf(
           staticMethod()
               .onClass("com.google.common.collect.AndroidAccessToCompactDataStructures")
               .namedAnyOf("newCompactHashSet", "newCompactLinkedHashSet")
-              .withParametersOfType(JUST_OBJECT_ARRAY),
+              .withParametersOfType(JUST_OBJECT_TYPE_ARRAY),
           staticMethod()
               .onClassAny(
                   "com.google.common.collect.CompactHashSet",
                   "com.google.common.collect.CompactLinkedHashSet")
               .named("create")
-              .withParametersOfType(JUST_OBJECT_ARRAY),
+              .withParametersOfType(JUST_OBJECT_TYPE_ARRAY),
           staticMethod()
               .onClassAny(
                   "com.google.common.collect.Iterables", "com.google.common.collect.Iterators")
               .namedAnyOf("cycle", "forArray")
-              .withParametersOfType(JUST_OBJECT_ARRAY),
+              .withParametersOfType(JUST_OBJECT_TYPE_ARRAY),
           staticMethod()
               .onClass("com.google.common.collect.Lists")
               .namedAnyOf("newArrayList", "newLinkedList")
-              .withParametersOfType(JUST_OBJECT_ARRAY),
+              .withParametersOfType(JUST_OBJECT_TYPE_ARRAY),
           staticMethod()
               .onClass("com.google.common.collect.Sets")
               .namedAnyOf("newHashSet", "newLinkedHashSet")
-              .withParametersOfType(JUST_OBJECT_ARRAY),
+              .withParametersOfType(JUST_OBJECT_TYPE_ARRAY),
           staticMethod().onClass("java.util.Arrays").named("asList"),
           staticMethod()
               .onClass("java.util.stream.Stream")
@@ -196,5 +196,5 @@ private static Type targetTypElementType(VisitorState state) {
                * (Similarly, we may or may not need to call `withParametersOfType` for the other
                * cases above in which overloads exist, but we do anyway.)
                */
-              .withParametersOfType(JUST_OBJECT_ARRAY));
+              .withParametersOfType(JUST_OBJECT_TYPE_ARRAY));
 }
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/formatstring/FormatStringShouldUsePlaceholders.java b/core/src/main/java/com/google/errorprone/bugpatterns/formatstring/FormatStringShouldUsePlaceholders.java
@@ -17,7 +17,10 @@
 package com.google.errorprone.bugpatterns.formatstring;
 
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.suppliers.Suppliers.OBJECT_TYPE_ARRAY;
 import static com.google.errorprone.util.ASTHelpers.getStartPosition;
+import static com.google.errorprone.util.ASTHelpers.getType;
+import static com.google.errorprone.util.ASTHelpers.isSameType;
 
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugPattern;
@@ -88,6 +91,10 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     if (arguments.isEmpty()) {
       return Description.NO_MATCH;
     }
+    if (arguments.size() == 2
+        && isSameType(getType(arguments.get(1)), OBJECT_TYPE_ARRAY.get(state), state)) {
+      return Description.NO_MATCH;
+    }
     ExpressionTree formatString = arguments.getFirst();
 
     // If the value is a compile-time constant, either it has no concatenations or the
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/formatstring/FormatStringShouldUsePlaceholdersTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/formatstring/FormatStringShouldUsePlaceholdersTest.java
@@ -143,6 +143,30 @@ public void checkPositionIndex(int i, int j) {
     Preconditions.checkPositionIndex(i, j + 10);
   }
 }
+""")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void negativeVarargs() {
+    refactoringHelper
+        .addInputLines(
+            "Test.java",
+"""
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Collections;
+
+public class Test {
+  public void checkPositionIndex(int i, String moreFormat, Object moreArgs) {
+    List<Object> args = new ArrayList<>();
+    args.add(i);
+    Collections.addAll(args, moreArgs);
+    Preconditions.checkArgument(false, String.format("%s: %s" + moreFormat, args.toArray()));
+  }
+}
 """)
         .expectUnchanged()
         .doTest();
