Skip to content
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
77d0413
Added BaselineClasspathConflict plugin
Mar 16, 2018
8c38f0c
Rename to BaselineClasspathDuplicatesPlugin
iamdanfox Apr 30, 2018
d94f939
Add meta info properties file
iamdanfox Apr 30, 2018
7941840
silentConflict -> checkClasspathIsDuplicateFree
iamdanfox Apr 30, 2018
19b30c2
Factor out logic to a new CheckUniqueClassNamesTask
iamdanfox Apr 30, 2018
5ce2ee8
apply plugin order shouldn't matter
iamdanfox Apr 30, 2018
c9c3b55
Checkstyle
iamdanfox Apr 30, 2018
0919961
Ugh groovy
iamdanfox Apr 30, 2018
9d0745b
Add some magic publishing stuff
iamdanfox Apr 30, 2018
a81b6b3
Try removing the 'final' modifier
iamdanfox Apr 30, 2018
b66f48c
Convert to Java
iamdanfox Apr 30, 2018
a1275df
Remove 'final' modifier
iamdanfox Apr 30, 2018
ed4425e
Verify up-to-date behaviour
iamdanfox Apr 30, 2018
5ea80f6
Slightly different syntax
iamdanfox Apr 30, 2018
a1b2156
Don't use org.gradle.internal.impldep HashMultimap
iamdanfox Apr 30, 2018
982e92d
Checkstyle
iamdanfox Apr 30, 2018
bcf86e8
Test verifies duplicates can be detected
iamdanfox Apr 30, 2018
cfa7aec
test for up-to-date
iamdanfox Apr 30, 2018
830af37
Sweet human readable output
iamdanfox Apr 30, 2018
7784164
Factor out the crawling
iamdanfox Apr 30, 2018
9e9ab49
Get ready for parallel
iamdanfox Apr 30, 2018
4591774
parallelStream over dependencies
iamdanfox Apr 30, 2018
ea7cd0b
Test case for project dependencies
iamdanfox Apr 30, 2018
67264ff
Rename to 'com.palantir.baseline-class-uniqueness'
iamdanfox Apr 30, 2018
27b793c
README
iamdanfox Apr 30, 2018
8692c46
Don't enable straight away for everyone
iamdanfox Apr 30, 2018
e676bb2
Add sweet formatted table with count of offending classes
iamdanfox May 1, 2018
94c6b38
Sort table nicely
iamdanfox May 1, 2018
4f1f8c8
Check runtime instead of testRuntime by default
iamdanfox May 1, 2018
f6b4ebd
More comprehensive test
iamdanfox May 1, 2018
9f4963d
Factor out ClassUniquenessAnalyzer
iamdanfox May 1, 2018
c17aa36
Pull in guava
iamdanfox May 2, 2018
73bf75f
Tests pass
iamdanfox May 2, 2018
3d5c250
Checkstyle
iamdanfox May 2, 2018
778c4fa
Some CR tweaks
iamdanfox May 2, 2018
2bdcc50
Add some javadoc
iamdanfox May 2, 2018
087003f
Don't bother sorting the table
iamdanfox May 2, 2018
2ac6526
Only report classes with differing impls
iamdanfox May 2, 2018
f3e98d4
Ensure we only log classes with different impls
iamdanfox May 2, 2018
452a0b6
More minimal tests
iamdanfox May 2, 2018
4526967
Javadoc mentioning netflix's impl
iamdanfox May 2, 2018
4640a4d
Be better at programming
iamdanfox May 2, 2018
caaa580
Remove duplicate guava dep
iamdanfox May 2, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the plugin configures the task with the runtime configuration

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
Expand Down
4 changes: 4 additions & 0 deletions gradle-baseline-java/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ pluginBundle {
id = 'com.palantir.baseline-idea'
displayName = 'Palantir Baseline IntelliJ Plugin'
}
baselineClassUniquenessPlugin {
id = 'com.palantir.baseline-class-uniqueness'
displayName = 'Palantir Baseline Class Uniqueness Plugin'
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@ class Baseline implements Plugin<Project> {
project.plugins.apply BaselineEclipse
project.plugins.apply BaselineIdea
project.plugins.apply BaselineErrorProne

// TODO(dfox): enable this when it has been validated on a few real projects
// project.plugins.apply BaselineClassUniquenessPlugin
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* (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 com.palantir.baseline.tasks.CheckClassUniquenessTask;
import org.gradle.api.Project;

@SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can make the apply method final and get rid of this suppression

public class BaselineClassUniquenessPlugin extends AbstractBaselinePlugin {

@Override
public void apply(Project project) {
project.getPlugins().withId("java", plugin -> {
project.getTasks().create("checkClassUniqueness", CheckClassUniquenessTask.class, task -> {
task.setConfiguration(project.getConfigurations().getByName("testRuntime"));
project.getTasks().getByName("check").dependsOn(task);
});
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
/*
* (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.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;
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;

@SuppressWarnings("checkstyle:designforextension") // making this 'final' breaks gradle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, should be able to make the individual methods final. also, maybe include some javadoc mentioning the gradle lint rule and differences in our implementation (e.g. hashing of class files)?

public class CheckClassUniquenessTask extends DefaultTask {

private Configuration configuration;

public CheckClassUniquenessTask() {
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() {
Map<String, Collection<ModuleVersionIdentifier>> classToJarMap = constructClassNameToSourceJarMap();

Map<Set<ModuleVersionIdentifier>, Collection<String>> jarsToOverlappingClasses = new HashMap<>();
classToJarMap.forEach((className, sourceJars) -> {
if (sourceJars.size() > 1) {
addToMultiMap(jarsToOverlappingClasses, new HashSet<>(sourceJars), className);
}
});

boolean success = jarsToOverlappingClasses.isEmpty();
writeResultFile(success);

if (!success) {
jarsToOverlappingClasses.forEach((problemJars, classes) -> {
getLogger().error("Identically named classes found in {} jars ({}): {}",
problemJars.size(), problemJars, classes);
});

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\t%s",
configuration.getName(),
jarsToOverlappingClasses.keySet()
));
}
}

private Map<String, Collection<ModuleVersionIdentifier>> constructClassNameToSourceJarMap() {
Map<String, Collection<ModuleVersionIdentifier>> classToJarMap = new ConcurrentHashMap<>();

Instant before = Instant.now();
Set<ResolvedArtifact> 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<JarEntry> 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think this comment is strictly necessary -- I feel this is pretty standard/well understood for people who write gradle plugins

Copy link
Contributor Author

@iamdanfox iamdanfox May 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's helpful to reassure readers of this code that the file we write really is inconsequential and not magically wired to something elsewhere in the project.

*/
@OutputFile
public File getResultFile() {
return getProject().getBuildDir().toPath()
.resolve(Paths.get("uniqueClassNames", configuration.getName()))
.toFile();
}

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

/**
* Implemented here so we don't pull in guava. Note that CopyOnWriteArrayList is threadsafe
* so we can parallelize elsewhere.
*/
private static <K, V> void addToMultiMap(Map<K, Collection<V>> multiMap, K key, V value) {
multiMap.compute(key, (unused, collection) -> {
Collection<V> newCollection = collection != null ? collection : new CopyOnWriteArrayList<>();
newCollection.add(value);
return newCollection;
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
implementation-class=com.palantir.baseline.plugins.BaselineClassUniquenessPlugin
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* (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 java.nio.file.Files
import java.util.stream.Stream
import org.gradle.testkit.runner.BuildResult
import org.gradle.testkit.runner.TaskOutcome

class BaselineClassUniquenessPluginIntegrationTest extends AbstractPluginTest {

def standardBuildFile = """
plugins {
id 'java'
id 'com.palantir.baseline-class-uniqueness'
}
subprojects {
apply plugin: 'java'
}
repositories {
mavenCentral()
}
""".stripIndent()

def 'Task should run as part of :check'() {
when:
buildFile << standardBuildFile

then:
def result = with('check', '--stacktrace').build()
result.task(':checkClassUniqueness').outcome == TaskOutcome.SUCCESS
}

def 'detect duplicates in two external jars'() {
when:
buildFile << standardBuildFile
buildFile << """
dependencies {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd prefer to trim this test case down to a more minimal one

Copy link
Contributor Author

@iamdanfox iamdanfox May 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of entries here allows me to verify the sorted table works nicely.

EDIT have thrown away the complicated sorting and split these unit tests.

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('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")
}

def 'task should be up-to-date when classpath is unchanged'() {
when:
buildFile << standardBuildFile

then:
BuildResult result1 = with('checkClassUniqueness').build()
result1.task(':checkClassUniqueness').outcome == TaskOutcome.SUCCESS

BuildResult result = with('checkClassUniqueness').build()
result.task(':checkClassUniqueness').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('checkClassUniqueness', '--info').build()

then:
result.task(":checkClassUniqueness").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('checkClassUniqueness').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('checkClassUniqueness', '--info').build()
println result.getOutput()
result.task(":checkClassUniqueness").outcome == TaskOutcome.SUCCESS // ideally should should say failed!
}
}