Skip to content

Commit 2816f47

Browse files
JonathanPerry651JonathanPerry651
committed
Implement toolchain-driven unused dependencies checking (core Bazel changes)
1 parent a5c5dcd commit 2816f47

13 files changed

Lines changed: 246 additions & 7 deletions

File tree

‎src/java_tools/buildjar/java/com/google/devtools/build/buildjar/JavaLibraryBuildRequest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@ public JavaLibraryBuildRequest(
165165
if (optionsParser.getStrictJavaDeps() != null) {
166166
depsBuilder.setStrictJavaDeps(optionsParser.getStrictJavaDeps());
167167
}
168+
if (!optionsParser.getDirectDepJars().isEmpty()) {
169+
depsBuilder.addDirectDepJarsToVerify(
170+
optionsParser.getDirectDepJars().stream().map(this::asPath).collect(toImmutableList()),
171+
optionsParser.getDirectDepLabels());
172+
}
168173
if (optionsParser.getOutputDepsProtoFile() != null) {
169174
depsBuilder.setOutputDepsProtoFile(asPath(optionsParser.getOutputDepsProtoFile()));
170175
}

‎src/java_tools/buildjar/java/com/google/devtools/build/buildjar/OptionsParser.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ public final class OptionsParser {
5050

5151
private String outputDepsProtoFile;
5252
private final Set<String> depsArtifacts = new LinkedHashSet<>();
53+
private final List<String> directDepJars = new ArrayList<>();
54+
private final List<String> directDepLabels = new ArrayList<>();
5355

5456
/** This modes controls how a probablistic Java classpath reduction is used. */
5557
public enum ReduceClasspathMode {
@@ -138,6 +140,12 @@ private void processCommandlineArgs(Deque<String> argQueue) throws InvalidComman
138140
case "--strict_java_deps":
139141
strictJavaDeps = getArgument(argQueue, arg);
140142
break;
143+
case "--direct_dep_jar":
144+
directDepJars.add(getArgument(argQueue, arg));
145+
break;
146+
case "--direct_dep_label":
147+
directDepLabels.add(getArgument(argQueue, arg));
148+
break;
141149
case "--experimental_fix_deps_tool":
142150
fixDepsTool = getArgument(argQueue, arg);
143151
break;
@@ -365,6 +373,14 @@ public Set<String> getDepsArtifacts() {
365373
return depsArtifacts;
366374
}
367375

376+
public List<String> getDirectDepJars() {
377+
return directDepJars;
378+
}
379+
380+
public List<String> getDirectDepLabels() {
381+
return directDepLabels;
382+
}
383+
368384
public ReduceClasspathMode reduceClasspathMode() {
369385
return reduceClasspathMode;
370386
}

‎src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/DependencyModule.java‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ public static enum StrictJavaDeps {
8888
private final FixMessage fixMessage;
8989
private final Set<String> exemptGenerators;
9090
private final Set<PackageSymbol> packages;
91+
private final Map<Path, String> directDepJarsToVerify;
92+
private final Set<String> allDeclaredLabels;
9193

9294
DependencyModule(
9395
StrictJavaDeps strictJavaDeps,
@@ -99,7 +101,9 @@ public static enum StrictJavaDeps {
99101
String targetLabel,
100102
Path outputDepsProtoFile,
101103
FixMessage fixMessage,
102-
Set<String> exemptGenerators) {
104+
Set<String> exemptGenerators,
105+
Map<Path, String> directDepJarsToVerify,
106+
Set<String> allDeclaredLabels) {
103107
this.strictJavaDeps = strictJavaDeps;
104108
this.fixDepsTool = fixDepsTool;
105109
this.directJars = directJars;
@@ -113,6 +117,16 @@ public static enum StrictJavaDeps {
113117
this.fixMessage = fixMessage;
114118
this.exemptGenerators = exemptGenerators;
115119
this.packages = new HashSet<>();
120+
this.directDepJarsToVerify = directDepJarsToVerify;
121+
this.allDeclaredLabels = allDeclaredLabels;
122+
}
123+
124+
public Map<Path, String> getDirectDepJarsToVerify() {
125+
return directDepJarsToVerify;
126+
}
127+
128+
public Set<String> getAllDeclaredLabels() {
129+
return allDeclaredLabels;
116130
}
117131

118132
/** Returns a plugin to be enabled in the compiler. */
@@ -359,6 +373,9 @@ public String get(Iterable<JarOwner> missing, String recipient) {
359373
}
360374
}
361375

376+
private final Map<Path, String> directDepJarsToVerify = new HashMap<>();
377+
private final Set<String> allDeclaredLabels = new HashSet<>();
378+
362379
/**
363380
* Constructs the DependencyModule, guaranteeing that the maps are never null (they may be
364381
* empty), and the default strictJavaDeps setting is OFF.
@@ -376,7 +393,20 @@ public DependencyModule build() {
376393
targetLabel,
377394
outputDepsProtoFile,
378395
fixMessage,
379-
exemptGenerators);
396+
exemptGenerators,
397+
directDepJarsToVerify,
398+
allDeclaredLabels);
399+
}
400+
401+
@CanIgnoreReturnValue
402+
public Builder addDirectDepJarsToVerify(List<Path> jars, List<String> labels) {
403+
for (int i = 0; i < jars.size(); i++) {
404+
Path jar = jars.get(i);
405+
String label = labels.get(i);
406+
this.directDepJarsToVerify.put(jar, label);
407+
this.allDeclaredLabels.add(label);
408+
}
409+
return this;
380410
}
381411

382412
/**

‎src/java_tools/buildjar/java/com/google/devtools/build/buildjar/javac/plugins/dependency/StrictJavaDepsPlugin.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,28 @@ public void finish() {
240240
dependencyModule.setHasMissingTargets();
241241
}
242242
}
243+
244+
Map<Path, String> directDepJarsToVerify = dependencyModule.getDirectDepJarsToVerify();
245+
if (!directDepJarsToVerify.isEmpty()) {
246+
Set<String> usedLabels = new HashSet<>();
247+
for (Path usedJar : dependencyModule.getExplicitDependenciesMap().keySet()) {
248+
String label = directDepJarsToVerify.get(usedJar);
249+
if (label != null) {
250+
usedLabels.add(label);
251+
}
252+
}
253+
Set<String> unusedLabels = new java.util.TreeSet<>(dependencyModule.getAllDeclaredLabels());
254+
unusedLabels.removeAll(usedLabels);
255+
if (!unusedLabels.isEmpty()) {
256+
String targetLabel = dependencyModule.getTargetLabel();
257+
for (String unusedLabel : unusedLabels) {
258+
String msg = targetLabel != null
259+
? String.format("[unused-deps] Target '%s' is declared as a direct dependency of '%s' but is unused.", unusedLabel, targetLabel)
260+
: String.format("[unused-deps] Target '%s' is declared as a direct dependency but is unused.", unusedLabel);
261+
log.error(Position.NOPOS, Errors.ProcMessager(msg));
262+
}
263+
}
264+
}
243265
}
244266

245267
/**

‎src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@ && getJavaConfiguration().experimentalEnableJspecify()
298298
.getFixDepsTool(ruleContext.getRule(), getJavaConfiguration())
299299
.ifPresent(builder::setFixDepsTool);
300300
builder.setCompileTimeDependencyArtifacts(attributes.getCompileTimeDependencyArtifacts());
301+
builder.setDirectDepJarsToVerify(attributes.getDirectDepJarsToVerify());
302+
builder.setDirectDepLabelsToVerify(attributes.getDirectDepLabelsToVerify());
301303
builder.setTargetLabel(
302304
attributes.getTargetLabel() == null ? label : attributes.getTargetLabel());
303305
builder.setInjectingRuleKind(attributes.getInjectingRuleKind());

‎src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileActionBuilder.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ public void extend(ExtraActionInfo.Builder builder, ImmutableList<String> argume
157157
private NestedSet<Artifact> extraData = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER);
158158
private Label targetLabel;
159159
@Nullable private String injectingRuleKind;
160+
private ImmutableList<Artifact> directDepJarsToVerify = ImmutableList.of();
161+
private ImmutableList<String> directDepLabelsToVerify = ImmutableList.of();
160162
private ImmutableList<Artifact> additionalInputs = ImmutableList.of();
161163
private Artifact genSourceOutput;
162164
private JavaCompileOutputs<Artifact> outputs;
@@ -320,6 +322,12 @@ private CustomCommandLine buildParamFileContents(ImmutableList<String> javacOpts
320322
result.add("--strict_java_deps", strictJavaDeps.toString());
321323
result.addExecPaths("--direct_dependencies", directJars);
322324
}
325+
if (!directDepJarsToVerify.isEmpty()) {
326+
for (int i = 0; i < directDepJarsToVerify.size(); i++) {
327+
result.addExecPath("--direct_dep_jar", directDepJarsToVerify.get(i));
328+
result.add("--direct_dep_label", directDepLabelsToVerify.get(i));
329+
}
330+
}
323331
result.add("--experimental_fix_deps_tool", fixDepsTool);
324332

325333
// Chose what artifact to pass to JavaBuilder, as input to jacoco instrumentation processor.
@@ -378,6 +386,18 @@ public JavaCompileActionBuilder setCompileTimeDependencyArtifacts(
378386
return this;
379387
}
380388

389+
@CanIgnoreReturnValue
390+
public JavaCompileActionBuilder setDirectDepJarsToVerify(ImmutableList<Artifact> directDepJarsToVerify) {
391+
this.directDepJarsToVerify = directDepJarsToVerify;
392+
return this;
393+
}
394+
395+
@CanIgnoreReturnValue
396+
public JavaCompileActionBuilder setDirectDepLabelsToVerify(ImmutableList<String> directDepLabelsToVerify) {
397+
this.directDepLabelsToVerify = directDepLabelsToVerify;
398+
return this;
399+
}
400+
381401
@CanIgnoreReturnValue
382402
public JavaCompileActionBuilder setJavacOpts(ImmutableList<String> copts) {
383403
this.javacOpts = Preconditions.checkNotNull(copts);

‎src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ public void createCompilationAction(
201201
boolean enableJSpecify,
202202
boolean enableDirectClasspath,
203203
Sequence<?> additionalInputs,
204-
Sequence<?> additionalOutputs)
204+
Sequence<?> additionalOutputs,
205+
Sequence<?> directDepJarsToVerify)
205206
throws EvalException,
206207
TypeException,
207208
RuleErrorException,
@@ -217,11 +218,25 @@ public void createCompilationAction(
217218
.nativeHeader(nativeHeader == Starlark.NONE ? null : (Artifact) nativeHeader)
218219
.manifestProto(manifestProto)
219220
.build();
221+
ImmutableList.Builder<Artifact> directDepJarsToVerifyBuilder = ImmutableList.builder();
222+
ImmutableList.Builder<String> directDepLabelsToVerifyBuilder = ImmutableList.builder();
223+
for (Object obj : Sequence.cast(directDepJarsToVerify, Object.class, "direct_dep_jars_to_verify")) {
224+
if (obj instanceof StarlarkInfo struct) {
225+
Artifact jar = (Artifact) struct.getValue("jar");
226+
String label = (String) struct.getValue("label");
227+
if (jar != null && label != null) {
228+
directDepJarsToVerifyBuilder.add(jar);
229+
directDepLabelsToVerifyBuilder.add(label);
230+
}
231+
}
232+
}
220233
JavaTargetAttributes.Builder attributesBuilder =
221234
new JavaTargetAttributes.Builder()
222235
.addSourceJars(Sequence.cast(sourceJars, Artifact.class, "source_jars"))
223236
.addSourceFiles(Depset.noneableCast(sourceFiles, Artifact.class, "sources").toList())
224237
.addDirectJars(directJars.getSet(Artifact.class))
238+
.addDirectDepJarsToVerify(
239+
directDepJarsToVerifyBuilder.build(), directDepLabelsToVerifyBuilder.build())
225240
.setCompileTimeClassPathEntriesWithPrependedDirectJars(
226241
compileTimeClasspath.getSet(Artifact.class))
227242
.addClassPathResources(
@@ -415,6 +430,11 @@ public Sequence<?> tokenizeJavacOpts(Sequence<?> opts) throws EvalException {
415430
JavaHelper.tokenizeJavaOptions(Sequence.noneableCast(opts, String.class, "opts")));
416431
}
417432

433+
@Override
434+
public boolean isUnusedDepsSupported(StarlarkThread thread) throws EvalException {
435+
return true;
436+
}
437+
418438
static boolean isInstanceOfProvider(Object obj, Provider provider) {
419439
if (obj instanceof NativeInfo nativeInfo) {
420440
return nativeInfo.getProvider().getKey().equals(provider.getKey());

‎src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ public static class Builder {
6969
private StrictDepsMode strictJavaDeps = StrictDepsMode.ERROR;
7070

7171
private final NestedSetBuilder<Artifact> directJarsBuilder = NestedSetBuilder.naiveLinkOrder();
72+
private final ImmutableList.Builder<Artifact> directDepJarsToVerify = ImmutableList.builder();
73+
private final ImmutableList.Builder<String> directDepLabelsToVerify = ImmutableList.builder();
7274
private final NestedSetBuilder<Artifact> headerCompilationDirectJarsBuilder =
7375
NestedSetBuilder.naiveLinkOrder();
7476
private final NestedSetBuilder<Artifact> compileTimeDependencyArtifacts =
@@ -245,6 +247,14 @@ public Builder addAdditionalOutputs(Iterable<Artifact> outputs) {
245247
return this;
246248
}
247249

250+
@CanIgnoreReturnValue
251+
public Builder addDirectDepJarsToVerify(List<Artifact> jars, List<String> labels) {
252+
Preconditions.checkArgument(!built);
253+
this.directDepJarsToVerify.addAll(jars);
254+
this.directDepLabelsToVerify.addAll(labels);
255+
return this;
256+
}
257+
248258
public JavaTargetAttributes build() {
249259
built = true;
250260
NestedSet<Artifact> directJars = directJarsBuilder.build();
@@ -272,7 +282,9 @@ public JavaTargetAttributes build() {
272282
compileTimeDependencyArtifacts.build(),
273283
targetLabel,
274284
injectingRuleKind,
275-
strictJavaDeps);
285+
strictJavaDeps,
286+
directDepJarsToVerify.build(),
287+
directDepLabelsToVerify.build());
276288
}
277289

278290
// TODO(bazel-team): delete the following method - users should use the built
@@ -314,6 +326,8 @@ public boolean hasSourceFiles() {
314326
@Nullable private final String injectingRuleKind;
315327

316328
private final StrictDepsMode strictJavaDeps;
329+
private final ImmutableList<Artifact> directDepJarsToVerify;
330+
private final ImmutableList<String> directDepLabelsToVerify;
317331

318332
/** Constructor of JavaTargetAttributes. */
319333
private JavaTargetAttributes(
@@ -332,7 +346,9 @@ private JavaTargetAttributes(
332346
NestedSet<Artifact> compileTimeDependencyArtifacts,
333347
Label targetLabel,
334348
@Nullable String injectingRuleKind,
335-
StrictDepsMode strictJavaDeps) {
349+
StrictDepsMode strictJavaDeps,
350+
ImmutableList<Artifact> directDepJarsToVerify,
351+
ImmutableList<String> directDepLabelsToVerify) {
336352
this.sourceFiles = sourceFiles;
337353
this.directJars = directJars;
338354
this.headerCompilationDirectJars = headerCompilationDirectJars;
@@ -349,6 +365,8 @@ private JavaTargetAttributes(
349365
this.targetLabel = targetLabel;
350366
this.injectingRuleKind = injectingRuleKind;
351367
this.strictJavaDeps = strictJavaDeps;
368+
this.directDepJarsToVerify = directDepJarsToVerify;
369+
this.directDepLabelsToVerify = directDepLabelsToVerify;
352370
}
353371

354372
JavaTargetAttributes appendAdditionalTransitiveClassPathEntries(
@@ -373,7 +391,17 @@ JavaTargetAttributes appendAdditionalTransitiveClassPathEntries(
373391
compileTimeDependencyArtifacts,
374392
targetLabel,
375393
injectingRuleKind,
376-
strictJavaDeps);
394+
strictJavaDeps,
395+
directDepJarsToVerify,
396+
directDepLabelsToVerify);
397+
}
398+
399+
public ImmutableList<Artifact> getDirectDepJarsToVerify() {
400+
return directDepJarsToVerify;
401+
}
402+
403+
public ImmutableList<String> getDirectDepLabelsToVerify() {
404+
return directDepLabelsToVerify;
377405
}
378406

379407
public NestedSet<Artifact> getDirectJars() {

‎src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaCommonApi.java‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,11 @@ void createHeaderCompilationAction(
524524
@Param(name = "enable_direct_classpath", defaultValue = "True", named = true),
525525
@Param(name = "additional_inputs", defaultValue = "[]", named = true),
526526
@Param(name = "additional_outputs", defaultValue = "[]", named = true),
527+
@Param(
528+
name = "direct_dep_jars_to_verify",
529+
defaultValue = "[]",
530+
named = true,
531+
positional = false),
527532
})
528533
void createCompilationAction(
529534
StarlarkRuleContextT ctx,
@@ -553,7 +558,8 @@ void createCompilationAction(
553558
boolean enableJSpecify,
554559
boolean enableDirectClasspath,
555560
Sequence<?> additionalInputs,
556-
Sequence<?> additionalOutputs)
561+
Sequence<?> additionalOutputs,
562+
Sequence<?> directDepJarsToVerify)
557563
throws EvalException,
558564
TypeException,
559565
RuleErrorException,
@@ -697,4 +703,12 @@ Sequence<?> expandJavaOpts(
697703
documented = false,
698704
parameters = {@Param(name = "opts")})
699705
Sequence<?> tokenizeJavacOpts(Sequence<?> opts) throws EvalException, InterruptedException;
706+
707+
@StarlarkMethod(
708+
name = "is_unused_deps_supported",
709+
documented = false,
710+
useStarlarkThread = true)
711+
default boolean isUnusedDepsSupported(StarlarkThread thread) throws EvalException {
712+
return false;
713+
}
700714
}

‎src/main/starlark/builtins_bzl/common/java/java_common.bzl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ def _internal_exports():
3636
incompatible_disable_non_executable_java_binary = _java_common_internal.incompatible_disable_non_executable_java_binary,
3737
incompatible_java_info_merge_runtime_module_flags = _java_common_internal._incompatible_java_info_merge_runtime_module_flags,
3838
target_kind = _java_common_internal.target_kind,
39+
is_unused_deps_supported = _java_common_internal.is_unused_deps_supported,
3940
)
4041

4142
java_common = struct(internal_DO_NOT_USE = _internal_exports)

0 commit comments

Comments
 (0)