Skip to content

Commit 58dd1d1

Browse files
authored
feat: Add option to filter linkage checker problems coming from a list of artifacts (#2393)
* feat: Allow LinkageChecker to be called via exec:java goal * feat: Add an Source filter to check if errors come from subset of files * chore: Source Filter takes a list of artifacts * chore: Update source filter to take in multiple args * chore: Fix broken tests * chore: Add comments and update artifacts equals method * chore: Fix broken tests * chore: Fix broken tests * chore: Fix issues * chore: Fix issues * Revert "chore: Fix issues" This reverts commit 3701953. * chore: Fix issues * chore: update comments to explain why not use problemFilter * chore: Add test for source-filter * chore: Add comments about including areArtifactsEquals helper method
1 parent 081130c commit 58dd1d1

File tree

9 files changed

+102
-14
lines changed

9 files changed

+102
-14
lines changed

dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckResultException.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* <p>The caller of the tool can tell the existence of linkage errors by checking the exit status of
2323
* the {@link LinkageCheckerMain}.
2424
*/
25-
final class LinkageCheckResultException extends Exception {
25+
public final class LinkageCheckResultException extends Exception {
2626
LinkageCheckResultException(int linkageErrorCount) {
2727
super(
2828
"Found "

dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageChecker.java

+38-6
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.Queue;
3737
import java.util.Set;
3838
import java.util.logging.Logger;
39+
import java.util.stream.Collectors;
3940
import javax.annotation.Nullable;
4041
import org.apache.bcel.classfile.Field;
4142
import org.apache.bcel.classfile.FieldOrMethod;
@@ -54,6 +55,7 @@ public class LinkageChecker {
5455
private final ImmutableList<ClassPathEntry> classPath;
5556
private final SymbolReferences symbolReferences;
5657
private final ClassReferenceGraph classReferenceGraph;
58+
private final List<Artifact> sourceFilterList;
5759
private final ExcludedErrors excludedErrors;
5860

5961
@VisibleForTesting
@@ -66,7 +68,7 @@ public ClassReferenceGraph getClassReferenceGraph() {
6668
}
6769

6870
public static LinkageChecker create(List<ClassPathEntry> classPath) throws IOException {
69-
return create(classPath, ImmutableSet.copyOf(classPath), null);
71+
return create(classPath, ImmutableSet.copyOf(classPath), ImmutableList.of(), null);
7072
}
7173

7274
/**
@@ -79,6 +81,7 @@ public static LinkageChecker create(List<ClassPathEntry> classPath) throws IOExc
7981
public static LinkageChecker create(
8082
List<ClassPathEntry> classPath,
8183
Iterable<ClassPathEntry> entryPoints,
84+
List<Artifact> sourceFilterList,
8285
@Nullable Path exclusionFile)
8386
throws IOException {
8487
Preconditions.checkArgument(!classPath.isEmpty(), "The linkage classpath is empty.");
@@ -93,6 +96,7 @@ public static LinkageChecker create(
9396
classPath,
9497
symbolReferenceMaps,
9598
classReferenceGraph,
99+
sourceFilterList,
96100
ExcludedErrors.create(exclusionFile));
97101
}
98102

@@ -129,28 +133,42 @@ public static LinkageChecker create(Bom bom, Path exclusionFile)
129133
List<ClassPathEntry> artifactsInBom = classpath.subList(0, managedDependencies.size());
130134
ImmutableSet<ClassPathEntry> entryPoints = ImmutableSet.copyOf(artifactsInBom);
131135

132-
return LinkageChecker.create(classpath, entryPoints, exclusionFile);
136+
return LinkageChecker.create(classpath, entryPoints, ImmutableList.of(), exclusionFile);
133137
}
134138

135139
@VisibleForTesting
136140
LinkageChecker cloneWith(SymbolReferences newSymbolMaps) {
137141
return new LinkageChecker(
138-
classDumper, classPath, newSymbolMaps, classReferenceGraph, excludedErrors);
142+
classDumper, classPath, newSymbolMaps, classReferenceGraph, ImmutableList.of(), excludedErrors);
139143
}
140144

141145
private LinkageChecker(
142146
ClassDumper classDumper,
143147
List<ClassPathEntry> classPath,
144148
SymbolReferences symbolReferenceMaps,
145149
ClassReferenceGraph classReferenceGraph,
150+
List<Artifact> sourceFilterList,
146151
ExcludedErrors excludedErrors) {
147152
this.classDumper = Preconditions.checkNotNull(classDumper);
148153
this.classPath = ImmutableList.copyOf(classPath);
149154
this.classReferenceGraph = Preconditions.checkNotNull(classReferenceGraph);
150155
this.symbolReferences = Preconditions.checkNotNull(symbolReferenceMaps);
156+
this.sourceFilterList = sourceFilterList;
151157
this.excludedErrors = Preconditions.checkNotNull(excludedErrors);
152158
}
153159

160+
/**
161+
* Two artifacts are considered equal if only the Maven Coordinates (GAV) are equal. This is
162+
* included instead of using Artifact.equals() because the `equals()` implementation
163+
* of DefaultArtifact checks more fields than just the GAV Coordinates (also checks the classifier,
164+
* file path, properties, etc are all equal).
165+
*/
166+
private boolean areArtifactsEquals(Artifact artifact1, Artifact artifact2) {
167+
return artifact1.getGroupId().equals(artifact2.getGroupId())
168+
&& artifact1.getArtifactId().equals(artifact2.getArtifactId())
169+
&& artifact1.getVersion().equals(artifact2.getVersion());
170+
}
171+
154172
/**
155173
* Searches the classpath for linkage errors.
156174
*
@@ -161,7 +179,21 @@ public ImmutableSet<LinkageProblem> findLinkageProblems() throws IOException {
161179
ImmutableSet.Builder<LinkageProblem> problemToClass = ImmutableSet.builder();
162180

163181
// This sourceClassFile is a source of references to other symbols.
164-
for (ClassFile classFile : symbolReferences.getClassFiles()) {
182+
Set<ClassFile> classFiles = symbolReferences.getClassFiles();
183+
184+
// Filtering the classFiles from the JARs (instead of using the problem filter) has additional a few
185+
// additional benefits. 1. Reduces the total amount of linkage references to match and 2. Doesn't require
186+
// an exclusion file to know all the possible flaky or false positive problems
187+
if (!sourceFilterList.isEmpty()) {
188+
// Filter the list to only contain class files that come from the classes we are interested in.
189+
// Run through each class file and check that the class file's corresponding artifact matches
190+
// any artifact specified in the sourceFilterList
191+
classFiles = classFiles.stream()
192+
.filter(x -> sourceFilterList.stream()
193+
.anyMatch(y -> areArtifactsEquals(x.getClassPathEntry().getArtifact(), y)))
194+
.collect(Collectors.toSet());
195+
}
196+
for (ClassFile classFile : classFiles) {
165197
ImmutableSet<ClassSymbol> classSymbols = symbolReferences.getClassSymbols(classFile);
166198
for (ClassSymbol classSymbol : classSymbols) {
167199
if (classSymbol instanceof SuperClassSymbol) {
@@ -202,7 +234,7 @@ public ImmutableSet<LinkageProblem> findLinkageProblems() throws IOException {
202234
}
203235
}
204236

205-
for (ClassFile classFile : symbolReferences.getClassFiles()) {
237+
for (ClassFile classFile : classFiles) {
206238
ImmutableSet<MethodSymbol> methodSymbols = symbolReferences.getMethodSymbols(classFile);
207239
ImmutableSet<String> classFileNames = classFile.getClassPathEntry().getFileNames();
208240
for (MethodSymbol methodSymbol : methodSymbols) {
@@ -215,7 +247,7 @@ public ImmutableSet<LinkageProblem> findLinkageProblems() throws IOException {
215247
}
216248
}
217249

218-
for (ClassFile classFile : symbolReferences.getClassFiles()) {
250+
for (ClassFile classFile : classFiles) {
219251
ImmutableSet<FieldSymbol> fieldSymbols = symbolReferences.getFieldSymbols(classFile);
220252
ImmutableSet<String> classFileNames = classFile.getClassPathEntry().getFileNames();
221253
for (FieldSymbol fieldSymbol : fieldSymbols) {

dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerArguments.java

+28
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
import java.nio.file.Path;
2626
import java.nio.file.Paths;
2727
import java.util.Arrays;
28+
import java.util.List;
29+
import java.util.stream.Collectors;
30+
2831
import org.apache.commons.cli.CommandLine;
2932
import org.apache.commons.cli.CommandLineParser;
3033
import org.apache.commons.cli.DefaultParser;
@@ -124,6 +127,17 @@ private static Options configureOptions() {
124127
.build();
125128
inputGroup.addOption(jarOption);
126129

130+
Option sourceFilter =
131+
Option.builder("s")
132+
.longOpt("source-filter")
133+
.hasArgs()
134+
.valueSeparator(',')
135+
.desc("List of maven coordinates for artifacts (separated by ','). " +
136+
"These are artifacts whose class files are searched as the source of the " +
137+
"potential Linkage Errors.")
138+
.build();
139+
options.addOption(sourceFilter);
140+
127141
Option repositoryOption =
128142
Option.builder("m")
129143
.longOpt("maven-repositories")
@@ -277,4 +291,18 @@ Path getOutputExclusionFile() {
277291
}
278292
return null;
279293
}
294+
295+
/**
296+
* Returns a list of artifacts to search where Linkage Errors stem from. If the argument is not
297+
* specified, return an empty List.
298+
*/
299+
List<Artifact> getSourceFilterArtifactList() {
300+
if (commandLine.hasOption("s")) {
301+
String[] mavenCoordinatesOption = commandLine.getOptionValues("s");
302+
return Arrays.stream(mavenCoordinatesOption)
303+
.map(DefaultArtifact::new)
304+
.collect(Collectors.toList());
305+
}
306+
return ImmutableList.of();
307+
}
280308
}

dependencies/src/main/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerMain.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
/**
3434
* A tool to find linkage errors in a class path.
3535
*/
36-
class LinkageCheckerMain {
36+
public class LinkageCheckerMain {
3737

3838
/**
3939
* 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(
133133
ImmutableSet<ClassPathEntry> entryPoints = ImmutableSet.copyOf(inputClassPath);
134134
LinkageChecker linkageChecker =
135135
LinkageChecker.create(
136-
inputClassPath, entryPoints, linkageCheckerArguments.getInputExclusionFile());
136+
inputClassPath, entryPoints, ImmutableList.of(), linkageCheckerArguments.getInputExclusionFile());
137137

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

162162
LinkageChecker linkageChecker =
163163
LinkageChecker.create(
164-
inputClassPath, entryPoints, linkageCheckerArguments.getInputExclusionFile());
164+
inputClassPath, entryPoints, linkageCheckerArguments.getSourceFilterArtifactList(), linkageCheckerArguments.getInputExclusionFile());
165165
ImmutableSet<LinkageProblem> linkageProblems =
166166
findLinkageProblems(linkageChecker,
167167
linkageCheckerArguments.getReportOnlyReachable());

dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/ExclusionFilesIntegrationTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void testExclusion()
5858
classPathBuilder.resolve(ImmutableList.of(artifact), false, DependencyMediation.MAVEN);
5959

6060
LinkageChecker linkagechecker =
61-
LinkageChecker.create(classPathResult.getClassPath(), ImmutableList.of(), exclusionFile);
61+
LinkageChecker.create(classPathResult.getClassPath(), ImmutableList.of(), ImmutableList.of(), exclusionFile);
6262

6363
ImmutableSet<LinkageProblem> linkageProblems = linkagechecker.findLinkageProblems();
6464
Truth.assertThat(linkageProblems).isEmpty();

dependencies/src/test/java/com/google/cloud/tools/opensource/classpath/LinkageCheckerTest.java

+28
Original file line numberDiff line numberDiff line change
@@ -1340,4 +1340,32 @@ public void testProtectedMethodsOfObject() throws Exception {
13401340
"has linkage errors that reference symbols on class"))
13411341
.doesNotContain("java.lang.Object");
13421342
}
1343+
1344+
@Test
1345+
public void testSourceFilter() throws InvalidVersionSpecificationException, IOException {
1346+
// BQ-Storage v3.9.3 contains 3 known binary incompatibilities with Protobuf-Java 4.x
1347+
DefaultArtifact sourceArtifact = new DefaultArtifact("com.google.cloud:google-cloud-bigquerystorage:3.9.3");
1348+
ClassPathResult classPathResult =
1349+
new ClassPathBuilder()
1350+
.resolve(
1351+
ImmutableList.of(
1352+
sourceArtifact,
1353+
new DefaultArtifact("com.google.protobuf:protobuf-java:4.27.4"),
1354+
new DefaultArtifact("com.google.protobuf:protobuf-java-util:4.27.4")),
1355+
false,
1356+
DependencyMediation.MAVEN);
1357+
1358+
ImmutableList<ClassPathEntry> classPath = classPathResult.getClassPath();
1359+
LinkageChecker linkageChecker = LinkageChecker.create(
1360+
classPath,
1361+
ImmutableSet.copyOf(classPath),
1362+
ImmutableList.of(sourceArtifact),
1363+
null);
1364+
1365+
// Without a source-filter to narrow down the linkage errors that stem from BQ-Storage, Linkage Checker
1366+
// would report 119 errors. Narrowing it down with the source filter will only report the 3 known binary
1367+
// incompatibilities
1368+
ImmutableSet<LinkageProblem> problems = linkageChecker.findLinkageProblems();
1369+
Truth.assertThat(problems.size()).isEqualTo(3);
1370+
}
13431371
}

enforcer-rules/src/main/java/com/google/cloud/tools/dependencies/enforcer/LinkageCheckerRule.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ public void execute() throws EnforcerRuleException {
224224
// findLinkageProblems immediately after create.
225225

226226
Path exclusionFile = this.exclusionFile == null ? null : Paths.get(this.exclusionFile);
227-
LinkageChecker linkageChecker = LinkageChecker.create(classPath, entryPoints, exclusionFile);
227+
LinkageChecker linkageChecker = LinkageChecker.create(classPath, entryPoints, ImmutableList.of(), exclusionFile);
228228
ImmutableSet<LinkageProblem> linkageProblems = linkageChecker.findLinkageProblems();
229229
if (reportOnlyReachable) {
230230
ClassReferenceGraph classReferenceGraph = linkageChecker.getClassReferenceGraph();

gradle-plugin/src/main/java/com/google/cloud/tools/dependencies/gradle/LinkageCheckTask.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ private boolean findLinkageErrors(Configuration configuration) throws IOExceptio
145145
}
146146

147147
// TODO(suztomo): Specify correct entry points if reportOnlyReachable is true.
148-
LinkageChecker linkageChecker = LinkageChecker.create(classPath, classPath, exclusionFile);
148+
LinkageChecker linkageChecker = LinkageChecker.create(classPath, classPath, ImmutableList.of(), exclusionFile);
149149

150150
ImmutableSet<LinkageProblem> linkageProblems = linkageChecker.findLinkageProblems();
151151

linkage-monitor/src/main/java/com/google/cloud/tools/dependencies/linkagemonitor/LinkageMonitor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ private ImmutableSet<LinkageProblem> run(String groupId, String artifactId)
294294
List<ClassPathEntry> entryPointJars = classpath.subList(0, snapshotManagedDependencies.size());
295295

296296
ImmutableSet<LinkageProblem> problemsInSnapshot =
297-
LinkageChecker.create(classpath, ImmutableSet.copyOf(entryPointJars), null)
297+
LinkageChecker.create(classpath, ImmutableSet.copyOf(entryPointJars), ImmutableList.of(), null)
298298
.findLinkageProblems();
299299

300300
if (problemsInBaseline.equals(problemsInSnapshot)) {

0 commit comments

Comments
 (0)