From 77d041391f0d534b997dbc8cf2d8a53790a4c3ee Mon Sep 17 00:00:00 2001 From: Carter Kozak Date: Fri, 16 Mar 2018 13:57:06 -0400 Subject: [PATCH 01/43] Added BaselineClasspathConflict plugin --- .../palantir/baseline/plugins/Baseline.groovy | 1 + .../plugins/BaselineClasspathConflict.groovy | 62 +++++++++++++++++++ .../BaselineClasspathConflictTest.groovy | 42 +++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy create mode 100644 gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy index a39ed4305..734a8fef0 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy @@ -30,5 +30,6 @@ class Baseline implements Plugin { project.plugins.apply BaselineEclipse project.plugins.apply BaselineIdea project.plugins.apply BaselineErrorProne + project.plugins.apply BaselineClasspathConflict } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy new file mode 100644 index 000000000..ec9dde13f --- /dev/null +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy @@ -0,0 +1,62 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.plugins + +import java.util.jar.JarEntry +import java.util.jar.JarFile +import org.gradle.api.Project +import org.gradle.api.plugins.JavaPlugin +import org.gradle.internal.impldep.com.google.common.collect.HashMultimap +import org.gradle.internal.impldep.com.google.common.collect.SetMultimap + +/** + * Adds a simple test for shadowed classes. + */ +class BaselineClasspathConflict extends AbstractBaselinePlugin { + + @Override + void apply(Project project) { + if (project.plugins.hasPlugin(JavaPlugin)) { + def silentConflict = project.tasks.create("silentConflict") { + dependsOn: project.configurations.testRuntime + doLast { + SetMultimap classToJarMap = HashMultimap.create() + project.configurations.testRuntime.getResolvedConfiguration().getFiles().each { File jarFile -> + new JarFile(jarFile).entries() + // Only check class uniquness for now + .find({JarEntry entry -> entry.getName().endsWith(".class")}) + .each {JarEntry entry -> + classToJarMap.put(entry.getName(), jarFile) + } + } + StringBuilder errors = new StringBuilder(); + classToJarMap.keys().forEach({key -> + Set jars = classToJarMap.get(key) + if (jars.size() > 1) { + errors.append("Multiple copies of $key discovered in: $jars\n") + } + }) + if (errors.length() > 0) { + throw new IllegalStateException("Encountered classpath conflicts:\n$errors") + } + } + } + + project.tasks.check.dependsOn silentConflict + } + } +} diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy new file mode 100644 index 000000000..199363e66 --- /dev/null +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy @@ -0,0 +1,42 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline + +import com.palantir.baseline.plugins.BaselineClasspathConflict +import org.gradle.api.Project +import org.gradle.testfixtures.ProjectBuilder +import spock.lang.Specification + +class BaselineClasspathConflictTest extends Specification { + private Project project + + def setup() { + project = ProjectBuilder.builder().build() + project.plugins.apply 'java' + project.plugins.apply BaselineClasspathConflict + } + + def baselineClasspathConflictPluginApplied() { + expect: + project.plugins.hasPlugin(BaselineClasspathConflict.class) + } + + def baselineClasspathConflictTaskCreated() { + expect: + project.tasks.getByName("silentConflict") + } +} From 8c38f0c8daf6053b3d97879d1a3174ef97197cb6 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 16:17:09 +0100 Subject: [PATCH 02/43] Rename to BaselineClasspathDuplicatesPlugin --- .../groovy/com/palantir/baseline/plugins/Baseline.groovy | 2 +- ...ct.groovy => BaselineClasspathDuplicatesPlugin.groovy} | 5 +---- ...roovy => BaselineClasspathDuplicatesPluginTest.groovy} | 8 ++++---- 3 files changed, 6 insertions(+), 9 deletions(-) rename gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/{BaselineClasspathConflict.groovy => BaselineClasspathDuplicatesPlugin.groovy} (95%) rename gradle-baseline-java/src/test/groovy/com/palantir/baseline/{BaselineClasspathConflictTest.groovy => BaselineClasspathDuplicatesPluginTest.groovy} (80%) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy index 734a8fef0..6ac9f8d94 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy @@ -30,6 +30,6 @@ class Baseline implements Plugin { project.plugins.apply BaselineEclipse project.plugins.apply BaselineIdea project.plugins.apply BaselineErrorProne - project.plugins.apply BaselineClasspathConflict + project.plugins.apply BaselineClasspathDuplicatesPlugin } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy similarity index 95% rename from gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy rename to gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy index ec9dde13f..828e28982 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathConflict.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy @@ -23,10 +23,7 @@ import org.gradle.api.plugins.JavaPlugin import org.gradle.internal.impldep.com.google.common.collect.HashMultimap import org.gradle.internal.impldep.com.google.common.collect.SetMultimap -/** - * Adds a simple test for shadowed classes. - */ -class BaselineClasspathConflict extends AbstractBaselinePlugin { +class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { @Override void apply(Project project) { diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy similarity index 80% rename from gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy rename to gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy index 199363e66..786a6389f 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathConflictTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy @@ -16,23 +16,23 @@ package com.palantir.baseline -import com.palantir.baseline.plugins.BaselineClasspathConflict +import com.palantir.baseline.plugins.BaselineClasspathDuplicatesPlugin import org.gradle.api.Project import org.gradle.testfixtures.ProjectBuilder import spock.lang.Specification -class BaselineClasspathConflictTest extends Specification { +class BaselineClasspathDuplicatesPluginTest extends Specification { private Project project def setup() { project = ProjectBuilder.builder().build() project.plugins.apply 'java' - project.plugins.apply BaselineClasspathConflict + project.plugins.apply BaselineClasspathDuplicatesPlugin } def baselineClasspathConflictPluginApplied() { expect: - project.plugins.hasPlugin(BaselineClasspathConflict.class) + project.plugins.hasPlugin(BaselineClasspathDuplicatesPlugin.class) } def baselineClasspathConflictTaskCreated() { From d94f939daedf8e2fe2a4f98ff9d09d4ebc7b162a Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 16:17:21 +0100 Subject: [PATCH 03/43] Add meta info properties file --- .../com.palantir.baseline-classpath-duplicates.properties | 1 + 1 file changed, 1 insertion(+) create mode 100644 gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties diff --git a/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties b/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties new file mode 100644 index 000000000..3bd060255 --- /dev/null +++ b/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties @@ -0,0 +1 @@ +implementation-class=com.palantir.baseline.plugins.BaselineClasspathDuplicatesPlugin From 7941840a721bfa2322b71262c63f9fb951fc373c Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 16:20:36 +0100 Subject: [PATCH 04/43] silentConflict -> checkClasspathIsDuplicateFree --- .../plugins/BaselineClasspathDuplicatesPlugin.groovy | 7 +++---- .../baseline/BaselineClasspathDuplicatesPluginTest.groovy | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy index 828e28982..5d02d10f1 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy @@ -28,7 +28,7 @@ class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { @Override void apply(Project project) { if (project.plugins.hasPlugin(JavaPlugin)) { - def silentConflict = project.tasks.create("silentConflict") { + def task = project.tasks.create("checkClasspathIsDuplicateFree") { dependsOn: project.configurations.testRuntime doLast { SetMultimap classToJarMap = HashMultimap.create() @@ -36,8 +36,7 @@ class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { new JarFile(jarFile).entries() // Only check class uniquness for now .find({JarEntry entry -> entry.getName().endsWith(".class")}) - .each {JarEntry entry -> - classToJarMap.put(entry.getName(), jarFile) + .each {JarEntry entry -> classToJarMap.put(entry.getName(), jarFile) } } StringBuilder errors = new StringBuilder(); @@ -53,7 +52,7 @@ class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { } } - project.tasks.check.dependsOn silentConflict + project.tasks.check.dependsOn task } } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy index 786a6389f..f14f42b65 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy @@ -37,6 +37,6 @@ class BaselineClasspathDuplicatesPluginTest extends Specification { def baselineClasspathConflictTaskCreated() { expect: - project.tasks.getByName("silentConflict") + project.tasks.getByName("checkClasspathIsDuplicateFree") } } From 19b30c21945eee39dd8476377d707a2c92abaa1d Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 16:58:59 +0100 Subject: [PATCH 05/43] Factor out logic to a new CheckUniqueClassNamesTask --- .../BaselineClasspathDuplicatesPlugin.groovy | 34 ++------ .../tasks/CheckUniqueClassNamesTask.java | 84 +++++++++++++++++++ ...selineClasspathDuplicatesPluginTest.groovy | 4 +- 3 files changed, 92 insertions(+), 30 deletions(-) create mode 100644 gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy index 5d02d10f1..2066988fa 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy @@ -16,43 +16,21 @@ package com.palantir.baseline.plugins -import java.util.jar.JarEntry -import java.util.jar.JarFile +import com.palantir.baseline.tasks.CheckUniqueClassNamesTask import org.gradle.api.Project import org.gradle.api.plugins.JavaPlugin -import org.gradle.internal.impldep.com.google.common.collect.HashMultimap -import org.gradle.internal.impldep.com.google.common.collect.SetMultimap class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { @Override void apply(Project project) { if (project.plugins.hasPlugin(JavaPlugin)) { - def task = project.tasks.create("checkClasspathIsDuplicateFree") { - dependsOn: project.configurations.testRuntime - doLast { - SetMultimap classToJarMap = HashMultimap.create() - project.configurations.testRuntime.getResolvedConfiguration().getFiles().each { File jarFile -> - new JarFile(jarFile).entries() - // Only check class uniquness for now - .find({JarEntry entry -> entry.getName().endsWith(".class")}) - .each {JarEntry entry -> classToJarMap.put(entry.getName(), jarFile) - } - } - StringBuilder errors = new StringBuilder(); - classToJarMap.keys().forEach({key -> - Set jars = classToJarMap.get(key) - if (jars.size() > 1) { - errors.append("Multiple copies of $key discovered in: $jars\n") - } - }) - if (errors.length() > 0) { - throw new IllegalStateException("Encountered classpath conflicts:\n$errors") - } - } - } - project.tasks.check.dependsOn task + CheckUniqueClassNamesTask task = + project.getTasks().create("checkUniqueClassNames", CheckUniqueClassNamesTask) + task.setConfiguration(project.getConfigurations().getByName("testRuntime")) + + project.getTasks().getByName("check").dependsOn() } } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java new file mode 100644 index 000000000..788288fbc --- /dev/null +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -0,0 +1,84 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.tasks; + +import java.io.File; +import java.io.IOException; +import java.util.Enumeration; +import java.util.Set; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import org.gradle.api.DefaultTask; +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.TaskAction; +import org.gradle.internal.impldep.com.google.common.collect.HashMultimap; +import org.gradle.internal.impldep.com.google.common.collect.SetMultimap; + +public class CheckUniqueClassNamesTask extends DefaultTask { + + private Configuration configuration; + + public CheckUniqueClassNamesTask() { + setGroup("Verification"); + setDescription("Checks that the given configuration contains no identically named classes."); + } + + @Input + public Configuration getConfiguration() { + return configuration; + } + + public void setConfiguration(Configuration configuration) { + this.configuration = configuration; + } + + @TaskAction + public void checkForDuplicateClasses() throws IOException { + Set files = getConfiguration().getResolvedConfiguration().getFiles(); + SetMultimap classToJarMap = HashMultimap.create(); + + for (File jarFile : files) { + Enumeration entries = new JarFile(jarFile).entries(); + + while (entries.hasMoreElements()) { + JarEntry jarEntry = entries.nextElement(); + + if (!jarEntry.getName().endsWith(".class")) { + continue; + } + + classToJarMap.put(jarEntry.getName(), jarFile); + } + } + + StringBuilder errors = new StringBuilder(); + for (String className : classToJarMap.keys()) { + Set jars = classToJarMap.get(className); + if (jars.size() > 1) { + errors.append(String.format( + "%s appears in: %s\n", className, jars)); + } + } + + if (errors.length() > 0) { + throw new IllegalStateException(String.format( + "%s contains duplicate classes: %s", + getConfiguration().getName(), errors.toString())); + } + } +} diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy index f14f42b65..96849b3c1 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy @@ -35,8 +35,8 @@ class BaselineClasspathDuplicatesPluginTest extends Specification { project.plugins.hasPlugin(BaselineClasspathDuplicatesPlugin.class) } - def baselineClasspathConflictTaskCreated() { + def task_should_exist() { expect: - project.tasks.getByName("checkClasspathIsDuplicateFree") + project.tasks.getByName("checkUniqueClassNames") } } From 5ce2ee8f904d84adfbf3175a1c44a0fdd95da77c Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 17:02:51 +0100 Subject: [PATCH 06/43] apply plugin order shouldn't matter --- .../plugins/BaselineClasspathDuplicatesPlugin.groovy | 6 +++--- .../baseline/BaselineClasspathDuplicatesPluginTest.groovy | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy index 2066988fa..e6be34880 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy @@ -18,19 +18,19 @@ package com.palantir.baseline.plugins import com.palantir.baseline.tasks.CheckUniqueClassNamesTask import org.gradle.api.Project -import org.gradle.api.plugins.JavaPlugin class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { @Override void apply(Project project) { - if (project.plugins.hasPlugin(JavaPlugin)) { + project.getPlugins().withId("java", { CheckUniqueClassNamesTask task = project.getTasks().create("checkUniqueClassNames", CheckUniqueClassNamesTask) task.setConfiguration(project.getConfigurations().getByName("testRuntime")) project.getTasks().getByName("check").dependsOn() - } + + }); } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy index 96849b3c1..0688623da 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy @@ -26,8 +26,8 @@ class BaselineClasspathDuplicatesPluginTest extends Specification { def setup() { project = ProjectBuilder.builder().build() - project.plugins.apply 'java' project.plugins.apply BaselineClasspathDuplicatesPlugin + project.plugins.apply 'java' } def baselineClasspathConflictPluginApplied() { From c9c3b551a771e5c691d2d21ca4010fc717bacf2a Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 17:23:30 +0100 Subject: [PATCH 07/43] Checkstyle --- .../com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index 788288fbc..2407352b1 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -29,7 +29,7 @@ import org.gradle.internal.impldep.com.google.common.collect.HashMultimap; import org.gradle.internal.impldep.com.google.common.collect.SetMultimap; -public class CheckUniqueClassNamesTask extends DefaultTask { +public final class CheckUniqueClassNamesTask extends DefaultTask { private Configuration configuration; From 0919961f68d168673680dc6e191b9bcb6d775ddb Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 17:30:44 +0100 Subject: [PATCH 08/43] Ugh groovy --- .../baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy index e6be34880..900dd8bd3 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy @@ -29,7 +29,7 @@ class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { project.getTasks().create("checkUniqueClassNames", CheckUniqueClassNamesTask) task.setConfiguration(project.getConfigurations().getByName("testRuntime")) - project.getTasks().getByName("check").dependsOn() + project.getTasks().getByName("check").dependsOn(task) }); } From 9d0745b28b29e34206a11e4532e8d39e475a33cc Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 17:32:48 +0100 Subject: [PATCH 09/43] Add some magic publishing stuff --- gradle-baseline-java/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gradle-baseline-java/build.gradle b/gradle-baseline-java/build.gradle index 3f5b34fad..29878cfa1 100644 --- a/gradle-baseline-java/build.gradle +++ b/gradle-baseline-java/build.gradle @@ -54,6 +54,10 @@ pluginBundle { id = 'com.palantir.baseline-idea' displayName = 'Palantir Baseline IntelliJ Plugin' } + baselineClasspathDuplicatesPlugin { + id = 'com.palantir.baseline-classpath-duplicates' + displayName = 'Palantir Baseline Classpath Duplicates Plugin' + } } } From a81b6b3730d1eaf6230c234aceb35dc59f18ab17 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 19:38:54 +0100 Subject: [PATCH 10/43] Try removing the 'final' modifier --- .../com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index 2407352b1..c3396bed4 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -29,7 +29,8 @@ import org.gradle.internal.impldep.com.google.common.collect.HashMultimap; import org.gradle.internal.impldep.com.google.common.collect.SetMultimap; -public final class CheckUniqueClassNamesTask extends DefaultTask { +@SuppressWarnings("checkstyle:designforextension") // adding 'final' breaks nebula-test +public class CheckUniqueClassNamesTask extends DefaultTask { private Configuration configuration; From b66f48c28e352b237f119e22ba2fcf656de6528b Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 19:51:29 +0100 Subject: [PATCH 11/43] Convert to Java --- ...=> BaselineClasspathDuplicatesPlugin.java} | 20 ++++---- .../tasks/CheckUniqueClassNamesTask.java | 25 +++++----- ...eClasspathDuplicatesIntegrationTest.groovy | 48 +++++++++++++++++++ ...selineClasspathDuplicatesPluginTest.groovy | 42 ---------------- 4 files changed, 72 insertions(+), 63 deletions(-) rename gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/{BaselineClasspathDuplicatesPlugin.groovy => BaselineClasspathDuplicatesPlugin.java} (61%) create mode 100644 gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy delete mode 100644 gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java similarity index 61% rename from gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy rename to gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java index 900dd8bd3..022f6f3b8 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java @@ -14,22 +14,22 @@ * limitations under the License. */ -package com.palantir.baseline.plugins +package com.palantir.baseline.plugins; -import com.palantir.baseline.tasks.CheckUniqueClassNamesTask -import org.gradle.api.Project +import com.palantir.baseline.tasks.CheckUniqueClassNamesTask; +import org.gradle.api.Project; -class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { +public final class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { @Override - void apply(Project project) { - project.getPlugins().withId("java", { + public void apply(Project project) { + project.getPlugins().withId("java", plugin -> { - CheckUniqueClassNamesTask task = - project.getTasks().create("checkUniqueClassNames", CheckUniqueClassNamesTask) - task.setConfiguration(project.getConfigurations().getByName("testRuntime")) + CheckUniqueClassNamesTask task = project.getTasks().create("checkUniqueClassNames", + CheckUniqueClassNamesTask.class); + task.setConfiguration(project.getConfigurations().getByName("testRuntime")); - project.getTasks().getByName("check").dependsOn(task) + project.getTasks().getByName("check").dependsOn(task); }); } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index c3396bed4..b813980af 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -17,7 +17,6 @@ package com.palantir.baseline.tasks; import java.io.File; -import java.io.IOException; import java.util.Enumeration; import java.util.Set; import java.util.jar.JarEntry; @@ -29,7 +28,7 @@ import org.gradle.internal.impldep.com.google.common.collect.HashMultimap; import org.gradle.internal.impldep.com.google.common.collect.SetMultimap; -@SuppressWarnings("checkstyle:designforextension") // adding 'final' breaks nebula-test +@SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle public class CheckUniqueClassNamesTask extends DefaultTask { private Configuration configuration; @@ -49,21 +48,25 @@ public void setConfiguration(Configuration configuration) { } @TaskAction - public void checkForDuplicateClasses() throws IOException { + public void checkForDuplicateClasses() { Set files = getConfiguration().getResolvedConfiguration().getFiles(); SetMultimap classToJarMap = HashMultimap.create(); - for (File jarFile : files) { - Enumeration entries = new JarFile(jarFile).entries(); + for (File file : files) { + try (JarFile jarFile1 = new JarFile(file)) { + Enumeration entries = jarFile1.entries(); - while (entries.hasMoreElements()) { - JarEntry jarEntry = entries.nextElement(); + while (entries.hasMoreElements()) { + JarEntry jarEntry = entries.nextElement(); - if (!jarEntry.getName().endsWith(".class")) { - continue; - } + if (!jarEntry.getName().endsWith(".class")) { + continue; + } - classToJarMap.put(jarEntry.getName(), jarFile); + classToJarMap.put(jarEntry.getName(), file); + } + } catch (Exception e) { + getLogger().error("Failed to read JarFile {}", file, e); } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy new file mode 100644 index 000000000..d6e1eb2bd --- /dev/null +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -0,0 +1,48 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline + +import org.gradle.testkit.runner.BuildResult +import org.gradle.testkit.runner.TaskOutcome + +class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { + + def standardBuildFile = """ + plugins { + id 'java' + id 'com.palantir.baseline-classpath-duplicates' + } + """.stripIndent() + + def 'Plugin can be applied'() { + when: + buildFile << standardBuildFile + + then: + BuildResult result = with('tasks', '--stacktrace').build() + assert result.getOutput().contains("checkUniqueClassNames aaa") + } + + def 'Task should run as part of :check'() { + when: + buildFile << standardBuildFile + + then: + def result = with('check', '--stacktrace').build() + result.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS + } +} diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy deleted file mode 100644 index 0688623da..000000000 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesPluginTest.groovy +++ /dev/null @@ -1,42 +0,0 @@ -/* - * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.palantir.baseline - -import com.palantir.baseline.plugins.BaselineClasspathDuplicatesPlugin -import org.gradle.api.Project -import org.gradle.testfixtures.ProjectBuilder -import spock.lang.Specification - -class BaselineClasspathDuplicatesPluginTest extends Specification { - private Project project - - def setup() { - project = ProjectBuilder.builder().build() - project.plugins.apply BaselineClasspathDuplicatesPlugin - project.plugins.apply 'java' - } - - def baselineClasspathConflictPluginApplied() { - expect: - project.plugins.hasPlugin(BaselineClasspathDuplicatesPlugin.class) - } - - def task_should_exist() { - expect: - project.tasks.getByName("checkUniqueClassNames") - } -} From a1275df509d66c2695270afa0582ee7a1e43f5b9 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 19:54:19 +0100 Subject: [PATCH 12/43] Remove 'final' modifier --- .../baseline/plugins/BaselineClasspathDuplicatesPlugin.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java index 022f6f3b8..d8a0bae6a 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java @@ -19,7 +19,8 @@ import com.palantir.baseline.tasks.CheckUniqueClassNamesTask; import org.gradle.api.Project; -public final class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { +@SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle +public class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { @Override public void apply(Project project) { From ed4425eaf4562405f5b3f0b13f31ca501413b705 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 19:58:56 +0100 Subject: [PATCH 13/43] Verify up-to-date behaviour --- ...aselineClasspathDuplicatesIntegrationTest.groovy | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index d6e1eb2bd..8d5d1ba38 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -16,7 +16,6 @@ package com.palantir.baseline -import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { @@ -28,21 +27,21 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { } """.stripIndent() - def 'Plugin can be applied'() { + def 'Task should run as part of :check'() { when: buildFile << standardBuildFile then: - BuildResult result = with('tasks', '--stacktrace').build() - assert result.getOutput().contains("checkUniqueClassNames aaa") + def result = with('check', '--stacktrace').build() + result.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS } - def 'Task should run as part of :check'() { + def 'Task should not run twice if configuration is unchanged'() { when: buildFile << standardBuildFile then: - def result = with('check', '--stacktrace').build() - result.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS + with('checkUniqueClassNames').build().task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS + with('checkUniqueClassNames').build().task(':checkUniqueClassNames').outcome == TaskOutcome.UP_TO_DATE } } From 5ea80f6b25dde5ba72a6ae099295f3cfabaf3edb Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 20:00:45 +0100 Subject: [PATCH 14/43] Slightly different syntax --- .../plugins/BaselineClasspathDuplicatesPlugin.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java index d8a0bae6a..f743dee3a 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java @@ -26,11 +26,10 @@ public class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { public void apply(Project project) { project.getPlugins().withId("java", plugin -> { - CheckUniqueClassNamesTask task = project.getTasks().create("checkUniqueClassNames", - CheckUniqueClassNamesTask.class); - task.setConfiguration(project.getConfigurations().getByName("testRuntime")); - - project.getTasks().getByName("check").dependsOn(task); + project.getTasks().create("checkUniqueClassNames", CheckUniqueClassNamesTask.class, task -> { + task.setConfiguration(project.getConfigurations().getByName("testRuntime")); + project.getTasks().getByName("check").dependsOn(task); + }); }); } From a1b2156733b3848c8f07e2e8e592745cfd6db855 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 20:11:04 +0100 Subject: [PATCH 15/43] Don't use org.gradle.internal.impldep HashMultimap --- .../tasks/CheckUniqueClassNamesTask.java | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index b813980af..ae08251f2 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -18,6 +18,9 @@ import java.io.File; import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -25,8 +28,6 @@ import org.gradle.api.artifacts.Configuration; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.TaskAction; -import org.gradle.internal.impldep.com.google.common.collect.HashMultimap; -import org.gradle.internal.impldep.com.google.common.collect.SetMultimap; @SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle public class CheckUniqueClassNamesTask extends DefaultTask { @@ -50,7 +51,7 @@ public void setConfiguration(Configuration configuration) { @TaskAction public void checkForDuplicateClasses() { Set files = getConfiguration().getResolvedConfiguration().getFiles(); - SetMultimap classToJarMap = HashMultimap.create(); + Map> classToJarMap = new HashMap<>(); for (File file : files) { try (JarFile jarFile1 = new JarFile(file)) { @@ -63,7 +64,13 @@ public void checkForDuplicateClasses() { continue; } - classToJarMap.put(jarEntry.getName(), file); + HashSet initialSet = new HashSet<>(); + Set previous = classToJarMap.putIfAbsent(jarEntry.getName(), initialSet); + if (previous != null) { + previous.add(file); + } else { + initialSet.add(file); + } } } catch (Exception e) { getLogger().error("Failed to read JarFile {}", file, e); @@ -71,7 +78,7 @@ public void checkForDuplicateClasses() { } StringBuilder errors = new StringBuilder(); - for (String className : classToJarMap.keys()) { + for (String className : classToJarMap.keySet()) { Set jars = classToJarMap.get(className); if (jars.size() > 1) { errors.append(String.format( From 982e92d2bd16479091c519db6282b60ecc0a7a0e Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 20:15:59 +0100 Subject: [PATCH 16/43] Checkstyle --- .../com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index ae08251f2..4d1fbfc50 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -64,7 +64,7 @@ public void checkForDuplicateClasses() { continue; } - HashSet initialSet = new HashSet<>(); + Set initialSet = new HashSet<>(); Set previous = classToJarMap.putIfAbsent(jarEntry.getName(), initialSet); if (previous != null) { previous.add(file); From bcf86e822c41d16dc3422bbdcd65e0ee76a9593b Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 20:27:27 +0100 Subject: [PATCH 17/43] Test verifies duplicates can be detected --- ...neClasspathDuplicatesIntegrationTest.groovy | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index 8d5d1ba38..6dc94613b 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -16,6 +16,7 @@ package com.palantir.baseline +import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { @@ -25,6 +26,9 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { id 'java' id 'com.palantir.baseline-classpath-duplicates' } + repositories { + mavenCentral() + } """.stripIndent() def 'Task should run as part of :check'() { @@ -36,12 +40,20 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { result.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS } - def 'Task should not run twice if configuration is unchanged'() { + def 'Task should detect duplicates'() { when: buildFile << standardBuildFile + buildFile << """ + dependencies { + compile group: 'javax.el', name: 'javax.el-api', version: '3.0.0' + compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' + } + """.stripIndent() + BuildResult result = with('checkUniqueClassNames').buildAndFail() then: - with('checkUniqueClassNames').build().task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS - with('checkUniqueClassNames').build().task(':checkUniqueClassNames').outcome == TaskOutcome.UP_TO_DATE + result.task(':checkUniqueClassNames').outcome == TaskOutcome.FAILED + result.getOutput().contains("testRuntime contains duplicate classes") + println result.getOutput() } } From cfa7aece18b0d1586fffbcc96b3c09dc7ab9c139 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 20:52:46 +0100 Subject: [PATCH 18/43] test for up-to-date --- .../tasks/CheckUniqueClassNamesTask.java | 47 ++++++++++++++++--- ...eClasspathDuplicatesIntegrationTest.groovy | 14 +++++- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index 4d1fbfc50..a79b53e17 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -17,6 +17,10 @@ package com.palantir.baseline.tasks; import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; @@ -26,7 +30,11 @@ import java.util.jar.JarFile; import org.gradle.api.DefaultTask; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.Classpath; +import org.gradle.api.tasks.CompileClasspath; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.SkipWhenEmpty; import org.gradle.api.tasks.TaskAction; @SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle @@ -39,9 +47,12 @@ public CheckUniqueClassNamesTask() { setDescription("Checks that the given configuration contains no identically named classes."); } - @Input - public Configuration getConfiguration() { - return configuration; + @InputFiles + @Classpath + @CompileClasspath + @SkipWhenEmpty + public Iterable getClasspath() { + return configuration.getResolvedConfiguration().getFiles(); } public void setConfiguration(Configuration configuration) { @@ -50,10 +61,9 @@ public void setConfiguration(Configuration configuration) { @TaskAction public void checkForDuplicateClasses() { - Set files = getConfiguration().getResolvedConfiguration().getFiles(); Map> classToJarMap = new HashMap<>(); - for (File file : files) { + for (File file : getClasspath()) { try (JarFile jarFile1 = new JarFile(file)) { Enumeration entries = jarFile1.entries(); @@ -87,9 +97,32 @@ public void checkForDuplicateClasses() { } if (errors.length() > 0) { + writeResult(false); throw new IllegalStateException(String.format( "%s contains duplicate classes: %s", - getConfiguration().getName(), errors.toString())); + configuration.getName(), errors.toString())); + } + + writeResult(true); + } + + /** + * This only exists to convince gradle this task is incremental. + */ + @OutputFile + public File getResultFile() { + return getProject().getBuildDir().toPath() + .resolve(Paths.get("uniqueClassNames", configuration.getName())) + .toFile(); + } + + private void writeResult(boolean success) { + try { + File result = getResultFile(); + Files.createDirectories(result.toPath().getParent()); + Files.write(result.toPath(), Boolean.toString(success).getBytes(StandardCharsets.UTF_8)); + } catch (IOException e) { + throw new RuntimeException("Unable to write boolean result file", e); } } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index 6dc94613b..fbc8b2a6e 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -40,7 +40,7 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { result.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS } - def 'Task should detect duplicates'() { + def 'detect duplicates in two external jars'() { when: buildFile << standardBuildFile buildFile << """ @@ -56,4 +56,16 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { result.getOutput().contains("testRuntime contains duplicate classes") println result.getOutput() } + + def 'task should be up-to-date when classpath is unchanged'() { + when: + buildFile << standardBuildFile + + then: + BuildResult result1 = with('checkUniqueClassNames').build() + result1.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS + + BuildResult result = with('checkUniqueClassNames').build() + result.task(':checkUniqueClassNames').outcome == TaskOutcome.UP_TO_DATE + } } From 830af377e7528fac2d3d2ae970876f1e9413dbed Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 21:31:02 +0100 Subject: [PATCH 19/43] Sweet human readable output --- .../tasks/CheckUniqueClassNamesTask.java | 76 +++++++++++-------- ...eClasspathDuplicatesIntegrationTest.groovy | 6 +- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index a79b53e17..eca6a4962 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -30,11 +30,10 @@ import java.util.jar.JarFile; import org.gradle.api.DefaultTask; import org.gradle.api.artifacts.Configuration; -import org.gradle.api.tasks.Classpath; -import org.gradle.api.tasks.CompileClasspath; -import org.gradle.api.tasks.InputFiles; +import org.gradle.api.artifacts.ModuleVersionIdentifier; +import org.gradle.api.artifacts.ResolvedArtifact; +import org.gradle.api.tasks.Input; import org.gradle.api.tasks.OutputFile; -import org.gradle.api.tasks.SkipWhenEmpty; import org.gradle.api.tasks.TaskAction; @SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle @@ -47,12 +46,9 @@ public CheckUniqueClassNamesTask() { setDescription("Checks that the given configuration contains no identically named classes."); } - @InputFiles - @Classpath - @CompileClasspath - @SkipWhenEmpty - public Iterable getClasspath() { - return configuration.getResolvedConfiguration().getFiles(); + @Input + public Configuration getConfiguration() { + return configuration; } public void setConfiguration(Configuration configuration) { @@ -61,10 +57,10 @@ public void setConfiguration(Configuration configuration) { @TaskAction public void checkForDuplicateClasses() { - Map> classToJarMap = new HashMap<>(); + Map> classToJarMap = new HashMap<>(); - for (File file : getClasspath()) { - try (JarFile jarFile1 = new JarFile(file)) { + for (ResolvedArtifact resolvedArtifact : getConfiguration().getResolvedConfiguration().getResolvedArtifacts()) { + try (JarFile jarFile1 = new JarFile(resolvedArtifact.getFile())) { Enumeration entries = jarFile1.entries(); while (entries.hasMoreElements()) { @@ -74,36 +70,47 @@ public void checkForDuplicateClasses() { continue; } - Set initialSet = new HashSet<>(); - Set previous = classToJarMap.putIfAbsent(jarEntry.getName(), initialSet); - if (previous != null) { - previous.add(file); - } else { - initialSet.add(file); - } + addToMultiMap(classToJarMap, + jarEntry.getName().replaceAll("/", ".").replaceAll(".class", ""), + resolvedArtifact.getModuleVersion().getId()); } } catch (Exception e) { - getLogger().error("Failed to read JarFile {}", file, e); + getLogger().error("Failed to read JarFile {}", resolvedArtifact, e); } } - StringBuilder errors = new StringBuilder(); + Map, Set> jarsToOverlappingClasses = new HashMap<>(); for (String className : classToJarMap.keySet()) { - Set jars = classToJarMap.get(className); - if (jars.size() > 1) { - errors.append(String.format( - "%s appears in: %s\n", className, jars)); + Set sourceJars = classToJarMap.get(className); + if (sourceJars.size() == 1) { + continue; } + + addToMultiMap(jarsToOverlappingClasses, sourceJars, className); } - if (errors.length() > 0) { - writeResult(false); + boolean success = jarsToOverlappingClasses.isEmpty(); + writeResult(success); + + if (!success) { + for (Set problemJars : jarsToOverlappingClasses.keySet()) { + Set classes = jarsToOverlappingClasses.get(problemJars); + + int problemSize = problemJars.size(); + getLogger().error("Identically named classes found in {} jars ({}): {}", + problemSize, problemJars, classes); + } + throw new IllegalStateException(String.format( - "%s contains duplicate classes: %s", - configuration.getName(), errors.toString())); + "'%s' contains multiple copies of identically named classes - " + + "this may cause different runtime behaviour depending on classpath ordering.\n" + + "To resolve this, try excluding one of the following jars, " + + "changing a version or shadowing:\n\n\t%s", + configuration.getName(), + jarsToOverlappingClasses.keySet() + )); } - writeResult(true); } /** @@ -125,4 +132,11 @@ private void writeResult(boolean success) { throw new RuntimeException("Unable to write boolean result file", e); } } + + // implemented here so we don't pull in guava + private static boolean addToMultiMap(Map> multiMap, K key, V value) { + Set initialSet = new HashSet(); + Set existing = multiMap.putIfAbsent(key, initialSet); + return existing != null ? existing.add(value) : initialSet.add(value); + } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index fbc8b2a6e..dc2d4a5bf 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -49,11 +49,11 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' } """.stripIndent() - BuildResult result = with('checkUniqueClassNames').buildAndFail() + BuildResult result = with('checkUniqueClassNames', '--quiet').buildAndFail() then: - result.task(':checkUniqueClassNames').outcome == TaskOutcome.FAILED - result.getOutput().contains("testRuntime contains duplicate classes") + result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.el.ExpressionFactory,") + result.getOutput().contains("'testRuntime' contains multiple copies of identically named classes") println result.getOutput() } From 7784164efc13b740309f6d0967de56f1a73afbb3 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 21:38:02 +0100 Subject: [PATCH 20/43] Factor out the crawling --- .../tasks/CheckUniqueClassNamesTask.java | 56 ++++++++++--------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index eca6a4962..ed3e64a23 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -57,27 +57,7 @@ public void setConfiguration(Configuration configuration) { @TaskAction public void checkForDuplicateClasses() { - Map> classToJarMap = new HashMap<>(); - - for (ResolvedArtifact resolvedArtifact : getConfiguration().getResolvedConfiguration().getResolvedArtifacts()) { - try (JarFile jarFile1 = new JarFile(resolvedArtifact.getFile())) { - Enumeration entries = jarFile1.entries(); - - while (entries.hasMoreElements()) { - JarEntry jarEntry = entries.nextElement(); - - if (!jarEntry.getName().endsWith(".class")) { - continue; - } - - addToMultiMap(classToJarMap, - jarEntry.getName().replaceAll("/", ".").replaceAll(".class", ""), - resolvedArtifact.getModuleVersion().getId()); - } - } catch (Exception e) { - getLogger().error("Failed to read JarFile {}", resolvedArtifact, e); - } - } + Map> classToJarMap = constructClassNameToSourceJarMap(); Map, Set> jarsToOverlappingClasses = new HashMap<>(); for (String className : classToJarMap.keySet()) { @@ -93,13 +73,10 @@ public void checkForDuplicateClasses() { writeResult(success); if (!success) { - for (Set problemJars : jarsToOverlappingClasses.keySet()) { - Set classes = jarsToOverlappingClasses.get(problemJars); - - int problemSize = problemJars.size(); + jarsToOverlappingClasses.forEach((problemJars, classes) -> { getLogger().error("Identically named classes found in {} jars ({}): {}", - problemSize, problemJars, classes); - } + problemJars.size(), problemJars, classes); + }); throw new IllegalStateException(String.format( "'%s' contains multiple copies of identically named classes - " @@ -110,7 +87,32 @@ public void checkForDuplicateClasses() { jarsToOverlappingClasses.keySet() )); } + } + + private Map> constructClassNameToSourceJarMap() { + Map> classToJarMap = new HashMap<>(); + + for (ResolvedArtifact resolvedArtifact : getConfiguration().getResolvedConfiguration().getResolvedArtifacts()) { + try (JarFile jarFile1 = new JarFile(resolvedArtifact.getFile())) { + Enumeration entries = jarFile1.entries(); + + while (entries.hasMoreElements()) { + JarEntry jarEntry = entries.nextElement(); + + if (!jarEntry.getName().endsWith(".class")) { + continue; + } + + addToMultiMap(classToJarMap, + jarEntry.getName().replaceAll("/", ".").replaceAll(".class", ""), + resolvedArtifact.getModuleVersion().getId()); + } + } catch (Exception e) { + getLogger().error("Failed to read JarFile {}", resolvedArtifact, e); + } + } + return classToJarMap; } /** From 9e9ab49837bcf501bd6e9cc470ad569fdd044630 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 21:58:54 +0100 Subject: [PATCH 21/43] Get ready for parallel --- .../tasks/CheckUniqueClassNamesTask.java | 48 ++++++++++++------- ...eClasspathDuplicatesIntegrationTest.groovy | 2 +- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index ed3e64a23..e2341d98f 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -21,11 +21,16 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; +import java.time.Duration; +import java.time.Instant; +import java.util.Collection; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.jar.JarEntry; import java.util.jar.JarFile; import org.gradle.api.DefaultTask; @@ -57,20 +62,17 @@ public void setConfiguration(Configuration configuration) { @TaskAction public void checkForDuplicateClasses() { - Map> classToJarMap = constructClassNameToSourceJarMap(); + Map> classToJarMap = constructClassNameToSourceJarMap(); - Map, Set> jarsToOverlappingClasses = new HashMap<>(); - for (String className : classToJarMap.keySet()) { - Set sourceJars = classToJarMap.get(className); - if (sourceJars.size() == 1) { - continue; + Map, Collection> jarsToOverlappingClasses = new HashMap<>(); + classToJarMap.forEach((className, sourceJars) -> { + if (sourceJars.size() > 1) { + addToMultiMap(jarsToOverlappingClasses, new HashSet<>(sourceJars), className); } - - addToMultiMap(jarsToOverlappingClasses, sourceJars, className); - } + }); boolean success = jarsToOverlappingClasses.isEmpty(); - writeResult(success); + writeResultFile(success); if (!success) { jarsToOverlappingClasses.forEach((problemJars, classes) -> { @@ -89,9 +91,10 @@ public void checkForDuplicateClasses() { } } - private Map> constructClassNameToSourceJarMap() { - Map> classToJarMap = new HashMap<>(); + private Map> constructClassNameToSourceJarMap() { + Map> classToJarMap = new ConcurrentHashMap<>(); + Instant before = Instant.now(); for (ResolvedArtifact resolvedArtifact : getConfiguration().getResolvedConfiguration().getResolvedArtifacts()) { try (JarFile jarFile1 = new JarFile(resolvedArtifact.getFile())) { Enumeration entries = jarFile1.entries(); @@ -111,6 +114,10 @@ private Map> constructClassNameToSourceJarM getLogger().error("Failed to read JarFile {}", resolvedArtifact, e); } } + Instant after = Instant.now(); + + getLogger().info("Scanned {} classes for uniqueness ({}ms)", + classToJarMap.size(), Duration.between(before, after).toMillis()); return classToJarMap; } @@ -125,7 +132,7 @@ public File getResultFile() { .toFile(); } - private void writeResult(boolean success) { + private void writeResultFile(boolean success) { try { File result = getResultFile(); Files.createDirectories(result.toPath().getParent()); @@ -135,10 +142,15 @@ private void writeResult(boolean success) { } } - // implemented here so we don't pull in guava - private static boolean addToMultiMap(Map> multiMap, K key, V value) { - Set initialSet = new HashSet(); - Set existing = multiMap.putIfAbsent(key, initialSet); - return existing != null ? existing.add(value) : initialSet.add(value); + /** + * Implemented here so we don't pull in guava. Note that CopyOnWriteArrayList is threadsafe + * so we can parallelize elsewhere. + */ + private static void addToMultiMap(Map> multiMap, K key, V value) { + multiMap.compute(key, (unused, collection) -> { + Collection newCollection = collection != null ? collection : new CopyOnWriteArrayList<>(); + newCollection.add(value); + return newCollection; + }); } } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index dc2d4a5bf..18ebaf4ed 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -52,7 +52,7 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { BuildResult result = with('checkUniqueClassNames', '--quiet').buildAndFail() then: - result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.el.ExpressionFactory,") + result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.") result.getOutput().contains("'testRuntime' contains multiple copies of identically named classes") println result.getOutput() } From 45917744014ac271382ba2918ba8194bd0c7d85d Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 22:27:57 +0100 Subject: [PATCH 22/43] parallelStream over dependencies --- .../tasks/CheckUniqueClassNamesTask.java | 15 +++++++------ ...eClasspathDuplicatesIntegrationTest.groovy | 21 +++++++++++++++++-- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index e2341d98f..e57c64db9 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -95,10 +95,13 @@ private Map> constructClassNameToSou Map> classToJarMap = new ConcurrentHashMap<>(); Instant before = Instant.now(); - for (ResolvedArtifact resolvedArtifact : getConfiguration().getResolvedConfiguration().getResolvedArtifacts()) { - try (JarFile jarFile1 = new JarFile(resolvedArtifact.getFile())) { - Enumeration entries = jarFile1.entries(); + Set dependencies = getConfiguration() + .getResolvedConfiguration() + .getResolvedArtifacts(); + dependencies.parallelStream().forEach(resolvedArtifact -> { + try (JarFile jarFile = new JarFile(resolvedArtifact.getFile())) { + Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) { JarEntry jarEntry = entries.nextElement(); @@ -113,11 +116,11 @@ private Map> constructClassNameToSou } catch (Exception e) { getLogger().error("Failed to read JarFile {}", resolvedArtifact, e); } - } + }); Instant after = Instant.now(); - getLogger().info("Scanned {} classes for uniqueness ({}ms)", - classToJarMap.size(), Duration.between(before, after).toMillis()); + getLogger().info("Checked {} classes from {} dependencies for uniqueness ({}ms)", + classToJarMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); return classToJarMap; } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index 18ebaf4ed..400bac8d6 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -49,12 +49,11 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' } """.stripIndent() - BuildResult result = with('checkUniqueClassNames', '--quiet').buildAndFail() + BuildResult result = with('checkUniqueClassNames').buildAndFail() then: result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.") result.getOutput().contains("'testRuntime' contains multiple copies of identically named classes") - println result.getOutput() } def 'task should be up-to-date when classpath is unchanged'() { @@ -68,4 +67,22 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { BuildResult result = with('checkUniqueClassNames').build() result.task(':checkUniqueClassNames').outcome == TaskOutcome.UP_TO_DATE } + + def 'passes when no duplicates are present'() { + when: + buildFile << standardBuildFile + buildFile << """ + dependencies { + compile 'com.google.guava:guava:19.0' + compile 'org.apache.commons:commons-io:1.3.2' + compile 'junit:junit:4.12' + compile 'com.netflix.nebula:nebula-test:6.4.2' + } + """.stripIndent() + BuildResult result = with('checkUniqueClassNames', '--info').build() + + then: + result.task(":checkUniqueClassNames").outcome == TaskOutcome.SUCCESS + println result.getOutput() + } } From ea7cd0bba03cd921817e6516eecf189c9dcb2f94 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 23:02:52 +0100 Subject: [PATCH 23/43] Test case for project dependencies --- .../tasks/CheckUniqueClassNamesTask.java | 12 ++++- ...eClasspathDuplicatesIntegrationTest.groovy | 53 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java index e57c64db9..52f1b3058 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java @@ -100,7 +100,14 @@ private Map> constructClassNameToSou .getResolvedArtifacts(); dependencies.parallelStream().forEach(resolvedArtifact -> { - try (JarFile jarFile = new JarFile(resolvedArtifact.getFile())) { + + File file = resolvedArtifact.getFile(); + if (!file.exists()) { + getLogger().info("Skipping non-existent jar {}: {}", resolvedArtifact, file); + return; + } + + try (JarFile jarFile = new JarFile(file)) { Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) { JarEntry jarEntry = entries.nextElement(); @@ -114,7 +121,8 @@ private Map> constructClassNameToSou resolvedArtifact.getModuleVersion().getId()); } } catch (Exception e) { - getLogger().error("Failed to read JarFile {}", resolvedArtifact, e); + getLogger().error("Failed to read JarFile {}, skipping...", resolvedArtifact, e); + throw new RuntimeException(e); } }); Instant after = Instant.now(); diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy index 400bac8d6..9605346f0 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy @@ -16,6 +16,8 @@ package com.palantir.baseline +import java.nio.file.Files +import java.util.stream.Stream import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome @@ -26,6 +28,9 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { id 'java' id 'com.palantir.baseline-classpath-duplicates' } + subprojects { + apply plugin: 'java' + } repositories { mavenCentral() } @@ -85,4 +90,52 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { result.task(":checkUniqueClassNames").outcome == TaskOutcome.SUCCESS println result.getOutput() } + + def 'should detect duplicates from transitive dependencies'() { + when: + multiProject.addSubproject('foo', """ + dependencies { + compile group: 'javax.el', name: 'javax.el-api', version: '3.0.0' + } + """) + multiProject.addSubproject('bar', """ + dependencies { + compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' + } + """) + + buildFile << standardBuildFile + buildFile << """ + dependencies { + compile project(':foo') + compile project(':bar') + } + """.stripIndent() + + then: + BuildResult result = with('checkUniqueClassNames').buildAndFail() + result.output.contains("Identically named classes found in 2 jars") + } + + def 'currently skips duplicates from user-authored code'() { + when: + Stream.of(multiProject.addSubproject('foo'), multiProject.addSubproject('bar')).forEach({ subproject -> + File myClass = new File(subproject, "src/main/com/something/MyClass.java") + Files.createDirectories(myClass.toPath().getParent()) + myClass << "package com.something; class MyClass {}" + }) + + buildFile << standardBuildFile + buildFile << """ + dependencies { + compile project(':foo') + compile project(':bar') + } + """.stripIndent() + + then: + BuildResult result = with('checkUniqueClassNames', '--info').build() + println result.getOutput() + result.task(":checkUniqueClassNames").outcome == TaskOutcome.SUCCESS // ideally should should say failed! + } } From 67264ff5388e7dcdcac0d3267320e539ea809469 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 23:13:10 +0100 Subject: [PATCH 24/43] Rename to 'com.palantir.baseline-class-uniqueness' --- gradle-baseline-java/build.gradle | 6 ++--- .../palantir/baseline/plugins/Baseline.groovy | 2 +- ...ava => BaselineClassUniquenessPlugin.java} | 8 +++--- ...ask.java => CheckClassUniquenessTask.java} | 4 +-- ...ntir.baseline-class-uniqueness.properties} | 2 +- ...assUniquenessPluginIntegrationTest.groovy} | 26 +++++++++---------- 6 files changed, 23 insertions(+), 25 deletions(-) rename gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/{BaselineClasspathDuplicatesPlugin.java => BaselineClassUniquenessPlugin.java} (81%) rename gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/{CheckUniqueClassNamesTask.java => CheckClassUniquenessTask.java} (98%) rename gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/{com.palantir.baseline-classpath-duplicates.properties => com.palantir.baseline-class-uniqueness.properties} (75%) rename gradle-baseline-java/src/test/groovy/com/palantir/baseline/{BaselineClasspathDuplicatesIntegrationTest.groovy => BaselineClassUniquenessPluginIntegrationTest.groovy} (79%) diff --git a/gradle-baseline-java/build.gradle b/gradle-baseline-java/build.gradle index 29878cfa1..8326d1d3c 100644 --- a/gradle-baseline-java/build.gradle +++ b/gradle-baseline-java/build.gradle @@ -54,9 +54,9 @@ pluginBundle { id = 'com.palantir.baseline-idea' displayName = 'Palantir Baseline IntelliJ Plugin' } - baselineClasspathDuplicatesPlugin { - id = 'com.palantir.baseline-classpath-duplicates' - displayName = 'Palantir Baseline Classpath Duplicates Plugin' + baselineClassUniquenessPlugin { + id = 'com.palantir.baseline-class-uniqueness' + displayName = 'Palantir Baseline Class Uniqueness Plugin' } } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy index 6ac9f8d94..f2fbe6f66 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy @@ -30,6 +30,6 @@ class Baseline implements Plugin { project.plugins.apply BaselineEclipse project.plugins.apply BaselineIdea project.plugins.apply BaselineErrorProne - project.plugins.apply BaselineClasspathDuplicatesPlugin + project.plugins.apply BaselineClassUniquenessPlugin } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java similarity index 81% rename from gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java rename to gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java index f743dee3a..4886b468d 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClasspathDuplicatesPlugin.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java @@ -16,21 +16,19 @@ package com.palantir.baseline.plugins; -import com.palantir.baseline.tasks.CheckUniqueClassNamesTask; +import com.palantir.baseline.tasks.CheckClassUniquenessTask; import org.gradle.api.Project; @SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle -public class BaselineClasspathDuplicatesPlugin extends AbstractBaselinePlugin { +public class BaselineClassUniquenessPlugin extends AbstractBaselinePlugin { @Override public void apply(Project project) { project.getPlugins().withId("java", plugin -> { - - project.getTasks().create("checkUniqueClassNames", CheckUniqueClassNamesTask.class, task -> { + project.getTasks().create("checkClassUniqueness", CheckClassUniquenessTask.class, task -> { task.setConfiguration(project.getConfigurations().getByName("testRuntime")); project.getTasks().getByName("check").dependsOn(task); }); - }); } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java similarity index 98% rename from gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java rename to gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 52f1b3058..dbf774b32 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUniqueClassNamesTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -42,11 +42,11 @@ import org.gradle.api.tasks.TaskAction; @SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle -public class CheckUniqueClassNamesTask extends DefaultTask { +public class CheckClassUniquenessTask extends DefaultTask { private Configuration configuration; - public CheckUniqueClassNamesTask() { + public CheckClassUniquenessTask() { setGroup("Verification"); setDescription("Checks that the given configuration contains no identically named classes."); } diff --git a/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties b/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-class-uniqueness.properties similarity index 75% rename from gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties rename to gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-class-uniqueness.properties index 3bd060255..550b78235 100644 --- a/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-classpath-duplicates.properties +++ b/gradle-baseline-java/src/main/resources/META-INF/gradle-plugins/com.palantir.baseline-class-uniqueness.properties @@ -1 +1 @@ -implementation-class=com.palantir.baseline.plugins.BaselineClasspathDuplicatesPlugin +implementation-class=com.palantir.baseline.plugins.BaselineClassUniquenessPlugin diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy similarity index 79% rename from gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy rename to gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index 9605346f0..b2890e3d1 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClasspathDuplicatesIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -21,12 +21,12 @@ import java.util.stream.Stream import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.TaskOutcome -class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { +class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { def standardBuildFile = """ plugins { id 'java' - id 'com.palantir.baseline-classpath-duplicates' + id 'com.palantir.baseline-class-uniqueness' } subprojects { apply plugin: 'java' @@ -42,7 +42,7 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { then: def result = with('check', '--stacktrace').build() - result.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS + result.task(':checkClassUniqueness').outcome == TaskOutcome.SUCCESS } def 'detect duplicates in two external jars'() { @@ -54,7 +54,7 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' } """.stripIndent() - BuildResult result = with('checkUniqueClassNames').buildAndFail() + BuildResult result = with('checkClassUniqueness').buildAndFail() then: result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.") @@ -66,11 +66,11 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { buildFile << standardBuildFile then: - BuildResult result1 = with('checkUniqueClassNames').build() - result1.task(':checkUniqueClassNames').outcome == TaskOutcome.SUCCESS + BuildResult result1 = with('checkClassUniqueness').build() + result1.task(':checkClassUniqueness').outcome == TaskOutcome.SUCCESS - BuildResult result = with('checkUniqueClassNames').build() - result.task(':checkUniqueClassNames').outcome == TaskOutcome.UP_TO_DATE + BuildResult result = with('checkClassUniqueness').build() + result.task(':checkClassUniqueness').outcome == TaskOutcome.UP_TO_DATE } def 'passes when no duplicates are present'() { @@ -84,10 +84,10 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { compile 'com.netflix.nebula:nebula-test:6.4.2' } """.stripIndent() - BuildResult result = with('checkUniqueClassNames', '--info').build() + BuildResult result = with('checkClassUniqueness', '--info').build() then: - result.task(":checkUniqueClassNames").outcome == TaskOutcome.SUCCESS + result.task(":checkClassUniqueness").outcome == TaskOutcome.SUCCESS println result.getOutput() } @@ -113,7 +113,7 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { """.stripIndent() then: - BuildResult result = with('checkUniqueClassNames').buildAndFail() + BuildResult result = with('checkClassUniqueness').buildAndFail() result.output.contains("Identically named classes found in 2 jars") } @@ -134,8 +134,8 @@ class BaselineClasspathDuplicatesIntegrationTest extends AbstractPluginTest { """.stripIndent() then: - BuildResult result = with('checkUniqueClassNames', '--info').build() + BuildResult result = with('checkClassUniqueness', '--info').build() println result.getOutput() - result.task(":checkUniqueClassNames").outcome == TaskOutcome.SUCCESS // ideally should should say failed! + result.task(":checkClassUniqueness").outcome == TaskOutcome.SUCCESS // ideally should should say failed! } } From 27b793c63bff9963e4af3f8c5f8d8e0e4435032c Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 23:11:46 +0100 Subject: [PATCH 25/43] README --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 5f9ec923b..ca3996630 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,7 @@ apply plugin: 'com.palantir.baseline-eclipse' apply plugin: 'com.palantir.baseline-idea' apply plugin: 'org.inferred.processors' // installs the "processor" configuration needed for baseline-error-prone apply plugin: 'com.palantir.baseline-error-prone' +apply plugin: 'com.palantir.baseline-class-uniqueness' ``` - Run ``./gradlew baselineUpdateConfig`` to download the config files @@ -255,6 +256,13 @@ checks](https://errorprone.info): - Slf4jLogsafeArgs: Allow only com.palantir.logsafe.Arg types as parameter inputs to slf4j log messages. More information on Safe Logging can be found at [github.com/palantir/safe-logging](https://github.com/palantir/safe-logging). + +### Class Uniqueness Plugin (com.palantir.baseline-class-uniqueness) + +Run `./gradlew checkClassUniqueness` to scan all jars on the `testRuntime` classpath for identically named classes. +This task will run automatically as part of `./gradlew build`. + + ### Copyright Checks By default Baseline enforces Palantir copyright at the beginning of files. To change this, edit the template copyright From 8692c46ff523d55055d7b01865ce5eac1ef4a772 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Mon, 30 Apr 2018 23:14:46 +0100 Subject: [PATCH 26/43] Don't enable straight away for everyone --- .../main/groovy/com/palantir/baseline/plugins/Baseline.groovy | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy index f2fbe6f66..766ec7e58 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/Baseline.groovy @@ -30,6 +30,8 @@ class Baseline implements Plugin { project.plugins.apply BaselineEclipse project.plugins.apply BaselineIdea project.plugins.apply BaselineErrorProne - project.plugins.apply BaselineClassUniquenessPlugin + + // TODO(dfox): enable this when it has been validated on a few real projects + // project.plugins.apply BaselineClassUniquenessPlugin } } From e676bb2ca8e0b56495484eb3442479746b7804e3 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 May 2018 14:23:35 +0100 Subject: [PATCH 27/43] Add sweet formatted table with count of offending classes --- .../tasks/CheckClassUniquenessTask.java | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index dbf774b32..6bbad190c 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -24,15 +24,18 @@ import java.time.Duration; import java.time.Instant; import java.util.Collection; +import java.util.Comparator; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import java.util.stream.Collectors; import org.gradle.api.DefaultTask; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ModuleVersionIdentifier; @@ -84,13 +87,34 @@ public void checkForDuplicateClasses() { "'%s' contains multiple copies of identically named classes - " + "this may cause different runtime behaviour depending on classpath ordering.\n" + "To resolve this, try excluding one of the following jars, " - + "changing a version or shadowing:\n\n\t%s", + + "changing a version or shadowing:\n\n%s", configuration.getName(), - jarsToOverlappingClasses.keySet() + formatProblemJarsTable(jarsToOverlappingClasses) )); } } + private String formatProblemJarsTable( + Map, Collection> jarsToOverlappingClasses) { + + int maxLength = jarsToOverlappingClasses.keySet().stream().flatMap(Set::stream) + .map(ModuleVersionIdentifier::toString) + .map(String::length) + .max(Comparator.naturalOrder()).get(); + String format = "%-" + (maxLength + 1) + "s"; + + Set tableRows = new TreeSet<>(); + jarsToOverlappingClasses.forEach((problemJars, classes) -> { + StringBuilder builder = new StringBuilder(); + builder.append(String.format("\t%-14s", "(" + classes.size() + " classes) ")); + problemJars.forEach(jar -> builder.append(String.format(format, jar))); + + tableRows.add(builder.toString()); + }); + + return tableRows.stream().collect(Collectors.joining("\n")); + } + private Map> constructClassNameToSourceJarMap() { Map> classToJarMap = new ConcurrentHashMap<>(); From 94c6b380a2271da3c5c9d9b18f7d02c83ea1bf41 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 May 2018 14:35:25 +0100 Subject: [PATCH 28/43] Sort table nicely --- .../tasks/CheckClassUniquenessTask.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 6bbad190c..074a50623 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -30,7 +30,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.TreeSet; +import java.util.TreeMap; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.jar.JarEntry; @@ -103,16 +103,18 @@ private String formatProblemJarsTable( .max(Comparator.naturalOrder()).get(); String format = "%-" + (maxLength + 1) + "s"; - Set tableRows = new TreeSet<>(); - jarsToOverlappingClasses.forEach((problemJars, classes) -> { - StringBuilder builder = new StringBuilder(); - builder.append(String.format("\t%-14s", "(" + classes.size() + " classes) ")); - problemJars.forEach(jar -> builder.append(String.format(format, jar))); - - tableRows.add(builder.toString()); - }); - - return tableRows.stream().collect(Collectors.joining("\n")); + TreeMap sortedTable = jarsToOverlappingClasses.entrySet().stream().collect(Collectors.toMap( + entry -> entry.getKey().stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()), + entry -> entry.getValue().size(), + (a, b) -> { + throw new RuntimeException("Unexpected collision: " + a + ", " + b); + }, + TreeMap::new)); + + StringBuilder builder = new StringBuilder(); + sortedTable.forEach((jars, classes) -> + builder.append(String.format("\t%-14s", "(" + classes + " classes) ") + jars + "\n")); + return builder.toString(); } private Map> constructClassNameToSourceJarMap() { From 4f1f8c8d3361f6bce699e7c751b8629e13387950 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 May 2018 14:43:57 +0100 Subject: [PATCH 29/43] Check runtime instead of testRuntime by default --- .../baseline/plugins/BaselineClassUniquenessPlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java index 4886b468d..3be5c6282 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java @@ -26,7 +26,7 @@ public class BaselineClassUniquenessPlugin extends AbstractBaselinePlugin { public void apply(Project project) { project.getPlugins().withId("java", plugin -> { project.getTasks().create("checkClassUniqueness", CheckClassUniquenessTask.class, task -> { - task.setConfiguration(project.getConfigurations().getByName("testRuntime")); + task.setConfiguration(project.getConfigurations().getByName("runtime")); project.getTasks().getByName("check").dependsOn(task); }); }); From f6b4ebd07261163bbbfa13e07cae64ed97b926c3 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 May 2018 14:51:08 +0100 Subject: [PATCH 30/43] More comprehensive test --- .../tasks/CheckClassUniquenessTask.java | 6 +++--- ...lassUniquenessPluginIntegrationTest.groovy | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 074a50623..8ca2d5779 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -103,11 +103,11 @@ private String formatProblemJarsTable( .max(Comparator.naturalOrder()).get(); String format = "%-" + (maxLength + 1) + "s"; - TreeMap sortedTable = jarsToOverlappingClasses.entrySet().stream().collect(Collectors.toMap( + Map sortedTable = jarsToOverlappingClasses.entrySet().stream().collect(Collectors.toMap( entry -> entry.getKey().stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()), entry -> entry.getValue().size(), - (a, b) -> { - throw new RuntimeException("Unexpected collision: " + a + ", " + b); + (first, second) -> { + throw new RuntimeException("Unexpected collision: " + first + ", " + second); }, TreeMap::new)); diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index b2890e3d1..e5abc5289 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -33,6 +33,7 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { } repositories { mavenCentral() + maven { url 'https://dl.bintray.com/palantir/releases' } } """.stripIndent() @@ -52,13 +53,25 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { dependencies { compile group: 'javax.el', name: 'javax.el-api', version: '3.0.0' compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' + compile 'com.google.code.findbugs:annotations:3.0.0' + compile 'com.github.stephenc.jcip:jcip-annotations:1.0-1' + compile 'commons-logging:commons-logging:1.2' + compile 'org.slf4j:jcl-over-slf4j:1.7.25' + compile 'com.palantir.tritium:tritium-api:0.9.0' + compile 'com.palantir.tritium:tritium-core:0.9.0' } """.stripIndent() BuildResult result = with('checkClassUniqueness').buildAndFail() then: result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.") - result.getOutput().contains("'testRuntime' contains multiple copies of identically named classes") + result.getOutput().contains("'runtime' contains multiple copies of identically named classes") + result.getOutput().contains("(4 classes) com.google.code.findbugs:annotations:3.0.1 net.jcip:jcip-annotations:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1"); + result.getOutput().contains("(35 classes) com.google.code.findbugs:jsr305:3.0.1 com.google.code.findbugs:annotations:3.0.1"); + result.getOutput().contains("(1 classes) com.palantir.tritium:tritium-api:0.9.0 com.palantir.tritium:tritium-core:0.9.0"); + result.getOutput().contains("(6 classes) commons-logging:commons-logging:1.2 org.slf4j:jcl-over-slf4j:1.7.25"); + result.getOutput().contains("(26 classes) javax.servlet.jsp:jsp-api:2.1 javax.el:javax.el-api:3.0.0"); + println result.getOutput() } def 'task should be up-to-date when classpath is unchanged'() { @@ -96,12 +109,12 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { multiProject.addSubproject('foo', """ dependencies { compile group: 'javax.el', name: 'javax.el-api', version: '3.0.0' - } + } """) multiProject.addSubproject('bar', """ dependencies { compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' - } + } """) buildFile << standardBuildFile From 9f4963d80d50b113087e57cf353b500e82a7777c Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Tue, 1 May 2018 17:33:19 +0100 Subject: [PATCH 31/43] Factor out ClassUniquenessAnalyzer --- .../tasks/CheckClassUniquenessTask.java | 91 ++------------ .../tasks/ClassUniquenessAnalyzer.java | 113 ++++++++++++++++++ 2 files changed, 122 insertions(+), 82 deletions(-) create mode 100644 gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 8ca2d5779..1b16899cd 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -21,25 +21,14 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; -import java.time.Duration; -import java.time.Instant; -import java.util.Collection; import java.util.Comparator; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.TreeMap; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; import java.util.stream.Collectors; import org.gradle.api.DefaultTask; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ModuleVersionIdentifier; -import org.gradle.api.artifacts.ResolvedArtifact; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; @@ -65,20 +54,14 @@ public void setConfiguration(Configuration configuration) { @TaskAction public void checkForDuplicateClasses() { - Map> classToJarMap = constructClassNameToSourceJarMap(); - - Map, Collection> jarsToOverlappingClasses = new HashMap<>(); - classToJarMap.forEach((className, sourceJars) -> { - if (sourceJars.size() > 1) { - addToMultiMap(jarsToOverlappingClasses, new HashSet<>(sourceJars), className); - } - }); - - boolean success = jarsToOverlappingClasses.isEmpty(); + ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer(getLogger()); + analyzer.analyzeConfiguration(getConfiguration()); + boolean success = analyzer.getProblemJars().isEmpty(); writeResultFile(success); if (!success) { - jarsToOverlappingClasses.forEach((problemJars, classes) -> { + analyzer.getProblemJars().forEach((problemJars) -> { + Set classes = analyzer.getDuplicateClassesInProblemJars(problemJars); getLogger().error("Identically named classes found in {} jars ({}): {}", problemJars.size(), problemJars, classes); }); @@ -89,21 +72,19 @@ public void checkForDuplicateClasses() { + "To resolve this, try excluding one of the following jars, " + "changing a version or shadowing:\n\n%s", configuration.getName(), - formatProblemJarsTable(jarsToOverlappingClasses) + formatSummary(analyzer) )); } } - private String formatProblemJarsTable( - Map, Collection> jarsToOverlappingClasses) { - - int maxLength = jarsToOverlappingClasses.keySet().stream().flatMap(Set::stream) + private static String formatSummary(ClassUniquenessAnalyzer summary) { + int maxLength = summary.jarsToClasses().keySet().stream().flatMap(Set::stream) .map(ModuleVersionIdentifier::toString) .map(String::length) .max(Comparator.naturalOrder()).get(); String format = "%-" + (maxLength + 1) + "s"; - Map sortedTable = jarsToOverlappingClasses.entrySet().stream().collect(Collectors.toMap( + Map sortedTable = summary.jarsToClasses().entrySet().stream().collect(Collectors.toMap( entry -> entry.getKey().stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()), entry -> entry.getValue().size(), (first, second) -> { @@ -117,48 +98,6 @@ private String formatProblemJarsTable( return builder.toString(); } - private Map> constructClassNameToSourceJarMap() { - Map> classToJarMap = new ConcurrentHashMap<>(); - - Instant before = Instant.now(); - Set dependencies = getConfiguration() - .getResolvedConfiguration() - .getResolvedArtifacts(); - - dependencies.parallelStream().forEach(resolvedArtifact -> { - - File file = resolvedArtifact.getFile(); - if (!file.exists()) { - getLogger().info("Skipping non-existent jar {}: {}", resolvedArtifact, file); - return; - } - - try (JarFile jarFile = new JarFile(file)) { - Enumeration entries = jarFile.entries(); - while (entries.hasMoreElements()) { - JarEntry jarEntry = entries.nextElement(); - - if (!jarEntry.getName().endsWith(".class")) { - continue; - } - - addToMultiMap(classToJarMap, - jarEntry.getName().replaceAll("/", ".").replaceAll(".class", ""), - resolvedArtifact.getModuleVersion().getId()); - } - } catch (Exception e) { - getLogger().error("Failed to read JarFile {}, skipping...", resolvedArtifact, e); - throw new RuntimeException(e); - } - }); - Instant after = Instant.now(); - - getLogger().info("Checked {} classes from {} dependencies for uniqueness ({}ms)", - classToJarMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); - - return classToJarMap; - } - /** * This only exists to convince gradle this task is incremental. */ @@ -178,16 +117,4 @@ private void writeResultFile(boolean success) { throw new RuntimeException("Unable to write boolean result file", e); } } - - /** - * Implemented here so we don't pull in guava. Note that CopyOnWriteArrayList is threadsafe - * so we can parallelize elsewhere. - */ - private static void addToMultiMap(Map> multiMap, K key, V value) { - multiMap.compute(key, (unused, collection) -> { - Collection newCollection = collection != null ? collection : new CopyOnWriteArrayList<>(); - newCollection.add(value); - return newCollection; - }); - } } diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java new file mode 100644 index 000000000..ab6a8ea8e --- /dev/null +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -0,0 +1,113 @@ +/* + * (c) Copyright 2018 Palantir Technologies Inc. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.palantir.baseline.tasks; + +import java.io.File; +import java.time.Duration; +import java.time.Instant; +import java.util.Collection; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ModuleVersionIdentifier; +import org.gradle.api.artifacts.ResolvedArtifact; +import org.slf4j.Logger; + +public final class ClassUniquenessAnalyzer { + + private final Map> classToJarsMap = new HashMap<>(); + private final Map, Set> jarsToClasses = new HashMap<>(); + private final Logger log; + + public ClassUniquenessAnalyzer(Logger log) { + this.log = log; + } + + public void analyzeConfiguration(Configuration configuration) { + Instant before = Instant.now(); + + Set dependencies = configuration + .getResolvedConfiguration() + .getResolvedArtifacts(); + + Map> tempClassToJarMap = new HashMap<>(); + dependencies.stream().forEach(resolvedArtifact -> { + File file = resolvedArtifact.getFile(); + if (!file.exists()) { + log.info("Skipping non-existent jar {}: {}", resolvedArtifact, file); + return; + } + + try (JarFile jarFile = new JarFile(file)) { + Enumeration entries = jarFile.entries(); + while (entries.hasMoreElements()) { + JarEntry jarEntry = entries.nextElement(); + + if (!jarEntry.getName().endsWith(".class")) { + continue; + } + + multiMapPut(tempClassToJarMap, + jarEntry.getName().replaceAll("/", ".").replaceAll(".class", ""), + resolvedArtifact.getModuleVersion().getId()); + } + } catch (Exception e) { + log.error("Failed to read JarFile {}", resolvedArtifact, e); + throw new RuntimeException(e); + } + }); + + tempClassToJarMap.entrySet().stream() + .filter(entry -> entry.getValue().size() > 1) + .forEach(entry -> { + // add to the top level map + entry.getValue().forEach(value -> multiMapPut(classToJarsMap, entry.getKey(), value)); + + // add to the opposite direction index + multiMapPut(jarsToClasses, entry.getValue(), entry.getKey()); + }); + + Instant after = Instant.now(); + log.info("Checked {} classes from {} dependencies for uniqueness ({}ms)", + tempClassToJarMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); + } + + public Collection> getProblemJars() { + return classToJarsMap.values(); + } + + public Map, Set> jarsToClasses() { + return jarsToClasses; + } + + public Set getDuplicateClassesInProblemJars(Set problemJars) { + return jarsToClasses.get(problemJars); + } + + private static void multiMapPut(Map> map, K key, V value) { + map.compute(key, (unused, collection) -> { + Set newCollection = collection != null ? collection : new HashSet<>(); + newCollection.add(value); + return newCollection; + }); + } +} From c17aa36c8da13e97bfb903949b86968f27b169e5 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 15:40:57 +0100 Subject: [PATCH 32/43] Pull in guava --- gradle-baseline-java/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/gradle-baseline-java/build.gradle b/gradle-baseline-java/build.gradle index 8326d1d3c..4a3cd9257 100644 --- a/gradle-baseline-java/build.gradle +++ b/gradle-baseline-java/build.gradle @@ -10,6 +10,7 @@ repositories { dependencies { compile gradleApi() compile 'net.ltgt.gradle:gradle-errorprone-plugin' + compile 'com.google.guava:guava' testCompile gradleTestKit() testCompile 'com.netflix.nebula:nebula-test' // for better temp directory junit rule only From 73bf75fcff45042f47ee778555c3589b92507695 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 15:46:46 +0100 Subject: [PATCH 33/43] Tests pass --- .../tasks/CheckClassUniquenessTask.java | 11 ++-- .../tasks/ClassUniquenessAnalyzer.java | 55 ++++++++++++++----- ...lassUniquenessPluginIntegrationTest.groovy | 10 ++-- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 1b16899cd..fa9abb116 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -61,9 +61,12 @@ public void checkForDuplicateClasses() { if (!success) { analyzer.getProblemJars().forEach((problemJars) -> { - Set classes = analyzer.getDuplicateClassesInProblemJars(problemJars); - getLogger().error("Identically named classes found in {} jars ({}): {}", - problemJars.size(), problemJars, classes); + Set classes = analyzer.getSharedClassesInProblemJars(problemJars); + Set differingClasses = analyzer.getDifferingSharedClassesInProblemJars(problemJars); + getLogger().error( + "{} Identically named classes found in ({}), {} were identical implementations too: {}", + differingClasses.size(), problemJars, + classes.size() - differingClasses.size(), differingClasses); }); throw new IllegalStateException(String.format( @@ -86,7 +89,7 @@ private static String formatSummary(ClassUniquenessAnalyzer summary) { Map sortedTable = summary.jarsToClasses().entrySet().stream().collect(Collectors.toMap( entry -> entry.getKey().stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()), - entry -> entry.getValue().size(), + entry -> summary.getDifferingSharedClassesInProblemJars(entry.getKey()).size(), (first, second) -> { throw new RuntimeException("Unexpected collision: " + first + ", " + second); }, diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java index ab6a8ea8e..2f240fc9b 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -16,17 +16,24 @@ package com.palantir.baseline.tasks; +import static java.util.stream.Collectors.toSet; + +import com.google.common.hash.HashCode; +import com.google.common.hash.Hashing; +import com.google.common.hash.HashingInputStream; +import com.google.common.io.ByteStreams; import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; import java.time.Duration; import java.time.Instant; import java.util.Collection; -import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.jar.JarEntry; -import java.util.jar.JarFile; +import java.util.jar.JarInputStream; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ModuleVersionIdentifier; import org.gradle.api.artifacts.ResolvedArtifact; @@ -36,6 +43,7 @@ public final class ClassUniquenessAnalyzer { private final Map> classToJarsMap = new HashMap<>(); private final Map, Set> jarsToClasses = new HashMap<>(); + private final Map> classToHashCode = new HashMap<>(); private final Logger log; public ClassUniquenessAnalyzer(Logger log) { @@ -44,12 +52,13 @@ public ClassUniquenessAnalyzer(Logger log) { public void analyzeConfiguration(Configuration configuration) { Instant before = Instant.now(); - Set dependencies = configuration .getResolvedConfiguration() .getResolvedArtifacts(); Map> tempClassToJarMap = new HashMap<>(); + Map> tempClassToHashCode = new HashMap<>(); + dependencies.stream().forEach(resolvedArtifact -> { File file = resolvedArtifact.getFile(); if (!file.exists()) { @@ -57,20 +66,27 @@ public void analyzeConfiguration(Configuration configuration) { return; } - try (JarFile jarFile = new JarFile(file)) { - Enumeration entries = jarFile.entries(); - while (entries.hasMoreElements()) { - JarEntry jarEntry = entries.nextElement(); - - if (!jarEntry.getName().endsWith(".class")) { + try (FileInputStream fileInputStream = new FileInputStream(file); + JarInputStream jarInputStream = new JarInputStream(fileInputStream)) { + JarEntry entry; + while ((entry = jarInputStream.getNextJarEntry()) != null) { + if (entry.isDirectory() || !entry.getName().endsWith(".class")) { continue; } + String className = entry.getName().replaceAll("/", ".").replaceAll(".class", ""); + HashingInputStream inputStream = new HashingInputStream(Hashing.sha256(), jarInputStream); + ByteStreams.exhaust(inputStream); + multiMapPut(tempClassToJarMap, - jarEntry.getName().replaceAll("/", ".").replaceAll(".class", ""), + className, resolvedArtifact.getModuleVersion().getId()); + + multiMapPut(tempClassToHashCode, + className, + inputStream.hash()); } - } catch (Exception e) { + } catch (IOException e) { log.error("Failed to read JarFile {}", resolvedArtifact, e); throw new RuntimeException(e); } @@ -85,7 +101,14 @@ public void analyzeConfiguration(Configuration configuration) { // add to the opposite direction index multiMapPut(jarsToClasses, entry.getValue(), entry.getKey()); }); - + + // figure out which classes have differing hashes + tempClassToHashCode.entrySet().stream() + .filter(entry -> entry.getValue().size() > 1) + .forEach(entry -> { + entry.getValue().forEach(value -> multiMapPut(classToHashCode, entry.getKey(), value)); + }); + Instant after = Instant.now(); log.info("Checked {} classes from {} dependencies for uniqueness ({}ms)", tempClassToJarMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); @@ -99,10 +122,16 @@ public Map, Set> jarsToClasses() { return jarsToClasses; } - public Set getDuplicateClassesInProblemJars(Set problemJars) { + public Set getSharedClassesInProblemJars(Set problemJars) { return jarsToClasses.get(problemJars); } + public Set getDifferingSharedClassesInProblemJars(Set problemJars) { + return jarsToClasses.get(problemJars).stream() + .filter(classToHashCode::containsKey) + .collect(toSet()); + } + private static void multiMapPut(Map> map, K key, V value) { map.compute(key, (unused, collection) -> { Set newCollection = collection != null ? collection : new HashSet<>(); diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index e5abc5289..9fe7e8d81 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -64,11 +64,11 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { BuildResult result = with('checkClassUniqueness').buildAndFail() then: - result.getOutput().contains("Identically named classes found in 2 jars ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]): [javax.") + result.getOutput().contains("26 Identically named classes found in ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]), 0 were identical implementations too: [javax.") result.getOutput().contains("'runtime' contains multiple copies of identically named classes") - result.getOutput().contains("(4 classes) com.google.code.findbugs:annotations:3.0.1 net.jcip:jcip-annotations:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1"); - result.getOutput().contains("(35 classes) com.google.code.findbugs:jsr305:3.0.1 com.google.code.findbugs:annotations:3.0.1"); - result.getOutput().contains("(1 classes) com.palantir.tritium:tritium-api:0.9.0 com.palantir.tritium:tritium-core:0.9.0"); + result.getOutput().contains("(1 classes) com.google.code.findbugs:annotations:3.0.1 net.jcip:jcip-annotations:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1"); + result.getOutput().contains("(0 classes) com.google.code.findbugs:jsr305:3.0.1 com.google.code.findbugs:annotations:3.0.1"); + result.getOutput().contains("(0 classes) com.palantir.tritium:tritium-api:0.9.0 com.palantir.tritium:tritium-core:0.9.0"); result.getOutput().contains("(6 classes) commons-logging:commons-logging:1.2 org.slf4j:jcl-over-slf4j:1.7.25"); result.getOutput().contains("(26 classes) javax.servlet.jsp:jsp-api:2.1 javax.el:javax.el-api:3.0.0"); println result.getOutput() @@ -127,7 +127,7 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { then: BuildResult result = with('checkClassUniqueness').buildAndFail() - result.output.contains("Identically named classes found in 2 jars") + result.output.contains("26 Identically named classes found in ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0])") } def 'currently skips duplicates from user-authored code'() { From 3d5c25097ddece209109b7ef6e3c4e58783612f0 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 15:48:35 +0100 Subject: [PATCH 34/43] Checkstyle --- .../baseline/plugins/BaselineClassUniquenessPlugin.java | 3 +-- .../baseline/tasks/CheckClassUniquenessTask.java | 9 ++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java index 3be5c6282..c42b8c24a 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java @@ -19,11 +19,10 @@ import com.palantir.baseline.tasks.CheckClassUniquenessTask; import org.gradle.api.Project; -@SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle public class BaselineClassUniquenessPlugin extends AbstractBaselinePlugin { @Override - public void apply(Project project) { + public final void apply(Project project) { project.getPlugins().withId("java", plugin -> { project.getTasks().create("checkClassUniqueness", CheckClassUniquenessTask.class, task -> { task.setConfiguration(project.getConfigurations().getByName("runtime")); diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index fa9abb116..60534b60d 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -33,7 +33,6 @@ import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; -@SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle public class CheckClassUniquenessTask extends DefaultTask { private Configuration configuration; @@ -44,16 +43,16 @@ public CheckClassUniquenessTask() { } @Input - public Configuration getConfiguration() { + public final Configuration getConfiguration() { return configuration; } - public void setConfiguration(Configuration configuration) { + public final void setConfiguration(Configuration configuration) { this.configuration = configuration; } @TaskAction - public void checkForDuplicateClasses() { + public final void checkForDuplicateClasses() { ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer(getLogger()); analyzer.analyzeConfiguration(getConfiguration()); boolean success = analyzer.getProblemJars().isEmpty(); @@ -105,7 +104,7 @@ private static String formatSummary(ClassUniquenessAnalyzer summary) { * This only exists to convince gradle this task is incremental. */ @OutputFile - public File getResultFile() { + public final File getResultFile() { return getProject().getBuildDir().toPath() .resolve(Paths.get("uniqueClassNames", configuration.getName())) .toFile(); From 778c4fae7e4dc7d2f46522e36133c7293f79b63b Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 15:50:41 +0100 Subject: [PATCH 35/43] Some CR tweaks --- README.md | 4 ++-- .../palantir/baseline/tasks/CheckClassUniquenessTask.java | 3 +-- .../palantir/baseline/tasks/ClassUniquenessAnalyzer.java | 8 ++++---- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index ca3996630..5a6748a1e 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ repositories { apply plugin: 'com.palantir.baseline-config' dependencies { - // Adds a dependency on the Baseline configuration files. Typically use + // Adds a dependency on the Baseline configuration files. Typically use // the same version as the plugin itself. baseline "com.palantir.baseline:gradle-baseline-java-config:@zip" } @@ -259,7 +259,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c ### Class Uniqueness Plugin (com.palantir.baseline-class-uniqueness) -Run `./gradlew checkClassUniqueness` to scan all jars on the `testRuntime` classpath for identically named classes. +Run `./gradlew checkClassUniqueness` to scan all jars on the `runtime` classpath for identically named classes. This task will run automatically as part of `./gradlew build`. diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 60534b60d..f81c4118d 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -71,8 +71,7 @@ public final void checkForDuplicateClasses() { throw new IllegalStateException(String.format( "'%s' contains multiple copies of identically named classes - " + "this may cause different runtime behaviour depending on classpath ordering.\n" - + "To resolve this, try excluding one of the following jars, " - + "changing a version or shadowing:\n\n%s", + + "To resolve this, try excluding one of the following jars:\n\n%s", configuration.getName(), formatSummary(analyzer) )); diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java index 2f240fc9b..4d25168f5 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -56,7 +56,7 @@ public void analyzeConfiguration(Configuration configuration) { .getResolvedConfiguration() .getResolvedArtifacts(); - Map> tempClassToJarMap = new HashMap<>(); + Map> tempClassToJarsMap = new HashMap<>(); Map> tempClassToHashCode = new HashMap<>(); dependencies.stream().forEach(resolvedArtifact -> { @@ -78,7 +78,7 @@ public void analyzeConfiguration(Configuration configuration) { HashingInputStream inputStream = new HashingInputStream(Hashing.sha256(), jarInputStream); ByteStreams.exhaust(inputStream); - multiMapPut(tempClassToJarMap, + multiMapPut(tempClassToJarsMap, className, resolvedArtifact.getModuleVersion().getId()); @@ -92,7 +92,7 @@ public void analyzeConfiguration(Configuration configuration) { } }); - tempClassToJarMap.entrySet().stream() + tempClassToJarsMap.entrySet().stream() .filter(entry -> entry.getValue().size() > 1) .forEach(entry -> { // add to the top level map @@ -111,7 +111,7 @@ public void analyzeConfiguration(Configuration configuration) { Instant after = Instant.now(); log.info("Checked {} classes from {} dependencies for uniqueness ({}ms)", - tempClassToJarMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); + tempClassToJarsMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); } public Collection> getProblemJars() { From 2bdcc50c087fa20437378a465f0f9a4b430b9b91 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 16:05:52 +0100 Subject: [PATCH 36/43] Add some javadoc --- .../tasks/ClassUniquenessAnalyzer.java | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java index 4d25168f5..1e399271c 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -34,6 +34,7 @@ import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarInputStream; +import java.util.stream.Collectors; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ModuleVersionIdentifier; import org.gradle.api.artifacts.ResolvedArtifact; @@ -43,7 +44,7 @@ public final class ClassUniquenessAnalyzer { private final Map> classToJarsMap = new HashMap<>(); private final Map, Set> jarsToClasses = new HashMap<>(); - private final Map> classToHashCode = new HashMap<>(); + private final Map> classToHashCodes = new HashMap<>(); private final Logger log; public ClassUniquenessAnalyzer(Logger log) { @@ -57,7 +58,7 @@ public void analyzeConfiguration(Configuration configuration) { .getResolvedArtifacts(); Map> tempClassToJarsMap = new HashMap<>(); - Map> tempClassToHashCode = new HashMap<>(); + Map> tempClassToHashCodes = new HashMap<>(); dependencies.stream().forEach(resolvedArtifact -> { File file = resolvedArtifact.getFile(); @@ -82,7 +83,7 @@ public void analyzeConfiguration(Configuration configuration) { className, resolvedArtifact.getModuleVersion().getId()); - multiMapPut(tempClassToHashCode, + multiMapPut(tempClassToHashCodes, className, inputStream.hash()); } @@ -103,10 +104,10 @@ public void analyzeConfiguration(Configuration configuration) { }); // figure out which classes have differing hashes - tempClassToHashCode.entrySet().stream() + tempClassToHashCodes.entrySet().stream() .filter(entry -> entry.getValue().size() > 1) .forEach(entry -> { - entry.getValue().forEach(value -> multiMapPut(classToHashCode, entry.getKey(), value)); + entry.getValue().forEach(value -> multiMapPut(classToHashCodes, entry.getKey(), value)); }); Instant after = Instant.now(); @@ -114,6 +115,11 @@ public void analyzeConfiguration(Configuration configuration) { tempClassToJarsMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); } + /** + * Any groups jars that all contain some identically named classes. + * Note: may contain non-scary duplicates - class files which are 100% identical, so their + * clashing name doesn't have any effect. + */ public Collection> getProblemJars() { return classToJarsMap.values(); } @@ -122,13 +128,19 @@ public Map, Set> jarsToClasses() { return jarsToClasses; } - public Set getSharedClassesInProblemJars(Set problemJars) { + /** + * Class names that appear in all of the given jars. + */ + public Set getSharedClassesInProblemJars(Collection problemJars) { return jarsToClasses.get(problemJars); } - public Set getDifferingSharedClassesInProblemJars(Set problemJars) { - return jarsToClasses.get(problemJars).stream() - .filter(classToHashCode::containsKey) + /** + * Class names which appear in all of the given jars and also do not have identical implementations. + */ + public Set getDifferingSharedClassesInProblemJars(Collection problemJars) { + return getSharedClassesInProblemJars(problemJars).stream() + .filter(classToHashCodes::containsKey) .collect(toSet()); } From 087003ff336e85e0d11ae95285587059def2401f Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 16:15:55 +0100 Subject: [PATCH 37/43] Don't bother sorting the table --- .../tasks/CheckClassUniquenessTask.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index f81c4118d..91bf07ad3 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -21,10 +21,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; +import java.util.Collection; import java.util.Comparator; -import java.util.Map; import java.util.Set; -import java.util.TreeMap; import java.util.stream.Collectors; import org.gradle.api.DefaultTask; import org.gradle.api.artifacts.Configuration; @@ -79,23 +78,25 @@ public final void checkForDuplicateClasses() { } private static String formatSummary(ClassUniquenessAnalyzer summary) { - int maxLength = summary.jarsToClasses().keySet().stream().flatMap(Set::stream) + Collection> allProblemJars = summary.getProblemJars(); + + int maxLength = allProblemJars.stream().flatMap(Set::stream) .map(ModuleVersionIdentifier::toString) .map(String::length) .max(Comparator.naturalOrder()).get(); String format = "%-" + (maxLength + 1) + "s"; - Map sortedTable = summary.jarsToClasses().entrySet().stream().collect(Collectors.toMap( - entry -> entry.getKey().stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()), - entry -> summary.getDifferingSharedClassesInProblemJars(entry.getKey()).size(), - (first, second) -> { - throw new RuntimeException("Unexpected collision: " + first + ", " + second); - }, - TreeMap::new)); - StringBuilder builder = new StringBuilder(); - sortedTable.forEach((jars, classes) -> - builder.append(String.format("\t%-14s", "(" + classes + " classes) ") + jars + "\n")); + + allProblemJars.forEach(problemJars -> { + int count = summary.getDifferingSharedClassesInProblemJars(problemJars).size(); + String countColumn = String.format("\t%-14s", "(" + count + " classes) "); + builder.append(countColumn); + + String jars = problemJars.stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()); + builder.append(jars); + }); + return builder.toString(); } From 2ac652670a92d77bf099be2a07b782d5ae8664ea Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 16:39:24 +0100 Subject: [PATCH 38/43] Only report classes with differing impls --- .../tasks/CheckClassUniquenessTask.java | 6 ++-- .../tasks/ClassUniquenessAnalyzer.java | 32 +++++++++++-------- ...lassUniquenessPluginIntegrationTest.groovy | 2 -- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 91bf07ad3..4a3d02e11 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -78,8 +78,8 @@ public final void checkForDuplicateClasses() { } private static String formatSummary(ClassUniquenessAnalyzer summary) { - Collection> allProblemJars = summary.getProblemJars(); - + Collection> allProblemJars = summary.getDifferingProblemJars(); + int maxLength = allProblemJars.stream().flatMap(Set::stream) .map(ModuleVersionIdentifier::toString) .map(String::length) @@ -95,6 +95,8 @@ private static String formatSummary(ClassUniquenessAnalyzer summary) { String jars = problemJars.stream().map(jar -> String.format(format, jar)).collect(Collectors.joining()); builder.append(jars); + + builder.append('\n'); }); return builder.toString(); diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java index 1e399271c..11ae01861 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -42,7 +42,7 @@ public final class ClassUniquenessAnalyzer { - private final Map> classToJarsMap = new HashMap<>(); + private final Map> classToJars = new HashMap<>(); private final Map, Set> jarsToClasses = new HashMap<>(); private final Map> classToHashCodes = new HashMap<>(); private final Logger log; @@ -57,7 +57,7 @@ public void analyzeConfiguration(Configuration configuration) { .getResolvedConfiguration() .getResolvedArtifacts(); - Map> tempClassToJarsMap = new HashMap<>(); + Map> tempClassToJars = new HashMap<>(); Map> tempClassToHashCodes = new HashMap<>(); dependencies.stream().forEach(resolvedArtifact -> { @@ -79,7 +79,7 @@ public void analyzeConfiguration(Configuration configuration) { HashingInputStream inputStream = new HashingInputStream(Hashing.sha256(), jarInputStream); ByteStreams.exhaust(inputStream); - multiMapPut(tempClassToJarsMap, + multiMapPut(tempClassToJars, className, resolvedArtifact.getModuleVersion().getId()); @@ -93,11 +93,11 @@ public void analyzeConfiguration(Configuration configuration) { } }); - tempClassToJarsMap.entrySet().stream() + tempClassToJars.entrySet().stream() .filter(entry -> entry.getValue().size() > 1) .forEach(entry -> { - // add to the top level map - entry.getValue().forEach(value -> multiMapPut(classToJarsMap, entry.getKey(), value)); + // we have determined this is a duplicate, so save it in the top level map + entry.getValue().forEach(value -> multiMapPut(classToJars, entry.getKey(), value)); // add to the opposite direction index multiMapPut(jarsToClasses, entry.getValue(), entry.getKey()); @@ -112,7 +112,7 @@ public void analyzeConfiguration(Configuration configuration) { Instant after = Instant.now(); log.info("Checked {} classes from {} dependencies for uniqueness ({}ms)", - tempClassToJarsMap.size(), dependencies.size(), Duration.between(before, after).toMillis()); + tempClassToJars.size(), dependencies.size(), Duration.between(before, after).toMillis()); } /** @@ -121,11 +121,7 @@ public void analyzeConfiguration(Configuration configuration) { * clashing name doesn't have any effect. */ public Collection> getProblemJars() { - return classToJarsMap.values(); - } - - public Map, Set> jarsToClasses() { - return jarsToClasses; + return classToJars.values(); } /** @@ -136,7 +132,17 @@ public Set getSharedClassesInProblemJars(Collection> getDifferingProblemJars() { + return classToJars.values() + .stream() + .filter(jars -> getDifferingSharedClassesInProblemJars(jars).size() > 0) + .collect(Collectors.toSet()); + } + + /** + * Class names which appear in all of the given jars and also have non-identical implementations. */ public Set getDifferingSharedClassesInProblemJars(Collection problemJars) { return getSharedClassesInProblemJars(problemJars).stream() diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index 9fe7e8d81..39b4bd73f 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -67,8 +67,6 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { result.getOutput().contains("26 Identically named classes found in ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]), 0 were identical implementations too: [javax.") result.getOutput().contains("'runtime' contains multiple copies of identically named classes") result.getOutput().contains("(1 classes) com.google.code.findbugs:annotations:3.0.1 net.jcip:jcip-annotations:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1"); - result.getOutput().contains("(0 classes) com.google.code.findbugs:jsr305:3.0.1 com.google.code.findbugs:annotations:3.0.1"); - result.getOutput().contains("(0 classes) com.palantir.tritium:tritium-api:0.9.0 com.palantir.tritium:tritium-core:0.9.0"); result.getOutput().contains("(6 classes) commons-logging:commons-logging:1.2 org.slf4j:jcl-over-slf4j:1.7.25"); result.getOutput().contains("(26 classes) javax.servlet.jsp:jsp-api:2.1 javax.el:javax.el-api:3.0.0"); println result.getOutput() From f3e98d40ae89f15827709b10a5583eb6b670ee92 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 16:50:49 +0100 Subject: [PATCH 39/43] Ensure we only log classes with different impls --- .../baseline/tasks/CheckClassUniquenessTask.java | 11 ++++------- ...aselineClassUniquenessPluginIntegrationTest.groovy | 4 ++-- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java index 4a3d02e11..4db3b37c8 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckClassUniquenessTask.java @@ -54,17 +54,14 @@ public final void setConfiguration(Configuration configuration) { public final void checkForDuplicateClasses() { ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer(getLogger()); analyzer.analyzeConfiguration(getConfiguration()); - boolean success = analyzer.getProblemJars().isEmpty(); + boolean success = analyzer.getDifferingProblemJars().isEmpty(); writeResultFile(success); if (!success) { - analyzer.getProblemJars().forEach((problemJars) -> { - Set classes = analyzer.getSharedClassesInProblemJars(problemJars); + analyzer.getDifferingProblemJars().forEach((problemJars) -> { Set differingClasses = analyzer.getDifferingSharedClassesInProblemJars(problemJars); - getLogger().error( - "{} Identically named classes found in ({}), {} were identical implementations too: {}", - differingClasses.size(), problemJars, - classes.size() - differingClasses.size(), differingClasses); + getLogger().error("{} Identically named classes with differing impls found in {}: {}", + differingClasses.size(), problemJars, differingClasses); }); throw new IllegalStateException(String.format( diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index 39b4bd73f..eb571580f 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -64,7 +64,7 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { BuildResult result = with('checkClassUniqueness').buildAndFail() then: - result.getOutput().contains("26 Identically named classes found in ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]), 0 were identical implementations too: [javax.") + result.getOutput().contains("6 Identically named classes with differing impls found in [commons-logging:commons-logging:1.2, org.slf4j:jcl-over-slf4j:1.7.25]: [org.") result.getOutput().contains("'runtime' contains multiple copies of identically named classes") result.getOutput().contains("(1 classes) com.google.code.findbugs:annotations:3.0.1 net.jcip:jcip-annotations:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1"); result.getOutput().contains("(6 classes) commons-logging:commons-logging:1.2 org.slf4j:jcl-over-slf4j:1.7.25"); @@ -125,7 +125,7 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { then: BuildResult result = with('checkClassUniqueness').buildAndFail() - result.output.contains("26 Identically named classes found in ([javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0])") + result.output.contains("26 Identically named classes with differing impls found in [javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]: [javax.") } def 'currently skips duplicates from user-authored code'() { From 452a0b61c8c2cd73c224bf16daec82924986b9d0 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 16:55:19 +0100 Subject: [PATCH 40/43] More minimal tests --- README.md | 2 +- ...lassUniquenessPluginIntegrationTest.groovy | 27 ++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 5a6748a1e..f6681d344 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ repositories { apply plugin: 'com.palantir.baseline-config' dependencies { - // Adds a dependency on the Baseline configuration files. Typically use + // Adds a dependency on the Baseline configuration files. Typically use // the same version as the plugin itself. baseline "com.palantir.baseline:gradle-baseline-java-config:@zip" } diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy index eb571580f..c4cdf2ed3 100644 --- a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy +++ b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineClassUniquenessPluginIntegrationTest.groovy @@ -53,25 +53,32 @@ class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest { dependencies { compile group: 'javax.el', name: 'javax.el-api', version: '3.0.0' compile group: 'javax.servlet.jsp', name: 'jsp-api', version: '2.1' - compile 'com.google.code.findbugs:annotations:3.0.0' - compile 'com.github.stephenc.jcip:jcip-annotations:1.0-1' - compile 'commons-logging:commons-logging:1.2' - compile 'org.slf4j:jcl-over-slf4j:1.7.25' - compile 'com.palantir.tritium:tritium-api:0.9.0' - compile 'com.palantir.tritium:tritium-core:0.9.0' } """.stripIndent() BuildResult result = with('checkClassUniqueness').buildAndFail() then: - result.getOutput().contains("6 Identically named classes with differing impls found in [commons-logging:commons-logging:1.2, org.slf4j:jcl-over-slf4j:1.7.25]: [org.") + result.output.contains("26 Identically named classes with differing impls found in [javax.servlet.jsp:jsp-api:2.1, javax.el:javax.el-api:3.0.0]: [javax.") result.getOutput().contains("'runtime' contains multiple copies of identically named classes") - result.getOutput().contains("(1 classes) com.google.code.findbugs:annotations:3.0.1 net.jcip:jcip-annotations:1.0 com.github.stephenc.jcip:jcip-annotations:1.0-1"); - result.getOutput().contains("(6 classes) commons-logging:commons-logging:1.2 org.slf4j:jcl-over-slf4j:1.7.25"); - result.getOutput().contains("(26 classes) javax.servlet.jsp:jsp-api:2.1 javax.el:javax.el-api:3.0.0"); + result.getOutput().contains("(26 classes) javax.servlet.jsp:jsp-api:2.1 javax.el:javax.el-api:3.0.0"); println result.getOutput() } + + def 'ignores duplicates when the implementations are identical'() { + when: + buildFile << standardBuildFile + buildFile << """ + dependencies { + compile 'com.palantir.tritium:tritium-api:0.9.0' + compile 'com.palantir.tritium:tritium-core:0.9.0' + } + """.stripIndent() + + then: + with('checkClassUniqueness').build() + } + def 'task should be up-to-date when classpath is unchanged'() { when: buildFile << standardBuildFile From 4526967a3e28c2ed5c10a0de58e3c134f7d912ba Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 17:03:46 +0100 Subject: [PATCH 41/43] Javadoc mentioning netflix's impl --- .../baseline/plugins/BaselineClassUniquenessPlugin.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java index c42b8c24a..dfcc1cf5e 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineClassUniquenessPlugin.java @@ -19,6 +19,13 @@ import com.palantir.baseline.tasks.CheckClassUniquenessTask; import org.gradle.api.Project; +/** + * This plugin is similar to https://github.com/nebula-plugins/gradle-lint-plugin/wiki/Duplicate-Classes-Rule + * but goes one step further and actually hashes any identically named classfiles to figure out if they're + * completely identical (and therefore safely interchangeable). + * + * The task only fails if it finds classes which have the same name but different implementations. + */ public class BaselineClassUniquenessPlugin extends AbstractBaselinePlugin { @Override From 4640a4d202b6bdbe0bf7a0e0f8076cee4d371b45 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 17:12:03 +0100 Subject: [PATCH 42/43] Be better at programming --- .../tasks/ClassUniquenessAnalyzer.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java index 11ae01861..402efc71f 100644 --- a/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java +++ b/gradle-baseline-java/src/main/groovy/com/palantir/baseline/tasks/ClassUniquenessAnalyzer.java @@ -42,7 +42,6 @@ public final class ClassUniquenessAnalyzer { - private final Map> classToJars = new HashMap<>(); private final Map, Set> jarsToClasses = new HashMap<>(); private final Map> classToHashCodes = new HashMap<>(); private final Logger log; @@ -57,7 +56,9 @@ public void analyzeConfiguration(Configuration configuration) { .getResolvedConfiguration() .getResolvedArtifacts(); - Map> tempClassToJars = new HashMap<>(); + // we use these temporary maps to accumulate information as we process each jar, + // so they may include singletons which we filter out later + Map> classToJars = new HashMap<>(); Map> tempClassToHashCodes = new HashMap<>(); dependencies.stream().forEach(resolvedArtifact -> { @@ -79,7 +80,7 @@ public void analyzeConfiguration(Configuration configuration) { HashingInputStream inputStream = new HashingInputStream(Hashing.sha256(), jarInputStream); ByteStreams.exhaust(inputStream); - multiMapPut(tempClassToJars, + multiMapPut(classToJars, className, resolvedArtifact.getModuleVersion().getId()); @@ -93,26 +94,20 @@ public void analyzeConfiguration(Configuration configuration) { } }); - tempClassToJars.entrySet().stream() + // discard all the classes that only come from one jar - these are completely safe! + classToJars.entrySet().stream() .filter(entry -> entry.getValue().size() > 1) - .forEach(entry -> { - // we have determined this is a duplicate, so save it in the top level map - entry.getValue().forEach(value -> multiMapPut(classToJars, entry.getKey(), value)); - - // add to the opposite direction index - multiMapPut(jarsToClasses, entry.getValue(), entry.getKey()); - }); + .forEach(entry -> multiMapPut(jarsToClasses, entry.getValue(), entry.getKey())); // figure out which classes have differing hashes tempClassToHashCodes.entrySet().stream() .filter(entry -> entry.getValue().size() > 1) - .forEach(entry -> { - entry.getValue().forEach(value -> multiMapPut(classToHashCodes, entry.getKey(), value)); - }); + .forEach(entry -> + entry.getValue().forEach(value -> multiMapPut(classToHashCodes, entry.getKey(), value))); Instant after = Instant.now(); log.info("Checked {} classes from {} dependencies for uniqueness ({}ms)", - tempClassToJars.size(), dependencies.size(), Duration.between(before, after).toMillis()); + classToJars.size(), dependencies.size(), Duration.between(before, after).toMillis()); } /** @@ -121,7 +116,7 @@ public void analyzeConfiguration(Configuration configuration) { * clashing name doesn't have any effect. */ public Collection> getProblemJars() { - return classToJars.values(); + return jarsToClasses.keySet(); } /** @@ -135,7 +130,7 @@ public Set getSharedClassesInProblemJars(Collection> getDifferingProblemJars() { - return classToJars.values() + return getProblemJars() .stream() .filter(jars -> getDifferingSharedClassesInProblemJars(jars).size() > 0) .collect(Collectors.toSet()); From caaa580eb4ded615f663ea96329fbdddd54ceaf4 Mon Sep 17 00:00:00 2001 From: Dan Fox Date: Wed, 2 May 2018 21:00:01 +0100 Subject: [PATCH 43/43] Remove duplicate guava dep --- gradle-baseline-java/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/gradle-baseline-java/build.gradle b/gradle-baseline-java/build.gradle index 4a3cd9257..573cd48b0 100644 --- a/gradle-baseline-java/build.gradle +++ b/gradle-baseline-java/build.gradle @@ -14,7 +14,6 @@ dependencies { testCompile gradleTestKit() testCompile 'com.netflix.nebula:nebula-test' // for better temp directory junit rule only - testCompile 'com.google.guava:guava' testCompile 'net.lingala.zip4j:zip4j' testCompile 'org.apache.commons:commons-io' }