Added support for logsafe Arg arrays in Slf4jLogsafeArgs.

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

@@ -31,7 +31,9 @@
 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;
@@ -43,7 +45,8 @@
         linkType = BugPattern.LinkType.CUSTOM,
         providesFix = BugPattern.ProvidesFix.REQUIRES_HUMAN_ATTENTION,
         severity = SeverityLevel.WARNING,
-        summary = "Allow only com.palantir.logsafe.Arg types as parameter inputs to slf4j log messages.")
+        summary = "Allow only com.palantir.logsafe.Arg types, or vararg arrays, as parameter inputs to slf4j log "
+                + "messages.")
 public final class Slf4jLogsafeArgs extends BugChecker implements MethodInvocationTreeMatcher {
 
     private static final long serialVersionUID = 1L;
@@ -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,26 +73,35 @@ 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);
+        int startArgIndex = MARKER.matches(allArgs.get(0), state) ? 2 : 1;
+        int lastArgIndex = allArgs.size() - 1;
+        ExpressionTree lastArg = allArgs.get(lastArgIndex);
+
+        if (startArgIndex == lastArgIndex && OBJECT_ARRAY.matches(lastArg, state)) {
+            return Description.NO_MATCH;
+        }
+
         boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state);
-        int endArg = lastArgIsThrowable ? lastIndex - 1 : lastIndex;
+        int lastNonThrowableArgIndex = lastArgIsThrowable ? lastArgIndex - 1 : lastArgIndex;
 
