Added support for logsafe Arg arrays in Slf4jLogsafeArgs.

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java

@@ -17,7 +17,6 @@
 package com.palantir.baseline.errorprone;
 
 import com.google.auto.service.AutoService;
-import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugPattern;
 import com.google.errorprone.BugPattern.SeverityLevel;
 import com.google.errorprone.VisitorState;
@@ -31,10 +30,14 @@
 import com.google.errorprone.matchers.method.MethodMatchers;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
+import com.sun.source.tree.Tree;
 import com.sun.source.util.SimpleTreeVisitor;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 @AutoService(BugChecker.class)
 @BugPattern(
@@ -55,6 +58,8 @@ public final class Slf4jLogsafeArgs extends BugChecker implements MethodInvocati
     private static final Matcher<ExpressionTree> THROWABLE = MoreMatchers.isSubtypeOf(Throwable.class);
     private static final Matcher<ExpressionTree> ARG = MoreMatchers.isSubtypeOf("com.palantir.logsafe.Arg");
     private static final Matcher<ExpressionTree> MARKER = MoreMatchers.isSubtypeOf("org.slf4j.Marker");
+    private static final Matcher<Tree> OBJECT_ARRAY = Matchers.isSubtypeOf(
+            s -> s.getType(s.getTypeFromString("java.lang.Object"), true, Collections.emptyList()));
 
     @Override
     public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
@@ -68,24 +73,27 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
         }
 
         List<? extends ExpressionTree> allArgs = tree.getArguments();
-        int lastIndex = allArgs.size() - 1;
-        int startArg = MARKER.matches(allArgs.get(0), state) ? 2 : 1;
-        ExpressionTree lastArg = allArgs.get(lastIndex);
-        boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state);
-        int endArg = lastArgIsThrowable ? lastIndex - 1 : lastIndex;
+        int startArgIndex = MARKER.matches(allArgs.get(0), state) ? 2 : 1;
+        int lastArgIndex = allArgs.size() - 1;
+        ExpressionTree lastArg = allArgs.get(lastArgIndex);
 
-        ImmutableList.Builder<Integer> badArgsBuilder = ImmutableList.builder();
-        for (int i = startArg; i <= endArg; i++) {
-            if (!ARG.matches(allArgs.get(i), state)) {
-                badArgsBuilder.add(i);
-            }
+        if (startArgIndex == lastArgIndex && OBJECT_ARRAY.matches(lastArg, state)) {
+            return Description.NO_MATCH;
         }
-        List<Integer> badArgs = badArgsBuilder.build();
-        if (badArgs.isEmpty() || TestCheckUtils.isTestCode(state)) {
+
+        boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state);
+        int lastNonThrowableArgIndex = lastArgIsThrowable ? lastArgIndex - 1 : lastArgIndex;
+
+        List<Integer> badArgIndices = IntStream.rangeClosed(startArgIndex, lastNonThrowableArgIndex)
+                .filter(i -> !ARG.matches(allArgs.get(i), state))
+                .boxed()
+                .collect(Collectors.toList());
+
+        if (badArgIndices.isEmpty() || TestCheckUtils.isTestCode(state)) {
             return Description.NO_MATCH;
         } else {
             return buildDescription(tree)
-                    .setMessage("slf4j log statement does not use logsafe parameters for arguments " + badArgs)
+                    .setMessage("slf4j log statement does not use logsafe parameters for arguments " + badArgIndices)
                     .build();
         }
     }

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgsTest.java

@@ -47,6 +47,7 @@ private void test(String logArgs, String failingArgs) throws Exception {
                         "    public String getName() { return null; }",
                         "    public void add(Marker reference) {}",
                         "    public boolean remove(Marker reference) { return true; }",
+                        "    @SuppressWarnings(\"deprecation\")",
                         "    public boolean hasChildren() { return false; }",
                         "    public boolean hasReferences() { return false; }",
                         "    public Iterator<Marker> iterator() { return null; }",
@@ -89,6 +90,7 @@ public void testPassingLogsafeArgs() throws Exception {
         LOG_LEVELS.forEach(logLevel -> CompilationTestHelper.newInstance(Slf4jLogsafeArgs.class, getClass())
                 .addSourceLines(
                         "Test.java",
+                        "import com.palantir.logsafe.Arg;",
                         "import com.palantir.logsafe.SafeArg;",
                         "import com.palantir.logsafe.UnsafeArg;",
                         "import org.slf4j.Logger;",
@@ -151,19 +153,35 @@ public void testPassingLogsafeArgs() throws Exception {
                         "        UnsafeArg.of(\"name2\", \"string2\"),",
                         "        new Throwable());",
                         "",
+                        "    // log.<>(String, Arg<T>[])",
+                        "    log." + logLevel + "(\"constant {}\",",
+                        "                         // BUG: Diagnostic contains: varargs method with inexact argument",
+                        "                         new Arg<?>[] { SafeArg.of(\"name\", \"string\") });",
+                        "",
+                        "    // log.<>(String, SafeArg<T>[])",
+                        "    log." + logLevel + "(\"constant {}\",",
+                        "                         // BUG: Diagnostic contains: varargs method with inexact argument",
+                        "                         new SafeArg<?>[] { SafeArg.of(\"name\", \"string\") });",
+                        "",
+                        "    // log.<>(String, (Object[]) SafeArg<T>[])",
+                        "    log." + logLevel + "(\"constant {}\",",
+                        "                         (Object[]) new SafeArg<?>[] { SafeArg.of(\"name\", \"string\") });",
+                        "",
                         "  }",
                         "",
                         "  class DummyMarker implements Marker {",
                         "    public String getName() { return null; }",
                         "    public void add(Marker reference) {}",
                         "    public boolean remove(Marker reference) { return true; }",
+                        "    @SuppressWarnings(\"deprecation\")",
                         "    public boolean hasChildren() { return false; }",
                         "    public boolean hasReferences() { return false; }",
                         "    public Iterator<Marker> iterator() { return null; }",
                         "    public boolean contains(Marker other) { return false; }",
                         "    public boolean contains(String name) { return false; }",
                         "  }",
                         "}")
+                .matchAllDiagnostics()
                 .doTest());
     }