Skip to content

Commit 6a1697c

Browse files
committed
Add flag to warn about unused dependencies
This commit adds an experimental flag to cargo, -Zwarn-unused-deps, with the goal of getting it eventually stabilized and enabled by default. The lint builds upon the --json=unused-externs flag of rustc that is being proposed for this purpose. This commit makes cargo pass the flag if -Zwarn-unused-deps is enabled to compilations of units it prints warnings for. During compilation, code collects the unused dependencies from all units, converts them into used dependencies, continuously extending the record of used dependencies for any type of DepKind: [dependencies], [dev-dependencies] and [build-dependencies]. Once the compilation has ended, this record is used to obtain those dependencies in a class that weren't used by any unit, and warn about them. The goal is to stabilize the flag and enable it by default once the lint has become robust and the robustness is proven. Then, cargo shall opportunistically warn about unused dependencies when it has compiled (or loaded the unused externs from cache of) all units that could possibly use dependencies of the kind. Roughly, it's like this: * cargo always compiles build.rs * cargo check compiles all units that would use [dependencies] * cargo test --no-run --all-targets compiles all units that can use [dev-dependencies]... except for the benches * cargo check --all-targets compiles all units that can use [dev-dependencies]... except for the doctests
1 parent 30a6d21 commit 6a1697c

File tree

11 files changed

+1201
-16
lines changed

11 files changed

+1201
-16
lines changed

crates/cargo-test-macro/src/lib.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
1414
))));
1515

1616
let build_std = contains_ident(&attr, "build_std");
17+
let unused_dependencies = contains_ident(&attr, "unused_dependencies");
1718

