Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
* <p>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 "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -54,6 +55,7 @@ public class LinkageChecker {
private final ImmutableList<ClassPathEntry> classPath;
private final SymbolReferences symbolReferences;
private final ClassReferenceGraph classReferenceGraph;
private final List<Artifact> sourceFilterList;
private final ExcludedErrors excludedErrors;

@VisibleForTesting
Expand All @@ -66,7 +68,7 @@ public ClassReferenceGraph getClassReferenceGraph() {
}

public static LinkageChecker create(List<ClassPathEntry> classPath) throws IOException {
return create(classPath, ImmutableSet.copyOf(classPath), null);
return create(classPath, ImmutableSet.copyOf(classPath), ImmutableList.of(), null);
}

/**
Expand All @@ -79,6 +81,7 @@ public static LinkageChecker create(List<ClassPathEntry> classPath) throws IOExc
public static LinkageChecker create(
List<ClassPathEntry> classPath,
Iterable<ClassPathEntry> entryPoints,
List<Artifact> sourceFilterList,
@Nullable Path exclusionFile)
throws IOException {
Preconditions.checkArgument(!classPath.isEmpty(), "The linkage classpath is empty.");
Expand All @@ -93,6 +96,7 @@ public static LinkageChecker create(
classPath,
symbolReferenceMaps,
classReferenceGraph,
sourceFilterList,
ExcludedErrors.create(exclusionFile));
}

Expand Down Expand Up @@ -129,28 +133,42 @@ public static LinkageChecker create(Bom bom, Path exclusionFile)
List<ClassPathEntry> artifactsInBom = classpath.subList(0, managedDependencies.size());
ImmutableSet<ClassPathEntry> 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(
ClassDumper classDumper,
List<ClassPathEntry> classPath,
SymbolReferences symbolReferenceMaps,
ClassReferenceGraph classReferenceGraph,
List<Artifact> 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.
*
Expand All @@ -161,7 +179,21 @@ public ImmutableSet<LinkageProblem> findLinkageProblems() throws IOException {
ImmutableSet.Builder<LinkageProblem> problemToClass = ImmutableSet.builder();

// This sourceClassFile is a source of references to other symbols.
for (ClassFile classFile : symbolReferences.getClassFiles()) {
Set<ClassFile> 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<ClassSymbol> classSymbols = symbolReferences.getClassSymbols(classFile);
for (ClassSymbol classSymbol : classSymbols) {
if (classSymbol instanceof SuperClassSymbol) {
Expand Down Expand Up @@ -202,7 +234,7 @@ public ImmutableSet<LinkageProblem> findLinkageProblems() throws IOException {
}
}

for (ClassFile classFile : symbolReferences.getClassFiles()) {
for (ClassFile classFile : classFiles) {
ImmutableSet<MethodSymbol> methodSymbols = symbolReferences.getMethodSymbols(classFile);
ImmutableSet<String> classFileNames = classFile.getClassPathEntry().getFileNames();
for (MethodSymbol methodSymbol : methodSymbols) {
Expand All @@ -215,7 +247,7 @@ public ImmutableSet<LinkageProblem> findLinkageProblems() throws IOException {
}
}

for (ClassFile classFile : symbolReferences.getClassFiles()) {
for (ClassFile classFile : classFiles) {
ImmutableSet<FieldSymbol> fieldSymbols = symbolReferences.getFieldSymbols(classFile);
ImmutableSet<String> classFileNames = classFile.getClassPathEntry().getFileNames();
for (FieldSymbol fieldSymbol : fieldSymbols) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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<Artifact> getSourceFilterArtifactList() {
if (commandLine.hasOption("s")) {
String[] mavenCoordinatesOption = commandLine.getOptionValues("s");
return Arrays.stream(mavenCoordinatesOption)
.map(DefaultArtifact::new)
.collect(Collectors.toList());
}
return ImmutableList.of();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -133,7 +133,7 @@ private static Problems checkJarFiles(
ImmutableSet<ClassPathEntry> entryPoints = ImmutableSet.copyOf(inputClassPath);
LinkageChecker linkageChecker =
LinkageChecker.create(
inputClassPath, entryPoints, linkageCheckerArguments.getInputExclusionFile());
inputClassPath, entryPoints, ImmutableList.of(), linkageCheckerArguments.getInputExclusionFile());

ImmutableSet<LinkageProblem> linkageProblems =
findLinkageProblems(linkageChecker, linkageCheckerArguments.getReportOnlyReachable());
Expand Down Expand Up @@ -161,7 +161,7 @@ private static Problems checkArtifacts(

LinkageChecker linkageChecker =
LinkageChecker.create(
inputClassPath, entryPoints, linkageCheckerArguments.getInputExclusionFile());
inputClassPath, entryPoints, linkageCheckerArguments.getSourceFilterArtifactList(), linkageCheckerArguments.getInputExclusionFile());
ImmutableSet<LinkageProblem> linkageProblems =
findLinkageProblems(linkageChecker,
linkageCheckerArguments.getReportOnlyReachable());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<LinkageProblem> linkageProblems = linkagechecker.findLinkageProblems();
Truth.assertThat(linkageProblems).isEmpty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClassPathEntry> 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<LinkageProblem> problems = linkageChecker.findLinkageProblems();
Truth.assertThat(problems.size()).isEqualTo(3);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<LinkageProblem> linkageProblems = linkageChecker.findLinkageProblems();
if (reportOnlyReachable) {
ClassReferenceGraph classReferenceGraph = linkageChecker.getClassReferenceGraph();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<LinkageProblem> linkageProblems = linkageChecker.findLinkageProblems();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ private ImmutableSet<LinkageProblem> run(String groupId, String artifactId)
List<ClassPathEntry> entryPointJars = classpath.subList(0, snapshotManagedDependencies.size());

ImmutableSet<LinkageProblem> problemsInSnapshot =
LinkageChecker.create(classpath, ImmutableSet.copyOf(entryPointJars), null)
LinkageChecker.create(classpath, ImmutableSet.copyOf(entryPointJars), ImmutableList.of(), null)
.findLinkageProblems();

if (problemsInBaseline.equals(problemsInSnapshot)) {
Expand Down
Loading