Skip to content
Merged
Show file tree
Hide file tree
Changes from 42 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 `runtime` 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
Expand Down
5 changes: 5 additions & 0 deletions gradle-baseline-java/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ repositories {
dependencies {
compile gradleApi()
compile 'net.ltgt.gradle:gradle-errorprone-plugin'
compile 'com.google.guava:guava'
Copy link
Contributor

Choose a reason for hiding this comment

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

let's remove the testCompile dep in line 17


testCompile gradleTestKit()
testCompile 'com.netflix.nebula:nebula-test' // for better temp directory junit rule only
Expand Down Expand Up @@ -54,6 +55,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,40 @@
/*
* (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;

/**
* 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
* <i>completely</i> 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
public final void apply(Project project) {
project.getPlugins().withId("java", plugin -> {
project.getTasks().create("checkClassUniqueness", CheckClassUniquenessTask.class, task -> {
task.setConfiguration(project.getConfigurations().getByName("runtime"));
project.getTasks().getByName("check").dependsOn(task);
});
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* (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.util.Collection;
import java.util.Comparator;
import java.util.Set;
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.tasks.Input;
import org.gradle.api.tasks.OutputFile;
import org.gradle.api.tasks.TaskAction;

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 final Configuration getConfiguration() {
return configuration;
}

public final void setConfiguration(Configuration configuration) {
this.configuration = configuration;
}

@TaskAction
public final void checkForDuplicateClasses() {
ClassUniquenessAnalyzer analyzer = new ClassUniquenessAnalyzer(getLogger());
analyzer.analyzeConfiguration(getConfiguration());
boolean success = analyzer.getDifferingProblemJars().isEmpty();
writeResultFile(success);

if (!success) {
analyzer.getDifferingProblemJars().forEach((problemJars) -> {
Set<String> differingClasses = analyzer.getDifferingSharedClassesInProblemJars(problemJars);
getLogger().error("{} Identically named classes with differing impls found in {}: {}",
differingClasses.size(), problemJars, differingClasses);
});

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:\n\n%s",
configuration.getName(),
formatSummary(analyzer)
));
}
}

private static String formatSummary(ClassUniquenessAnalyzer summary) {
Collection<Set<ModuleVersionIdentifier>> allProblemJars = summary.getDifferingProblemJars();

int maxLength = allProblemJars.stream().flatMap(Set::stream)
.map(ModuleVersionIdentifier::toString)
.map(String::length)
.max(Comparator.naturalOrder()).get();
String format = "%-" + (maxLength + 1) + "s";

StringBuilder builder = new StringBuilder();

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

builder.append('\n');
});

return builder.toString();
}

/**
* 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 final 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);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
/*
* (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 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.HashMap;
import java.util.HashSet;
import java.util.Map;
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;
import org.slf4j.Logger;

public final class ClassUniquenessAnalyzer {

private final Map<Set<ModuleVersionIdentifier>, Set<String>> jarsToClasses = new HashMap<>();
private final Map<String, Set<HashCode>> classToHashCodes = new HashMap<>();
private final Logger log;

public ClassUniquenessAnalyzer(Logger log) {
this.log = log;
}

public void analyzeConfiguration(Configuration configuration) {
Instant before = Instant.now();
Set<ResolvedArtifact> dependencies = configuration
.getResolvedConfiguration()
.getResolvedArtifacts();

// we use these temporary maps to accumulate information as we process each jar,
// so they may include singletons which we filter out later
Map<String, Set<ModuleVersionIdentifier>> classToJars = new HashMap<>();
Map<String, Set<HashCode>> tempClassToHashCodes = new HashMap<>();

dependencies.stream().forEach(resolvedArtifact -> {
File file = resolvedArtifact.getFile();
if (!file.exists()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

probably wanna filter out non-jars too to avoid spurious errors in line 92

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.

TBH I'd kinda like to just get an MVP released and get a few repos to pick it up and see what we encounter! The -sources.jar and -javadoc.jar thing doesn't seem to cause a problem because we only consider files ending in .class.

log.info("Skipping non-existent jar {}: {}", resolvedArtifact, file);
return;
}

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(classToJars,
className,
resolvedArtifact.getModuleVersion().getId());

multiMapPut(tempClassToHashCodes,
className,
inputStream.hash());
}
} catch (IOException e) {
log.error("Failed to read JarFile {}", resolvedArtifact, e);
Copy link
Contributor

Choose a reason for hiding this comment

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

throw new RuntimeException(e);
}
});

// discard all the classes that only come from one jar - these are completely safe!
classToJars.entrySet().stream()
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 we need this collection -- finding which entries of tempClassToHashCodes have value of size > 1 (which we're doing a few lines down) should be sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we delete classToJars then I don't really see how we can accumulate information about where a class came from and then derive the Map<Set<ModuleVersionIdentifier>, Set<String>> jarsToClasses we need to report problems?

People need to know which are the offending jars!

Copy link
Contributor

Choose a reason for hiding this comment

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

oh whoops I didn't read this closely enough

.filter(entry -> entry.getValue().size() > 1)
.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)));

Instant after = Instant.now();
log.info("Checked {} classes from {} dependencies for uniqueness ({}ms)",
classToJars.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<Set<ModuleVersionIdentifier>> getProblemJars() {
return jarsToClasses.keySet();
}

/**
* Class names that appear in all of the given jars.
*/
public Set<String> getSharedClassesInProblemJars(Collection<ModuleVersionIdentifier> problemJars) {
return jarsToClasses.get(problemJars);
}

/**
* Jars which contain identically named classes with non-identical implementations.
*/
public Collection<Set<ModuleVersionIdentifier>> getDifferingProblemJars() {
return getProblemJars()
.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<String> getDifferingSharedClassesInProblemJars(Collection<ModuleVersionIdentifier> problemJars) {
return getSharedClassesInProblemJars(problemJars).stream()
.filter(classToHashCodes::containsKey)
.collect(toSet());
}

private static <K, V> void multiMapPut(Map<K, Set<V>> map, K key, V value) {
map.compute(key, (unused, collection) -> {
Set<V> newCollection = collection != null ? collection : new HashSet<>();
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
Loading