1819
for token in item {
1920
let group = match token {
@@ -34,12 +35,14 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream {
3435
let mut new_body =
3536
to_token_stream("let _test_guard = cargo_test_support::paths::init_root();");
3637

37-
// If this is a `build_std` test (aka `tests/build-std/*.rs`) then they
38-
// only run on nightly and they only run when specifically instructed to
39-
// on CI.
40-
if build_std {
38+
// If this is a test that only runs on nightly (`build_std` and `unused_dependencies`)
39+
if build_std || unused_dependencies {
4140
let ts = to_token_stream("if !cargo_test_support::is_nightly() { return }");
4241
new_body.extend(ts);
42+
}
43+
// `build_std` tests (aka `tests/build-std/*.rs`) only run
44+
// when specifically instructed to on CI.
45+
if build_std {
4346
let ts = to_token_stream(
4447
"if std::env::var(\"CARGO_RUN_BUILD_STD_TESTS\").is_err() { return }",
4548
);

src/cargo/core/compiler/build_context/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use crate::core::compiler::unit_graph::UnitGraph;
2+
use crate::core::compiler::unused_dependencies::AllowedKinds;
23
use crate::core::compiler::{BuildConfig, CompileKind, Unit};
34
use crate::core::profiles::Profiles;
45
use crate::core::PackageSet;
@@ -30,6 +31,7 @@ pub struct BuildContext<'a, 'cfg> {
3031
pub profiles: Profiles,
3132
pub build_config: &'a BuildConfig,
3233

34+
pub allowed_kinds: AllowedKinds,
3335
/// Extra compiler args for either `rustc` or `rustdoc`.
3436
pub extra_compiler_args: HashMap<Unit, Vec<String>>,
3537

@@ -56,6 +58,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
5658
ws: &'a Workspace<'cfg>,
5759
packages: PackageSet<'cfg>,
5860
build_config: &'a BuildConfig,
61+
allowed_kinds: AllowedKinds,
5962
profiles: Profiles,
6063
extra_compiler_args: HashMap<Unit, Vec<String>>,
6164
target_data: RustcTargetData,
@@ -74,6 +77,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
7477
config: ws.config(),
7578
packages,
7679
build_config,
80+
allowed_kinds,
7781
profiles,
7882
extra_compiler_args,
7983
target_data,

src/cargo/core/compiler/job_queue.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ use super::job::{
7070
Job,
7171
};
7272
use super::timings::Timings;
73+
use super::unused_dependencies::UnusedDepState;
7374
use super::{BuildContext, BuildPlan, CompileMode, Context, Unit};
7475
use crate::core::compiler::future_incompat::{
7576
FutureBreakageItem, OnDiskReport, FUTURE_INCOMPAT_FILE,
@@ -133,6 +134,7 @@ struct DrainState<'cfg> {
133134
progress: Progress<'cfg>,
134135
next_id: u32,
135136
timings: Timings<'cfg>,
137+
unused_dep_state: UnusedDepState,
136138

137139
/// Tokens that are currently owned by this Cargo, and may be "associated"
138140
/// with a rustc process. They may also be unused, though if so will be
@@ -242,6 +244,7 @@ enum Message {
242244
Token(io::Result<Acquired>),
243245
Finish(JobId, Artifact, CargoResult<()>),
244246
FutureIncompatReport(JobId, Vec<FutureBreakageItem>),
247+
UnusedExterns(JobId, Vec<String>),
245248

246249
// This client should get release_raw called on it with one of our tokens
247250
NeedsToken(JobId),
@@ -301,6 +304,15 @@ impl<'a> JobState<'a> {
301304
.push(Message::FutureIncompatReport(self.id, report));
302305
}
303306

307+
/// The rustc emitted the list of unused `--extern` args.
308+
///
309+
/// This is useful for checking unused dependencies.
310+
/// Should only be called once, as the compiler only emits it once per compilation.
311+
pub fn unused_externs(&self, unused_externs: Vec<String>) {
312+
self.messages
313+
.push(Message::UnusedExterns(self.id, unused_externs));
314+
}
315+
304316
/// The rustc underlying this Job is about to acquire a jobserver token (i.e., block)
305317
/// on the passed client.
306318
///
@@ -423,6 +435,7 @@ impl<'cfg> JobQueue<'cfg> {
423435
progress,
424436
next_id: 0,
425437
timings: self.timings,
438+
unused_dep_state: UnusedDepState::new_with_graph(cx), // TODO
426439
tokens: Vec::new(),
427440
rustc_tokens: HashMap::new(),
428441
to_send_clients: BTreeMap::new(),
@@ -614,6 +627,11 @@ impl<'cfg> DrainState<'cfg> {
614627
self.per_crate_future_incompat_reports
615628
.push(FutureIncompatReportCrate { package_id, report });
616629
}
630+
Message::UnusedExterns(id, unused_externs) => {
631+
let unit = &self.active[&id];
632+
self.unused_dep_state
633+
.record_unused_externs_for_unit(cx, unit, unused_externs);
634+
}
617635
Message::Token(acquired_token) => {
618636
let token = acquired_token.chain_err(|| "failed to acquire jobserver token")?;
619637
self.tokens.push(token);
@@ -786,6 +804,10 @@ impl<'cfg> DrainState<'cfg> {
786804
}
787805
}
788806

807+
if !cx.bcx.build_config.build_plan && cx.bcx.config.cli_unstable().warn_unused_deps {
808+
drop(self.unused_dep_state.emit_unused_warnings(cx));
809+
}
810+
789811
if let Some(e) = error {
790812
(Err(e),)
791813
} else if self.queue.is_empty() && self.pending_queue.is_empty() {

src/cargo/core/compiler/mod.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ mod timings;
2020
mod unit;
2121
pub mod unit_dependencies;
2222
pub mod unit_graph;
23+
pub mod unused_dependencies;
2324

2425
use std::env;
2526
use std::ffi::{OsStr, OsString};
@@ -614,7 +615,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
614615
rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
615616
}
616617

617-
add_error_format_and_color(cx, &mut rustdoc, false);
618+
add_error_format_and_color(cx, unit, &mut rustdoc, false);
618619
add_allow_features(cx, &mut rustdoc);
619620

620621
if let Some(args) = cx.bcx.extra_args_for(unit) {
@@ -715,7 +716,12 @@ fn add_allow_features(cx: &Context<'_, '_>, cmd: &mut ProcessBuilder) {
715716
/// intercepting messages like rmeta artifacts, etc. rustc includes a
716717
/// "rendered" field in the JSON message with the message properly formatted,
717718
/// which Cargo will extract and display to the user.
718-
fn add_error_format_and_color(cx: &Context<'_, '_>, cmd: &mut ProcessBuilder, pipelined: bool) {
719+
fn add_error_format_and_color(
720+
cx: &Context<'_, '_>,
721+
unit: &Unit,
722+
cmd: &mut ProcessBuilder,
723+
pipelined: bool,
724+
) {
719725
cmd.arg("--error-format=json");
720726
let mut json = String::from("--json=diagnostic-rendered-ansi");
721727
if pipelined {
@@ -724,6 +730,12 @@ fn add_error_format_and_color(cx: &Context<'_, '_>, cmd: &mut ProcessBuilder, pi
724730
json.push_str(",artifacts");
725731
}
726732

733+
// Emit unused externs but only if the flag is enabled
734+
// and only for units we are interested in.
735+
if cx.bcx.config.cli_unstable().warn_unused_deps && unit.show_warnings(cx.bcx.config) {
736+
json.push_str(",unused-externs");
737+
}
738+
727739
match cx.bcx.build_config.message_format {
728740
MessageFormat::Short | MessageFormat::Json { short: true, .. } => {
729741
json.push_str(",diagnostic-short");
@@ -782,7 +794,7 @@ fn build_base_args(
782794
edition.cmd_edition_arg(cmd);
783795

784796
add_path_args(bcx.ws, unit, cmd);
785-
add_error_format_and_color(cx, cmd, cx.rmeta_required(unit));
797+
add_error_format_and_color(cx, unit, cmd, cx.rmeta_required(unit));
786798
add_allow_features(cx, cmd);
787799

788800
if !test {
@@ -1038,6 +1050,10 @@ fn build_deps_args(
10381050
cmd.arg(arg);
10391051
}
10401052

1053+
if cx.bcx.config.cli_unstable().warn_unused_deps {
1054+
unstable_opts = true;
1055+
}
1056+
10411057
// This will only be set if we're already using a feature
10421058
// requiring nightly rust
10431059
if unstable_opts {
@@ -1322,6 +1338,19 @@ fn on_stderr_line_inner(
13221338
}
13231339
}
13241340

1341+
#[derive(serde::Deserialize)]
1342+
struct UnusedExterns {
1343+
unused_extern_names: Vec<String>,
1344+
}
1345+
if let Ok(uext) = serde_json::from_str::<UnusedExterns>(compiler_message.get()) {
1346+
log::trace!(
1347+
"obtained unused externs list from rustc: `{:?}`",
1348+
uext.unused_extern_names
1349+
);
1350+
state.unused_externs(uext.unused_extern_names);
1351+
return Ok(true);
1352+
}
1353+
13251354
#[derive(serde::Deserialize)]
13261355
struct JobserverNotification {
13271356
jobserver_event: Event,

src/cargo/core/compiler/unit_dependencies.rs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
//! (for example, with and without tests), so we actually build a dependency
1616
//! graph of `Unit`s, which capture these properties.
1717
18-
use crate::core::compiler::unit_graph::{UnitDep, UnitGraph};
18+
use crate::core::compiler::unit_graph::{UnitDep, UnitDependency, UnitGraph};
1919
use crate::core::compiler::UnitInterner;
2020
use crate::core::compiler::{CompileKind, CompileMode, RustcTargetData, Unit};
2121
use crate::core::dependency::DepKind;
@@ -146,6 +146,9 @@ fn attach_std_deps(
146146
if !unit.kind.is_host() && !unit.mode.is_run_custom_build() {
147147
deps.extend(std_roots[&unit.kind].iter().map(|unit| UnitDep {
148148
unit: unit.clone(),
149+
// There is no dependency in the manifest giving rise to this
150+
// as the dependency is implicit
151+
dependency: UnitDependency(None),
149152
unit_for: UnitFor::new_normal(),
150153
extern_crate_name: unit.pkg.name(),
151154
// TODO: Does this `public` make sense?
@@ -269,20 +272,44 @@ fn compute_deps(
269272
let mode = check_or_build_mode(unit.mode, lib);
270273
let dep_unit_for = unit_for.with_dependency(unit, lib);
271274

275+
// TODO this does not take into account cases where a dependency
276+
// comes from multiple sources like appears both as dev-dependency
277+
// and proper dependency, same dependency through multiple cfg's, etc.
278+
let dep = deps.iter().next().cloned();
279+
272280
let start = ret.len();
273281
if state.config.cli_unstable().dual_proc_macros && lib.proc_macro() && !unit.kind.is_host()
274282
{
275-
let unit_dep = new_unit_dep(state, unit, pkg, lib, dep_unit_for, unit.kind, mode)?;
283+
let dep_cloned = dep.clone();
284+
let unit_dep = new_unit_dep(
285+
state,
286+
unit,
287+
pkg,
288+
lib,
289+
dep_cloned,
290+
dep_unit_for,
291+
unit.kind,
292+
mode,
293+
)?;
276294
ret.push(unit_dep);
277-
let unit_dep =
278-
new_unit_dep(state, unit, pkg, lib, dep_unit_for, CompileKind::Host, mode)?;
295+
let unit_dep = new_unit_dep(
296+
state,
297+
unit,
298+
pkg,
299+
lib,
300+
dep,
301+
dep_unit_for,
302+
CompileKind::Host,
303+
mode,
304+
)?;
279305
ret.push(unit_dep);
280306
} else {
281307
let unit_dep = new_unit_dep(
282308
state,
283309
unit,
284310
pkg,
285311
lib,
312+
dep,
286313
dep_unit_for,
287314
unit.kind.for_target(lib),
288315
mode,
@@ -351,6 +378,7 @@ fn compute_deps(
351378
unit,
352379
&unit.pkg,
353380
t,
381+
None,
354382
UnitFor::new_normal(),
355383
unit.kind.for_target(t),
356384
CompileMode::Build,
@@ -400,6 +428,7 @@ fn compute_deps_custom_build(
400428
unit,
401429
&unit.pkg,
402430
&unit.target,
431+
None,
403432
script_unit_for,
404433
// Build scripts always compiled for the host.
405434
CompileKind::Host,
@@ -435,6 +464,7 @@ fn compute_deps_doc(
435464
unit,
436465
dep,
437466
lib,
467+
None,
438468
dep_unit_for,
439469
unit.kind.for_target(lib),
440470
mode,
@@ -447,6 +477,7 @@ fn compute_deps_doc(
447477
unit,
448478
dep,
449479
lib,
480+
None,
450481
dep_unit_for,
451482
unit.kind.for_target(lib),
452483
unit.mode,
@@ -482,6 +513,7 @@ fn maybe_lib(
482513
unit,
483514
&unit.pkg,
484515
t,
516+
None,
485517
dep_unit_for,
486518
unit.kind.for_target(t),
487519
mode,
@@ -541,6 +573,7 @@ fn dep_build_script(
541573
unit,
542574
&unit.pkg,
543575
t,
576+
None,
544577
script_unit_for,
545578
unit.kind,
546579
CompileMode::RunCustomBuild,
@@ -574,6 +607,7 @@ fn new_unit_dep(
574607
parent: &Unit,
575608
pkg: &Package,
576609
target: &Target,
610+
dependency: Option<Dependency>,
577611
unit_for: UnitFor,
578612
kind: CompileKind,
579613
mode: CompileMode,
@@ -587,14 +621,17 @@ fn new_unit_dep(
587621
mode,
588622
kind,
589623
);
590-
new_unit_dep_with_profile(state, parent, pkg, target, unit_for, kind, mode, profile)
624+
new_unit_dep_with_profile(
625+
state, parent, pkg, target, dependency, unit_for, kind, mode, profile,
626+
)
591627
}
592628

593629
fn new_unit_dep_with_profile(
594630
state: &State<'_, '_>,
595631
parent: &Unit,
596632
pkg: &Package,
597633
target: &Target,
634+
dependency: Option<Dependency>,
598635
unit_for: UnitFor,
599636
kind: CompileKind,
600637
mode: CompileMode,
@@ -616,6 +653,7 @@ fn new_unit_dep_with_profile(
616653
.intern(pkg, target, profile, kind, mode, features, state.is_std, 0);
617654
Ok(UnitDep {
618655
unit,
656+
dependency: UnitDependency(dependency),
619657
unit_for,
620658
extern_crate_name,
621659
public,

0 commit comments

Comments
 (0)