+        List<Integer> badArgs = getBadArgIndices(state, allArgs, startArgIndex, lastNonThrowableArgIndex);
+        if (badArgs.isEmpty() || TestCheckUtils.isTestCode(state)) {
+            return Description.NO_MATCH;
+        }
+
+        return buildDescription(tree)
+                .setMessage("slf4j log statement does not use logsafe parameters for arguments " + badArgs)
+                .build();
+    }
+
+    private List<Integer> getBadArgIndices(VisitorState state, List<? extends ExpressionTree> args, int from, int to) {
         ImmutableList.Builder<Integer> badArgsBuilder = ImmutableList.builder();
-        for (int i = startArg; i <= endArg; i++) {
-            if (!ARG.matches(allArgs.get(i), state)) {
+        for (int i = from; i <= to; i++) {
+            if (!ARG.matches(args.get(i), state)) {
                 badArgsBuilder.add(i);
             }
         }
-        List<Integer> badArgs = badArgsBuilder.build();
-        if (badArgs.isEmpty() || TestCheckUtils.isTestCode(state)) {
-            return Description.NO_MATCH;
-        } else {
-            return buildDescription(tree)
-                    .setMessage("slf4j log statement does not use logsafe parameters for arguments " + badArgs)
-                    .build();
-        }
+        return badArgsBuilder.build();
     }
 
     private Optional<Description> checkThrowableArgumentNotWrapped(MethodInvocationTree tree, VisitorState state) {

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());
     }
 

changelog/3.28.0/pr-1411.v2.yml

@@ -0,0 +1,14 @@
+type: improvement
+improvement:
+  description: |-
+    Adds the proper configuration files upon IntelliJ import of a gradle project for checkstyle and copyright.
+
+    This generates the following additional files:
+    - .idea/copyright/profiles_settings.xml
+    - an xml file under .idea/copyright/ per copyright file under .baseline/copyright
+    - .idea/checkstyle-idea.xml (and adds Checkstyle-IDEA to the external dependencies) if baseline-checkstyle is applied
+    - Either .idea/codeStyleSettings.xml or a .idea/codeStyles/ folder with the contents being copied from .baseline/idea
+      - If .baseline/idea/codeStyles is present, it will copy its contents, otherwise, it will fall back to .baseline/idea/intellij-java-palantir-style.xml as currently
+      - The fallback is using a legacy IntelliJ format and requires closing and reopening the project to be taken into account
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1411

changelog/@unreleased/pr-1394.v2.yml

@@ -0,0 +1,6 @@
+type: improvement
+improvement:
+  description: `Slf4jLogsafeArgs` now allows object arrays be passed as vararg
+    argument to logging methods.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/1394

gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineIdea.groovy

@@ -16,25 +16,27 @@
 
 package com.palantir.baseline.plugins
 
+import com.google.common.collect.ImmutableMap
 import com.palantir.baseline.util.GitUtils
 import groovy.transform.CompileStatic
 import groovy.xml.XmlUtil
+import java.nio.file.Files
+import java.nio.file.Path
+import java.nio.file.Paths
+import java.util.function.Supplier
 import org.gradle.api.Action
 import org.gradle.api.Project
 import org.gradle.api.Task
 import org.gradle.api.file.FileTreeElement
 import org.gradle.api.specs.Spec
+import org.gradle.api.tasks.util.PatternFilterable
 import org.gradle.plugins.ide.idea.GenerateIdeaModule
 import org.gradle.plugins.ide.idea.GenerateIdeaProject
 import org.gradle.plugins.ide.idea.GenerateIdeaWorkspace
 import org.gradle.plugins.ide.idea.IdeaPlugin
 import org.gradle.plugins.ide.idea.model.IdeaModel
 import org.gradle.plugins.ide.idea.model.ModuleDependency
 
-import java.nio.file.Files
-import java.nio.file.Path
-import java.nio.file.Paths
-
 class BaselineIdea extends AbstractBaselinePlugin {
 
     static SAVE_ACTIONS_PLUGIN_MINIMUM_VERSION = '1.9.0'
@@ -92,6 +94,7 @@ class BaselineIdea extends AbstractBaselinePlugin {
             addGitHubIssueNavigation(node)
             ignoreCommonShadedPackages(node)
         }
+        configureProjectForIntellijImport(project)
 
         project.afterEvaluate {
             ideaRootModel.workspace.iws.withXml { provider ->
@@ -119,6 +122,15 @@ class BaselineIdea extends AbstractBaselinePlugin {
         { FileTreeElement details -> details.file == file } as Spec<FileTreeElement>
     }
 
+    private void configureProjectForIntellijImport(Project project) {
+        if (!Boolean.getBoolean("idea.active")) {
+            return
+        }
+        addCodeStyleIntellijImport()
+        addCheckstyleIntellijImport(project)
+        addCopyrightIntellijImport()
+    }
+
     /**
      * Extracts IDEA formatting configurations from Baseline directory and adds it to the Idea project XML node.
      */
@@ -127,39 +139,121 @@ class BaselineIdea extends AbstractBaselinePlugin {
         node.append(new XmlParser().parse(ideaStyleFile).component)
     }
 
+    private void addCodeStyleIntellijImport() {
+        def ideaStyleFile = project.file("${configDir}/idea/intellij-java-palantir-style.xml")
+
+        def ideaStyle = new XmlParser().parse(ideaStyleFile)
+                .component
+                .find { it.'@name' == 'ProjectCodeStyleSettingsManager' }
+
+        XmlUtils.createOrUpdateXmlFile(
+                project.file(".idea/codeStyles/codeStyleConfig.xml"),
+                {
+                    def state = GroovyXmlUtils.matchOrCreateChild(it, "state")
+                    def perProjectSettings = GroovyXmlUtils.matchOrCreateChild(
+                            state, "option", [name: 'USE_PER_PROJECT_SETTINGS'])
+                    perProjectSettings.attributes().'value' = "true"
+                },
+                {
+                    new Node(null, "component", ImmutableMap.of("name", "ProjectCodeStyleConfiguration"))
+                })
+
+
+        def ideaStyleSettings = ideaStyle.option.find {it.'@name' == 'PER_PROJECT_SETTINGS'}
+
+        XmlUtils.createOrUpdateXmlFile(
+                project.file(".idea/codeStyles/Project.xml"),
+                {
+                    def codeScheme = GroovyXmlUtils.matchOrCreateChild(it, "code_scheme", [name: 'Project'])
+                    // Just add the default configuration nodes on top of whatever nodes already exist
+                    // We could be better about this, but IDEA will mostly resolve the duplicates here for us
+                    ideaStyleSettings.value.option.forEach {
+                        codeScheme.append(it)
+                    }
+                },
+                {
+                    new Node(null, "component", ImmutableMap.of("name", "ProjectCodeStyleConfiguration"))
+                })
+    }
+
     /**
      * Extracts copyright headers from Baseline directory and adds them to Idea project XML node.
      */
     private void addCopyright(node) {
         def copyrightManager = node.component.find { it.'@name' == 'CopyrightManager' }
         def copyrightDir = Paths.get("${configDir}/copyright/")
-        assert Files.exists(copyrightDir), "${copyrightDir} must exist"
-        def copyrightFiles = project.fileTree(copyrightDir.toFile()).include("*")
-        assert copyrightFiles.iterator().hasNext(), "${copyrightDir} must contain one or more copyright file"
+        def copyrightFiles = getCopyrightFiles(copyrightDir)
         copyrightFiles.each { File file ->
             def fileName = copyrightDir.relativize(file.toPath())
             def copyrightNode = copyrightManager.copyright.find {
                 it.option.find { it.@name == "myName" }?.@value == fileName
             }
             if (copyrightNode == null) {
-                    def copyrightText = XmlUtil.escapeControlCharacters(XmlUtil.escapeXml(file.text.trim()))
-                    copyrightManager.append(new XmlParser().parseText("""
-                        <copyright>
-                            <option name="notice" value="${copyrightText}" />
-                            <option name="keyword" value="Copyright" />
-                            <option name="allowReplaceKeyword" value="" />
-                            <option name="myName" value="${fileName}" />
-                            <option name="myLocal" value="true" />
-                        </copyright>
-                        """.stripIndent()
-                    ))
+                addCopyrightFile(copyrightManager, file, fileName.toString())
             }
         }
 
         def lastFileName = copyrightDir.relativize(copyrightFiles.iterator().toList().sort().last().toPath())
         copyrightManager.@default = lastFileName
     }
 
+    private void addCopyrightIntellijImport() {
+        def copyrightDir = Paths.get("${configDir}/copyright/")
+        def copyrightFiles = getCopyrightFiles(copyrightDir)
+
+        Supplier<Node> copyrightManagerNode = {
+            return new Node(null, "component", ImmutableMap.of("name", "CopyrightManager"))
+        }
+
+        copyrightFiles.each { File file ->
+            def fileName = copyrightDir.relativize(file.toPath()).toString()
+            def extensionIndex = fileName.lastIndexOf(".")
+            if (extensionIndex == -1) {
+                extensionIndex = fileName.length()
+            }
+            def xmlFileName = fileName.substring(0, extensionIndex) + ".xml"
+
+            XmlUtils.createOrUpdateXmlFile(
+                    // Replace the extension by xml for the actual file
+                    project.file(".idea/copyright/" + xmlFileName),
+                    { node ->
+                        addCopyrightFile(node, file, fileName)
+                    },
+                    copyrightManagerNode)
+        }
+
+        def lastFileName = copyrightDir.relativize(copyrightFiles.iterator().toList().sort().last().toPath())
+
+        XmlUtils.createOrUpdateXmlFile(
+                project.file(".idea/copyright/profiles_settings.xml"),
+                { node ->
+                    GroovyXmlUtils.matchOrCreateChild(node, "settings").attributes().'default' = lastFileName
+                },
+                copyrightManagerNode)
+    }
+
+    private PatternFilterable getCopyrightFiles(copyrightDir) {
+        assert Files.exists(copyrightDir), "${copyrightDir} must exist"
+        def copyrightFiles = project.fileTree(copyrightDir.toFile()).include("*")
+        assert copyrightFiles.iterator().hasNext(), "${copyrightDir} must contain one or more copyright file"
+
+        return copyrightFiles
+    }
+
+    private static void addCopyrightFile(node, File file, String fileName) {
+        def copyrightText = XmlUtil.escapeControlCharacters(XmlUtil.escapeXml(file.text.trim()))
+        node.append(new XmlParser().parseText("""
+            <copyright>
+                <option name="notice" value="${copyrightText}" />
+                <option name="keyword" value="Copyright" />
+                <option name="allowReplaceKeyword" value="" />
+                <option name="myName" value="${fileName}" />
+                <option name="myLocal" value="true" />
+            </copyright>
+            """.stripIndent()
+        ))
+    }
+
     private void addEclipseFormat(node) {
         def baselineFormat = project.plugins.findPlugin(BaselineFormat)
         if (baselineFormat == null) {
@@ -196,13 +290,28 @@ class BaselineIdea extends AbstractBaselinePlugin {
     }
 
     private void addCheckstyle(node) {
-        def checkstyle = project.plugins.findPlugin(BaselineCheckstyle)
-        if (checkstyle == null) {
-            project.logger.debug "Baseline: Skipping IDEA checkstyle configuration since baseline-checkstyle not applied"
-            return
+        project.plugins.withType(BaselineCheckstyle) {
+            project.logger.debug "Baseline: Configuring Checkstyle for Idea"
+
+            addCheckstyleNode(node)
+            addCheckstyleExternalDependencies(node)
         }
+    }
+
+    private static void addCheckstyleIntellijImport(project) {
+        project.plugins.withType(BaselineCheckstyle) {
+            project.logger.debug "Baseline: Configuring Checkstyle for Idea"
 
-        project.logger.debug "Baseline: Configuring Checkstyle for Idea"
+            XmlUtils.createOrUpdateXmlFile(
+                    project.file(".idea/checkstyle-idea.xml"),
+                    BaselineIdea.&addCheckstyleNode)
+            XmlUtils.createOrUpdateXmlFile(
+                    project.file(".idea/externalDependencies.xml"),
+                    BaselineIdea.&addCheckstyleExternalDependencies)
+        }
+    }
+
+    private static void addCheckstyleNode(node) {
         def checkstyleFile = "LOCAL_FILE:\$PROJECT_DIR\$/.baseline/checkstyle/checkstyle.xml"
         node.append(new XmlParser().parseText("""
             <component name="CheckStyle-IDEA">
@@ -220,6 +329,9 @@ class BaselineIdea extends AbstractBaselinePlugin {
               </option>
             </component>
             """.stripIndent()))
+    }
+
+    private static void addCheckstyleExternalDependencies(node) {
         def externalDependencies = GroovyXmlUtils.matchOrCreateChild(node, 'component', [name: 'ExternalDependencies'])
         GroovyXmlUtils.matchOrCreateChild(externalDependencies, 'plugin', [id: 'CheckStyle-IDEA'])
     }