From 92fd52063fc7414c6abcab730b3ee4b022566728 Mon Sep 17 00:00:00 2001 From: Andreas Lundblad Date: Sun, 7 Jun 2020 19:02:44 -0700 Subject: [PATCH 01/12] Added support for logsafe Arg arrays in Slf4jLogsafeArgs. --- .../baseline/errorprone/Slf4jLogsafeArgs.java | 23 +++++++++++++++++++ .../errorprone/Slf4jLogsafeArgsTest.java | 18 +++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java index ad7b13310..8d8550478 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java @@ -31,7 +31,10 @@ 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.tree.TypeCastTree; import com.sun.source.util.SimpleTreeVisitor; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.regex.Pattern; @@ -55,6 +58,10 @@ public final class Slf4jLogsafeArgs extends BugChecker implements MethodInvocati private static final Matcher THROWABLE = MoreMatchers.isSubtypeOf(Throwable.class); private static final Matcher ARG = MoreMatchers.isSubtypeOf("com.palantir.logsafe.Arg"); private static final Matcher MARKER = MoreMatchers.isSubtypeOf("org.slf4j.Marker"); + private static final Matcher LOGSAFE_ARG_ARRAY = Matchers.isSubtypeOf( + state -> state.getType(state.getTypeFromString("com.palantir.logsafe.Arg"), true, Collections.emptyList())); + private static final Matcher OBJECT_ARRAY = + Matchers.isSameType(s -> s.getType(s.getTypeFromString("java.lang.Object"), true, Collections.emptyList())); @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { @@ -71,6 +78,22 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState int lastIndex = allArgs.size() - 1; int startArg = MARKER.matches(allArgs.get(0), state) ? 2 : 1; ExpressionTree lastArg = allArgs.get(lastIndex); + + // A single argument... + if (startArg == lastIndex) { + // ...that is an array of Arg + if (LOGSAFE_ARG_ARRAY.matches(lastArg, state)) { + return Description.NO_MATCH; + } + + // ...or an array of Arg cast to Object[] + if (lastArg instanceof TypeCastTree) { + if (Matchers.typeCast(OBJECT_ARRAY, LOGSAFE_ARG_ARRAY).matches((TypeCastTree) lastArg, state)) { + return Description.NO_MATCH; + } + } + } + boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state); int endArg = lastArgIsThrowable ? lastIndex - 1 : lastIndex; diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgsTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgsTest.java index 07260f3e6..a2fca8b66 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgsTest.java +++ b/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 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,12 +153,27 @@ public void testPassingLogsafeArgs() throws Exception { " UnsafeArg.of(\"name2\", \"string2\"),", " new Throwable());", "", + " // log.<>(String, Arg[])", + " log." + logLevel + "(\"constant {}\",", + " // BUG: Diagnostic contains: varargs method with inexact argument", + " new Arg[] { SafeArg.of(\"name\", \"string\") });", + "", + " // log.<>(String, SafeArg[])", + " log." + logLevel + "(\"constant {}\",", + " // BUG: Diagnostic contains: varargs method with inexact argument", + " new SafeArg[] { SafeArg.of(\"name\", \"string\") });", + "", + " // log.<>(String, (Object[]) SafeArg[])", + " 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 iterator() { return null; }", @@ -164,6 +181,7 @@ public void testPassingLogsafeArgs() throws Exception { " public boolean contains(String name) { return false; }", " }", "}") + .matchAllDiagnostics() .doTest()); } From 1bad8278c2530f254d4f781730b73fbadffe4886 Mon Sep 17 00:00:00 2001 From: Andreas Lundblad Date: Sun, 7 Jun 2020 22:39:39 -0700 Subject: [PATCH 02/12] Reduced cyclomatic complexity. --- .../baseline/errorprone/Slf4jLogsafeArgs.java | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java index 8d8550478..e94b66086 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java +++ b/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; @@ -38,6 +37,8 @@ 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( @@ -58,10 +59,11 @@ public final class Slf4jLogsafeArgs extends BugChecker implements MethodInvocati private static final Matcher THROWABLE = MoreMatchers.isSubtypeOf(Throwable.class); private static final Matcher ARG = MoreMatchers.isSubtypeOf("com.palantir.logsafe.Arg"); private static final Matcher MARKER = MoreMatchers.isSubtypeOf("org.slf4j.Marker"); - private static final Matcher LOGSAFE_ARG_ARRAY = Matchers.isSubtypeOf( + private static final Matcher ARG_ARRAY = Matchers.isSubtypeOf( state -> state.getType(state.getTypeFromString("com.palantir.logsafe.Arg"), true, Collections.emptyList())); private static final Matcher OBJECT_ARRAY = Matchers.isSameType(s -> s.getType(s.getTypeFromString("java.lang.Object"), true, Collections.emptyList())); + private static final Matcher ARG_CAST_TO_OBJ_ARRAY = Matchers.typeCast(OBJECT_ARRAY, ARG_ARRAY); @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { @@ -75,44 +77,42 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState } List allArgs = tree.getArguments(); - int lastIndex = allArgs.size() - 1; - int startArg = MARKER.matches(allArgs.get(0), state) ? 2 : 1; - ExpressionTree lastArg = allArgs.get(lastIndex); - - // A single argument... - if (startArg == lastIndex) { - // ...that is an array of Arg - if (LOGSAFE_ARG_ARRAY.matches(lastArg, state)) { - return Description.NO_MATCH; - } + int startArgIndex = MARKER.matches(allArgs.get(0), state) ? 2 : 1; + int lastArgIndex = allArgs.size() - 1; + ExpressionTree lastArg = allArgs.get(lastArgIndex); - // ...or an array of Arg cast to Object[] - if (lastArg instanceof TypeCastTree) { - if (Matchers.typeCast(OBJECT_ARRAY, LOGSAFE_ARG_ARRAY).matches((TypeCastTree) lastArg, state)) { - return Description.NO_MATCH; - } - } + if (startArgIndex == lastArgIndex && isArgArray(state, lastArg)) { + return Description.NO_MATCH; } boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state); - int endArg = lastArgIsThrowable ? lastIndex - 1 : lastIndex; + int lastNonThrowableArgIndex = lastArgIsThrowable ? lastArgIndex - 1 : lastArgIndex; - ImmutableList.Builder badArgsBuilder = ImmutableList.builder(); - for (int i = startArg; i <= endArg; i++) { - if (!ARG.matches(allArgs.get(i), state)) { - badArgsBuilder.add(i); - } - } - List badArgs = badArgsBuilder.build(); - if (badArgs.isEmpty() || TestCheckUtils.isTestCode(state)) { + List 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(); } } + private boolean isArgArray(VisitorState state, ExpressionTree lastArg) { + if (ARG_ARRAY.matches(lastArg, state)) { + return true; + } + // Arg array but cast as an Object array? + if (lastArg instanceof TypeCastTree && ARG_CAST_TO_OBJ_ARRAY.matches((TypeCastTree) lastArg, state)) { + return true; + } + return false; + } + private Optional checkThrowableArgumentNotWrapped(MethodInvocationTree tree, VisitorState state) { ExpressionTree lastArg = tree.getArguments().get(tree.getArguments().size() - 1); boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state); From d9eb188b96d85a54891429c5eb724bb96f3ec8eb Mon Sep 17 00:00:00 2001 From: Andreas Lundblad Date: Wed, 10 Jun 2020 22:16:05 -0700 Subject: [PATCH 03/12] Simplified logic. Vararg array argument now goes through regardless of type. --- .../baseline/errorprone/Slf4jLogsafeArgs.java | 21 +++---------------- 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java index e94b66086..6ba811ae3 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java @@ -31,7 +31,6 @@ import com.sun.source.tree.ExpressionTree; import com.sun.source.tree.MethodInvocationTree; import com.sun.source.tree.Tree; -import com.sun.source.tree.TypeCastTree; import com.sun.source.util.SimpleTreeVisitor; import java.util.Collections; import java.util.List; @@ -59,11 +58,8 @@ public final class Slf4jLogsafeArgs extends BugChecker implements MethodInvocati private static final Matcher THROWABLE = MoreMatchers.isSubtypeOf(Throwable.class); private static final Matcher ARG = MoreMatchers.isSubtypeOf("com.palantir.logsafe.Arg"); private static final Matcher MARKER = MoreMatchers.isSubtypeOf("org.slf4j.Marker"); - private static final Matcher ARG_ARRAY = Matchers.isSubtypeOf( - state -> state.getType(state.getTypeFromString("com.palantir.logsafe.Arg"), true, Collections.emptyList())); - private static final Matcher OBJECT_ARRAY = - Matchers.isSameType(s -> s.getType(s.getTypeFromString("java.lang.Object"), true, Collections.emptyList())); - private static final Matcher ARG_CAST_TO_OBJ_ARRAY = Matchers.typeCast(OBJECT_ARRAY, ARG_ARRAY); + private static final Matcher OBJECT_ARRAY = Matchers.isSubtypeOf( + s -> s.getType(s.getTypeFromString("java.lang.Object"), true, Collections.emptyList())); @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { @@ -81,7 +77,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState int lastArgIndex = allArgs.size() - 1; ExpressionTree lastArg = allArgs.get(lastArgIndex); - if (startArgIndex == lastArgIndex && isArgArray(state, lastArg)) { + if (startArgIndex == lastArgIndex && OBJECT_ARRAY.matches(lastArg, state)) { return Description.NO_MATCH; } @@ -102,17 +98,6 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState } } - private boolean isArgArray(VisitorState state, ExpressionTree lastArg) { - if (ARG_ARRAY.matches(lastArg, state)) { - return true; - } - // Arg array but cast as an Object array? - if (lastArg instanceof TypeCastTree && ARG_CAST_TO_OBJ_ARRAY.matches((TypeCastTree) lastArg, state)) { - return true; - } - return false; - } - private Optional checkThrowableArgumentNotWrapped(MethodInvocationTree tree, VisitorState state) { ExpressionTree lastArg = tree.getArguments().get(tree.getArguments().size() - 1); boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state); From 2de50062a96ed5d1fb2e012e7bc4337acab5fa8f Mon Sep 17 00:00:00 2001 From: Andreas Lundblad Date: Fri, 12 Jun 2020 12:07:35 -0700 Subject: [PATCH 04/12] Reverted from Stream to for loop --- .../baseline/errorprone/Slf4jLogsafeArgs.java | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java index 6ba811ae3..d074d844a 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/Slf4jLogsafeArgs.java @@ -17,6 +17,7 @@ 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; @@ -36,8 +37,6 @@ 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( @@ -46,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; @@ -84,18 +84,24 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState boolean lastArgIsThrowable = THROWABLE.matches(lastArg, state); int lastNonThrowableArgIndex = lastArgIsThrowable ? lastArgIndex - 1 : lastArgIndex; - List badArgIndices = IntStream.rangeClosed(startArgIndex, lastNonThrowableArgIndex) - .filter(i -> !ARG.matches(allArgs.get(i), state)) - .boxed() - .collect(Collectors.toList()); - - if (badArgIndices.isEmpty() || TestCheckUtils.isTestCode(state)) { + List badArgs = getBadArgIndices(state, allArgs, startArgIndex, lastNonThrowableArgIndex); + 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 " + badArgIndices) - .build(); } + + return buildDescription(tree) + .setMessage("slf4j log statement does not use logsafe parameters for arguments " + badArgs) + .build(); + } + + private List getBadArgIndices(VisitorState state, List args, int from, int to) { + ImmutableList.Builder badArgsBuilder = ImmutableList.builder(); + for (int i = from; i <= to; i++) { + if (!ARG.matches(args.get(i), state)) { + badArgsBuilder.add(i); + } + } + return badArgsBuilder.build(); } private Optional checkThrowableArgumentNotWrapped(MethodInvocationTree tree, VisitorState state) { From 79c2523037ea3f73abcd512225a9957a2aa45f44 Mon Sep 17 00:00:00 2001 From: Andreas Lundblad Date: Wed, 17 Jun 2020 09:13:34 -0700 Subject: [PATCH 05/12] Added changelog entry. --- changelog/@unreleased/pr-1394.v2.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/@unreleased/pr-1394.v2.yml diff --git a/changelog/@unreleased/pr-1394.v2.yml b/changelog/@unreleased/pr-1394.v2.yml new file mode 100644 index 000000000..e0d962159 --- /dev/null +++ b/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 From 3bd2a1e0bafa7199bf2220c44e6b5dae06992519 Mon Sep 17 00:00:00 2001 From: aldexis <53899003+aldexis@users.noreply.github.com> Date: Wed, 17 Jun 2020 18:36:20 +0200 Subject: [PATCH 06/12] Improve gradle-baseline-java integration with IntelliJ import (#1411) 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 --- changelog/@unreleased/pr-1411.v2.yml | 14 ++ .../baseline/plugins/BaselineIdea.groovy | 158 +++++++++++++++--- .../palantir/baseline/plugins/XmlUtils.java | 11 +- .../BaselineIdeaIntegrationTest.groovy | 69 ++++++++ 4 files changed, 228 insertions(+), 24 deletions(-) create mode 100644 changelog/@unreleased/pr-1411.v2.yml diff --git a/changelog/@unreleased/pr-1411.v2.yml b/changelog/@unreleased/pr-1411.v2.yml new file mode 100644 index 000000000..1f662adfa --- /dev/null +++ b/changelog/@unreleased/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 diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineIdea.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineIdea.groovy index 426bcafcf..a7b3f48de 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineIdea.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineIdea.groovy @@ -16,14 +16,20 @@ 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 @@ -31,10 +37,6 @@ 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 } + 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,32 +139,57 @@ 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(""" - - - """.stripIndent() - )) + addCopyrightFile(copyrightManager, file, fileName.toString()) } } @@ -160,6 +197,63 @@ class BaselineIdea extends AbstractBaselinePlugin { copyrightManager.@default = lastFileName } + private void addCopyrightIntellijImport() { + def copyrightDir = Paths.get("${configDir}/copyright/") + def copyrightFiles = getCopyrightFiles(copyrightDir) + + Supplier 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(""" + + + """.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(""" @@ -220,6 +329,9 @@ class BaselineIdea extends AbstractBaselinePlugin { """.stripIndent())) + } + + private static void addCheckstyleExternalDependencies(node) { def externalDependencies = GroovyXmlUtils.matchOrCreateChild(node, 'component', [name: 'ExternalDependencies']) GroovyXmlUtils.matchOrCreateChild(externalDependencies, 'plugin', [id: 'CheckStyle-IDEA']) } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/XmlUtils.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/XmlUtils.java index a1536cf63..19186b259 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/XmlUtils.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/XmlUtils.java @@ -27,6 +27,7 @@ import java.io.PrintWriter; import java.nio.charset.StandardCharsets; import java.util.function.Consumer; +import java.util.function.Supplier; import javax.xml.parsers.ParserConfigurationException; import org.xml.sax.SAXException; @@ -34,6 +35,12 @@ final class XmlUtils { private XmlUtils() {} static void createOrUpdateXmlFile(File configurationFile, Consumer configure) { + createOrUpdateXmlFile( + configurationFile, configure, () -> new Node(null, "project", ImmutableMap.of("version", "4"))); + } + + static void createOrUpdateXmlFile( + File configurationFile, Consumer configure, Supplier defaultRootNode) { Node rootNode; if (configurationFile.isFile()) { try { @@ -42,11 +49,13 @@ static void createOrUpdateXmlFile(File configurationFile, Consumer configu throw new RuntimeException("Couldn't parse existing configuration file: " + configurationFile, e); } } else { - rootNode = new Node(null, "project", ImmutableMap.of("version", "4")); + rootNode = defaultRootNode.get(); } configure.accept(rootNode); + configurationFile.getParentFile().mkdirs(); + try (BufferedWriter writer = Files.newWriter(configurationFile, StandardCharsets.UTF_8); PrintWriter printWriter = new PrintWriter(writer)) { XmlNodePrinter nodePrinter = new XmlNodePrinter(printWriter); diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineIdeaIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineIdeaIntegrationTest.groovy index e6911f114..11da5257a 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineIdeaIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineIdeaIntegrationTest.groovy @@ -21,6 +21,7 @@ import com.google.common.io.Files import org.apache.commons.io.FileUtils import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome +import spock.util.environment.RestoreSystemProperties class BaselineIdeaIntegrationTest extends AbstractPluginTest { def standardBuildFile = ''' @@ -86,6 +87,53 @@ class BaselineIdeaIntegrationTest extends AbstractPluginTest { rootIpr.contains('') } + @RestoreSystemProperties + def 'Idea project has copyright configuration when importing'() { + when: + buildFile << standardBuildFile + System.setProperty("idea.active", "true") + + then: + with().build() + def copyrightDir = new File(projectDir, ".idea/copyright") + copyrightDir.exists() + copyrightDir.isDirectory() + + def apacheCopyright = new File(copyrightDir, "001_apache-2.0.xml").text + apacheCopyright.contains('