diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckResultException.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckResultException.java index d7acd68a89..3e66a5ab7d 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckResultException.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckResultException.java @@ -22,7 +22,7 @@ *

The caller of the tool can tell the existence of linkage errors by checking the exit status of * the {@link LinkageCheckerMain}. */ -final class LinkageCheckResultException extends Exception { +public final class LinkageCheckResultException extends Exception { LinkageCheckResultException(int linkageErrorCount) { super( "Found " diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageChecker.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageChecker.java index 12dfc715de..4e4f74b572 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageChecker.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageChecker.java @@ -36,6 +36,7 @@ import java.util.Queue; import java.util.Set; import java.util.logging.Logger; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.apache.bcel.classfile.Field; import org.apache.bcel.classfile.FieldOrMethod; @@ -54,6 +55,7 @@ public class LinkageChecker { private final ImmutableList classPath; private final SymbolReferences symbolReferences; private final ClassReferenceGraph classReferenceGraph; + private final List sourceFilterList; private final ExcludedErrors excludedErrors; @VisibleForTesting @@ -66,7 +68,7 @@ public ClassReferenceGraph getClassReferenceGraph() { } public static LinkageChecker create(List classPath) throws IOException { - return create(classPath, ImmutableSet.copyOf(classPath), null); + return create(classPath, ImmutableSet.copyOf(classPath), ImmutableList.of(), null); } /** @@ -79,6 +81,7 @@ public static LinkageChecker create(List classPath) throws IOExc public static LinkageChecker create( List classPath, Iterable entryPoints, + List sourceFilterList, @Nullable Path exclusionFile) throws IOException { Preconditions.checkArgument(!classPath.isEmpty(), "The linkage classpath is empty."); @@ -93,6 +96,7 @@ public static LinkageChecker create( classPath, symbolReferenceMaps, classReferenceGraph, + sourceFilterList, ExcludedErrors.create(exclusionFile)); } @@ -129,13 +133,13 @@ public static LinkageChecker create(Bom bom, Path exclusionFile) List artifactsInBom = classpath.subList(0, managedDependencies.size()); ImmutableSet entryPoints = ImmutableSet.copyOf(artifactsInBom); - return LinkageChecker.create(classpath, entryPoints, exclusionFile); + return LinkageChecker.create(classpath, entryPoints, ImmutableList.of(), exclusionFile); } @VisibleForTesting LinkageChecker cloneWith(SymbolReferences newSymbolMaps) { return new LinkageChecker( - classDumper, classPath, newSymbolMaps, classReferenceGraph, excludedErrors); + classDumper, classPath, newSymbolMaps, classReferenceGraph, ImmutableList.of(), excludedErrors); } private LinkageChecker( @@ -143,14 +147,28 @@ private LinkageChecker( List classPath, SymbolReferences symbolReferenceMaps, ClassReferenceGraph classReferenceGraph, + List sourceFilterList, ExcludedErrors excludedErrors) { this.classDumper = Preconditions.checkNotNull(classDumper); this.classPath = ImmutableList.copyOf(classPath); this.classReferenceGraph = Preconditions.checkNotNull(classReferenceGraph); this.symbolReferences = Preconditions.checkNotNull(symbolReferenceMaps); + this.sourceFilterList = sourceFilterList; this.excludedErrors = Preconditions.checkNotNull(excludedErrors); } + /** + * Two artifacts are considered equal if only the Maven Coordinates (GAV) are equal. This is + * included instead of using Artifact.equals() because the `equals()` implementation + * of DefaultArtifact checks more fields than just the GAV Coordinates (also checks the classifier, + * file path, properties, etc are all equal). + */ + private boolean areArtifactsEquals(Artifact artifact1, Artifact artifact2) { + return artifact1.getGroupId().equals(artifact2.getGroupId()) + && artifact1.getArtifactId().equals(artifact2.getArtifactId()) + && artifact1.getVersion().equals(artifact2.getVersion()); + } + /** * Searches the classpath for linkage errors. * @@ -161,7 +179,21 @@ public ImmutableSet findLinkageProblems() throws IOException { ImmutableSet.Builder problemToClass = ImmutableSet.builder(); // This sourceClassFile is a source of references to other symbols. - for (ClassFile classFile : symbolReferences.getClassFiles()) { + Set classFiles = symbolReferences.getClassFiles(); + + // Filtering the classFiles from the JARs (instead of using the problem filter) has additional a few + // additional benefits. 1. Reduces the total amount of linkage references to match and 2. Doesn't require + // an exclusion file to know all the possible flaky or false positive problems + if (!sourceFilterList.isEmpty()) { + // Filter the list to only contain class files that come from the classes we are interested in. + // Run through each class file and check that the class file's corresponding artifact matches + // any artifact specified in the sourceFilterList + classFiles = classFiles.stream() + .filter(x -> sourceFilterList.stream() + .anyMatch(y -> areArtifactsEquals(x.getClassPathEntry().getArtifact(), y))) + .collect(Collectors.toSet()); + } + for (ClassFile classFile : classFiles) { ImmutableSet classSymbols = symbolReferences.getClassSymbols(classFile); for (ClassSymbol classSymbol : classSymbols) { if (classSymbol instanceof SuperClassSymbol) { @@ -202,7 +234,7 @@ public ImmutableSet findLinkageProblems() throws IOException { } } - for (ClassFile classFile : symbolReferences.getClassFiles()) { + for (ClassFile classFile : classFiles) { ImmutableSet methodSymbols = symbolReferences.getMethodSymbols(classFile); ImmutableSet classFileNames = classFile.getClassPathEntry().getFileNames(); for (MethodSymbol methodSymbol : methodSymbols) { @@ -215,7 +247,7 @@ public ImmutableSet findLinkageProblems() throws IOException { } } - for (ClassFile classFile : symbolReferences.getClassFiles()) { + for (ClassFile classFile : classFiles) { ImmutableSet fieldSymbols = symbolReferences.getFieldSymbols(classFile); ImmutableSet classFileNames = classFile.getClassPathEntry().getFileNames(); for (FieldSymbol fieldSymbol : fieldSymbols) { diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerArguments.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerArguments.java index 04a5556f86..1696fe095d 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerArguments.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerArguments.java @@ -25,6 +25,9 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; import org.apache.commons.cli.DefaultParser; @@ -124,6 +127,17 @@ private static Options configureOptions() { .build(); inputGroup.addOption(jarOption); + Option sourceFilter = + Option.builder("s") + .longOpt("source-filter") + .hasArgs() + .valueSeparator(',') + .desc("List of maven coordinates for artifacts (separated by ','). " + + "These are artifacts whose class files are searched as the source of the " + + "potential Linkage Errors.") + .build(); + options.addOption(sourceFilter); + Option repositoryOption = Option.builder("m") .longOpt("maven-repositories") @@ -277,4 +291,18 @@ Path getOutputExclusionFile() { } return null; } + + /** + * Returns a list of artifacts to search where Linkage Errors stem from. If the argument is not + * specified, return an empty List. + */ + List getSourceFilterArtifactList() { + if (commandLine.hasOption("s")) { + String[] mavenCoordinatesOption = commandLine.getOptionValues("s"); + return Arrays.stream(mavenCoordinatesOption) + .map(DefaultArtifact::new) + .collect(Collectors.toList()); + } + return ImmutableList.of(); + } } diff --git a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerMain.java b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerMain.java index 73102c3174..834b872cd2 100644 --- a/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerMain.java +++ b/dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerMain.java @@ -33,7 +33,7 @@ /** * A tool to find linkage errors in a class path. */ -class LinkageCheckerMain { +public class LinkageCheckerMain { /** * Forms a classpath from Maven coordinates or a list of jar files and reports linkage errors in @@ -133,7 +133,7 @@ private static Problems checkJarFiles( ImmutableSet entryPoints = ImmutableSet.copyOf(inputClassPath); LinkageChecker linkageChecker = LinkageChecker.create( - inputClassPath, entryPoints, linkageCheckerArguments.getInputExclusionFile()); + inputClassPath, entryPoints, ImmutableList.of(), linkageCheckerArguments.getInputExclusionFile()); ImmutableSet linkageProblems = findLinkageProblems(linkageChecker, linkageCheckerArguments.getReportOnlyReachable()); @@ -161,7 +161,7 @@ private static Problems checkArtifacts( LinkageChecker linkageChecker = LinkageChecker.create( - inputClassPath, entryPoints, linkageCheckerArguments.getInputExclusionFile()); + inputClassPath, entryPoints, linkageCheckerArguments.getSourceFilterArtifactList(), linkageCheckerArguments.getInputExclusionFile()); ImmutableSet linkageProblems = findLinkageProblems(linkageChecker, linkageCheckerArguments.getReportOnlyReachable()); diff --git a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ExclusionFilesIntegrationTest.java b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ExclusionFilesIntegrationTest.java index a0431398c5..c8bdf99fb4 100644 --- a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ExclusionFilesIntegrationTest.java +++ b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ExclusionFilesIntegrationTest.java @@ -58,7 +58,7 @@ public void testExclusion() classPathBuilder.resolve(ImmutableList.of(artifact), false, DependencyMediation.MAVEN); LinkageChecker linkagechecker = - LinkageChecker.create(classPathResult.getClassPath(), ImmutableList.of(), exclusionFile); + LinkageChecker.create(classPathResult.getClassPath(), ImmutableList.of(), ImmutableList.of(), exclusionFile); ImmutableSet linkageProblems = linkagechecker.findLinkageProblems(); Truth.assertThat(linkageProblems).isEmpty(); diff --git a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerTest.java b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerTest.java index df2912d2d5..96556c330f 100644 --- a/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerTest.java +++ b/dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerTest.java @@ -1340,4 +1340,32 @@ public void testProtectedMethodsOfObject() throws Exception { "has linkage errors that reference symbols on class")) .doesNotContain("java.lang.Object"); } + + @Test + public void testSourceFilter() throws InvalidVersionSpecificationException, IOException { + // BQ-Storage v3.9.3 contains 3 known binary incompatibilities with Protobuf-Java 4.x + DefaultArtifact sourceArtifact = new DefaultArtifact("com.google.cloud:google-cloud-bigquerystorage:3.9.3"); + ClassPathResult classPathResult = + new ClassPathBuilder() + .resolve( + ImmutableList.of( + sourceArtifact, + new DefaultArtifact("com.google.protobuf:protobuf-java:4.27.4"), + new DefaultArtifact("com.google.protobuf:protobuf-java-util:4.27.4")), + false, + DependencyMediation.MAVEN); + + ImmutableList classPath = classPathResult.getClassPath(); + LinkageChecker linkageChecker = LinkageChecker.create( + classPath, + ImmutableSet.copyOf(classPath), + ImmutableList.of(sourceArtifact), + null); + + // Without a source-filter to narrow down the linkage errors that stem from BQ-Storage, Linkage Checker + // would report 119 errors. Narrowing it down with the source filter will only report the 3 known binary + // incompatibilities + ImmutableSet problems = linkageChecker.findLinkageProblems(); + Truth.assertThat(problems.size()).isEqualTo(3); + } } diff --git a/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java b/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java index 81f102ad41..cd69e51907 100644 --- a/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java +++ b/enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java @@ -224,7 +224,7 @@ public void execute() throws EnforcerRuleException { // findLinkageProblems immediately after create. Path exclusionFile = this.exclusionFile == null ? null : Paths.get(this.exclusionFile); - LinkageChecker linkageChecker = LinkageChecker.create(classPath, entryPoints, exclusionFile); + LinkageChecker linkageChecker = LinkageChecker.create(classPath, entryPoints, ImmutableList.of(), exclusionFile); ImmutableSet linkageProblems = linkageChecker.findLinkageProblems(); if (reportOnlyReachable) { ClassReferenceGraph classReferenceGraph = linkageChecker.getClassReferenceGraph(); diff --git a/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java b/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java index fbb1cd2010..1526af1edf 100644 --- a/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java +++ b/gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java @@ -145,7 +145,7 @@ private boolean findLinkageErrors(Configuration configuration) throws IOExceptio } // TODO(suztomo): Specify correct entry points if reportOnlyReachable is true. - LinkageChecker linkageChecker = LinkageChecker.create(classPath, classPath, exclusionFile); + LinkageChecker linkageChecker = LinkageChecker.create(classPath, classPath, ImmutableList.of(), exclusionFile); ImmutableSet linkageProblems = linkageChecker.findLinkageProblems(); diff --git a/linkage-monitor/src/main/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitor.java b/linkage-monitor/src/main/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitor.java index 9e492a83d4..42cff216dd 100644 --- a/linkage-monitor/src/main/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitor.java +++ b/linkage-monitor/src/main/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitor.java @@ -294,7 +294,7 @@ private ImmutableSet run(String groupId, String artifactId) List entryPointJars = classpath.subList(0, snapshotManagedDependencies.size()); ImmutableSet problemsInSnapshot = - LinkageChecker.create(classpath, ImmutableSet.copyOf(entryPointJars), null) + LinkageChecker.create(classpath, ImmutableSet.copyOf(entryPointJars), ImmutableList.of(), null) .findLinkageProblems(); if (problemsInBaseline.equals(problemsInSnapshot)) {