Skip to content

Commit db80b90

Browse files
committed
feat(lints): Emit the rest of unused_dependencies
Fixes #15813 Built-on #8437 This does nothing for dev-dependencies because there isn't really a way to select all targets today without - tracking selected dep kinds to check on a per-package basis - checking the status of every bench to see if it can work as a test because `cargo test` (no args) with benches set to test is the only command today that can exercise all dev-dependencies as it is the only one that will compile tests and doctests. See also - https://blog.rust-lang.org/inside-rust/2024/10/01/this-development-cycle-in-cargo-1.82/#detecting-unused-dependencies - https://blog.rust-lang.org/inside-rust/2024/10/01/this-development-cycle-in-cargo-1.82/#all-targets-and-doc-tests - https://blog.rust-lang.org/inside-rust/2024/10/31/this-development-cycle-in-cargo-1.83/#target-and-target
1 parent 4aeaba3 commit db80b90

11 files changed

Lines changed: 833 additions & 6 deletions

File tree

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use crate::core::compiler::CompileKind;
77
use crate::core::compiler::Unit;
88
use crate::core::compiler::UnitIndex;
99
use crate::core::compiler::unit_graph::UnitGraph;
10+
use crate::core::dependency::DepKind;
1011
use crate::core::profiles::Profiles;
1112
use crate::util::Rustc;
1213
use crate::util::context::GlobalContext;
@@ -64,7 +65,7 @@ pub struct BuildContext<'a, 'gctx> {
6465
/// Configuration information for a rustc build.
6566
pub build_config: &'a BuildConfig,
6667

67-
/// Associated [`DepKind`][crate::core::dependency::DepKind]s for root targets
68+
/// Associated [`DepKind`]s for root targets
6869
pub selected_dep_kinds: DepKindSet,
6970

7071
/// Extra compiler args for either `rustc` or `rustdoc`.
@@ -169,3 +170,13 @@ pub struct DepKindSet {
169170
pub normal: bool,
170171
pub dev: bool,
171172
}
173+
174+
impl DepKindSet {
175+
pub fn contains(&self, kind: DepKind) -> bool {
176+
match kind {
177+
DepKind::Build => self.build,
178+
DepKind::Normal => self.normal,
179+
DepKind::Development => self.dev,
180+
}
181+
}
182+
}

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ use super::UnitIndex;
138138
use super::custom_build::Severity;
139139
use super::timings::SectionTiming;
140140
use super::timings::Timings;
141+
use super::unused_deps::UnusedDepState;
141142
use crate::core::compiler::descriptive_pkg_name;
142143
use crate::core::compiler::future_incompat::{
143144
self, FutureBreakageItem, FutureIncompatReportPackage,
@@ -186,6 +187,7 @@ struct DrainState<'gctx> {
186187
progress: Progress<'gctx>,
187188
next_id: u32,
188189
timings: Timings<'gctx>,
190+
unused_dep_state: UnusedDepState,
189191

190192
/// Map from unit index to unit, for looking up dependency information.
191193
index_to_unit: HashMap<UnitIndex, Unit>,
@@ -504,6 +506,7 @@ impl<'gctx> JobQueue<'gctx> {
504506
progress,
505507
next_id: 0,
506508
timings: self.timings,
509+
unused_dep_state: UnusedDepState::new(build_runner),
507510
index_to_unit: build_runner
508511
.bcx
509512
.unit_to_index
@@ -721,8 +724,10 @@ impl<'gctx> DrainState<'gctx> {
721724
items,
722725
});
723726
}
724-
Message::UnusedExterns(id, _unused_externs) => {
725-
let _unit = &self.active[&id];
727+
Message::UnusedExterns(id, unused_externs) => {
728+
let unit = &self.active[&id];
729+
self.unused_dep_state
730+
.record_unused_externs_for_unit(unit, unused_externs);
726731
}
727732
Message::Token(acquired_token) => {
728733
let token = acquired_token.context("failed to acquire jobserver token")?;
@@ -816,6 +821,18 @@ impl<'gctx> DrainState<'gctx> {
816821
}
817822
self.progress.clear();
818823

824+
if build_runner.bcx.gctx.cli_unstable().cargo_lints {
825+
let mut warn_count = 0;
826+
let mut error_count = 0;
827+
drop(self.unused_dep_state.emit_unused_warnings(
828+
&mut warn_count,
829+
&mut error_count,
830+
build_runner,
831+
));
832+
errors.count += error_count;
833+
build_runner.compilation.lint_warning_count += warn_count;
834+
}
835+
819836
let profile_name = build_runner.bcx.build_config.requested_profile;
820837
// NOTE: this may be a bit inaccurate, since this may not display the
821838
// profile for what was actually built. Profile overrides can change

src/cargo/core/compiler/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ pub mod timings;
5151
mod unit;
5252
pub mod unit_dependencies;
5353
pub mod unit_graph;
54+
mod unused_deps;
5455

5556
use std::borrow::Cow;
5657
use std::cell::OnceCell;
Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
use annotate_snippets::AnnotationKind;
2+
use annotate_snippets::Group;
3+
use annotate_snippets::Level;
4+
use annotate_snippets::Origin;
5+
use annotate_snippets::Patch;
6+
use annotate_snippets::Snippet;
7+
use cargo_util_schemas::manifest;
8+
use indexmap::IndexMap;
9+
use indexmap::IndexSet;
10+
use tracing::trace;
11+
12+
use super::BuildRunner;
13+
use super::unit::Unit;
14+
use crate::core::Dependency;
15+
use crate::core::Package;
16+
use crate::core::PackageId;
17+
use crate::core::compiler::build_config::CompileMode;
18+
use crate::core::dependency::DepKind;
19+
use crate::core::manifest::TargetKind;
20+
use crate::lints::LintLevel;
21+
use crate::lints::get_key_value_span;
22+
use crate::lints::rel_cwd_manifest_path;
23+
use crate::lints::rules::unused_dependencies::LINT;
24+
use crate::util::errors::CargoResult;
25+
use crate::util::interning::InternedString;
26+
27+
/// Track and translate `unused_externs` to `unused_dependencies`
28+
pub struct UnusedDepState {
29+
states: IndexMap<PackageId, IndexMap<DepKind, DependenciesState>>,
30+
}
31+
32+
impl UnusedDepState {
33+
pub fn new(build_runner: &mut BuildRunner<'_, '_>) -> Self {
34+
let mut states = IndexMap::<_, IndexMap<_, DependenciesState>>::new();
35+
36+
let roots = &build_runner.bcx.roots;
37+
38+
// Find all units for a package that can report unused externs
39+
let mut root_build_script_builds = IndexSet::new();
40+
for root in roots.iter() {
41+
for build_script_run in build_runner.unit_deps(root).iter() {
42+
if !build_script_run.unit.target.is_custom_build()
43+
&& build_script_run.unit.pkg.package_id() != root.pkg.package_id()
44+
{
45+
continue;
46+
}
47+
for build_script_build in build_runner.unit_deps(&build_script_run.unit).iter() {
48+
if !build_script_build.unit.target.is_custom_build()
49+
&& build_script_build.unit.pkg.package_id() != root.pkg.package_id()
50+
{
51+
continue;
52+
}
53+
if build_script_build.unit.mode != CompileMode::Build {
54+
continue;
55+
}
56+
root_build_script_builds.insert(build_script_build.unit.clone());
57+
}
58+
}
59+
}
60+
61+
trace!(
62+
"selected dep kinds: {:?}",
63+
build_runner.bcx.selected_dep_kinds
64+
);
65+
for root in roots.iter().chain(root_build_script_builds.iter()) {
66+
let pkg_id = root.pkg.package_id();
67+
let dep_kind = dep_kind_of(root);
68+
if !build_runner.bcx.selected_dep_kinds.contains(dep_kind) {
69+
trace!(
70+
"pkg {} v{} ({dep_kind:?}): ignoring unused deps due to non-exhaustive units",
71+
pkg_id.name(),
72+
pkg_id.version(),
73+
);
74+
continue;
75+
}
76+
trace!(
77+
"tracking root {} {} ({:?})",
78+
root.pkg.name(),
79+
unit_desc(root),
80+
dep_kind
81+
);
82+
83+
let state = states
84+
.entry(pkg_id)
85+
.or_default()
86+
.entry(dep_kind)
87+
.or_default();
88+
state.needed_units += 1;
89+
for dep in build_runner.unit_deps(root).iter() {
90+
trace!(
91+
" => {} (deps={})",
92+
dep.unit.pkg.name(),
93+
dep.manifest_deps.0.is_some()
94+
);
95+
let manifest_deps = if let Some(manifest_deps) = &dep.manifest_deps.0 {
96+
Some(manifest_deps.clone())
97+
} else if dep.unit.pkg.package_id() == root.pkg.package_id() {
98+
None
99+
} else {
100+
continue;
101+
};
102+
state.externs.insert(dep.extern_crate_name, manifest_deps);
103+
}
104+
}
105+
106+
Self { states }
107+
}
108+
109+
pub fn record_unused_externs_for_unit(&mut self, unit: &Unit, unused_externs: Vec<String>) {
110+
let pkg_id = unit.pkg.package_id();
111+
let kind = dep_kind_of(unit);
112+
if let Some(state) = self.states.get_mut(&pkg_id).and_then(|s| s.get_mut(&kind)) {
113+
state
114+
.unused_externs
115+
.entry(unit.clone())
116+
.or_default()
117+
.extend(unused_externs.into_iter().map(|s| InternedString::new(&s)));
118+
}
119+
}
120+
121+
pub fn emit_unused_warnings(
122+
&self,
123+
warn_count: &mut usize,
124+
error_count: &mut usize,
125+
build_runner: &mut BuildRunner<'_, '_>,
126+
) -> CargoResult<()> {
127+
for (pkg_id, states) in &self.states {
128+
let Some(pkg) = self.get_package(pkg_id) else {
129+
continue;
130+
};
131+
let toml_lints = pkg
132+
.manifest()
133+
.normalized_toml()
134+
.lints
135+
.clone()
136+
.map(|lints| lints.lints)
137+
.unwrap_or(manifest::TomlLints::default());
138+
let cargo_lints = toml_lints
139+
.get("cargo")
140+
.cloned()
141+
.unwrap_or(manifest::TomlToolLints::default());
142+
let (lint_level, reason) = LINT.level(
143+
&cargo_lints,
144+
pkg.manifest().edition(),
145+
pkg.manifest().unstable_features(),
146+
);
147+
148+
if lint_level == LintLevel::Allow {
149+
continue;
150+
}
151+
152+
let manifest_path = rel_cwd_manifest_path(pkg.manifest_path(), build_runner.bcx.gctx);
153+
let mut lint_count = 0;
154+
for (dep_kind, state) in states.iter() {
155+
if state.unused_externs.len() != state.needed_units {
156+
// Some compilations errored without printing the unused externs.
157+
// Don't print the warning in order to reduce false positive
158+
// spam during errors.
159+
trace!(
160+
"pkg {} v{} ({dep_kind:?}): ignoring unused deps due to {} outstanding units",
161+
pkg_id.name(),
162+
pkg_id.version(),
163+
state.needed_units
164+
);
165+
continue;
166+
}
167+
168+
for (ext, dependency) in &state.externs {
169+
if state
170+
.unused_externs
171+
.values()
172+
.any(|unused| !unused.contains(ext))
173+
{
174+
trace!(
175+
"pkg {} v{} ({dep_kind:?}): extern {} is used",
176+
pkg_id.name(),
177+
pkg_id.version(),
178+
ext
179+
);
180+
continue;
181+
}
182+
183+
// Implicitly added dependencies (in the same crate) aren't interesting
184+
let dependency = if let Some(dependency) = dependency {
185+
dependency
186+
} else {
187+
continue;
188+
};
189+
for dependency in dependency {
190+
let manifest = pkg.manifest();
191+
let document = manifest.document();
192+
let contents = manifest.contents();
193+
let level = lint_level.to_diagnostic_level();
194+
let emitted_source = LINT.emitted_source(lint_level, reason);
195+
let toml_path = dependency.toml_path();
196+
197+
let mut primary = Group::with_title(level.primary_title(LINT.desc));
198+
if let Some(document) = document
199+
&& let Some(contents) = contents
200+
&& let Some(span) = get_key_value_span(document, &toml_path)
201+
{
202+
let span = span.key.start..span.value.end;
203+
primary = primary.element(
204+
Snippet::source(contents)
205+
.path(&manifest_path)
206+
.annotation(AnnotationKind::Primary.span(span)),
207+
);
208+
} else {
209+
primary = primary.element(Origin::path(&manifest_path));
210+
}
211+
if lint_count == 0 {
212+
primary = primary.element(Level::NOTE.message(emitted_source));
213+
}
214+
lint_count += 1;
215+
let mut report = vec![primary];
216+
if let Some(document) = document
217+
&& let Some(contents) = contents
218+
&& let Some(span) = get_key_value_span(document, &toml_path)
219+
{
220+
let span = span.key.start..span.value.end;
221+
let mut help = Group::with_title(
222+
Level::HELP.secondary_title("remove the dependency"),
223+
);
224+
help = help.element(
225+
Snippet::source(contents)
226+
.path(&manifest_path)
227+
.patch(Patch::new(span, "")),
228+
);
229+
report.push(help);
230+
}
231+
232+
if lint_level.is_warn() {
233+
*warn_count += 1;
234+
}
235+
if lint_level.is_error() {
236+
*error_count += 1;
237+
}
238+
build_runner
239+
.bcx
240+
.gctx
241+
.shell()
242+
.print_report(&report, lint_level.force())?;
243+
}
244+
}
245+
}
246+
}
247+
Ok(())
248+
}
249+
250+
fn get_package(&self, pkg_id: &PackageId) -> Option<&Package> {
251+
let state = self.states.get(pkg_id)?;
252+
let mut iter = state.values();
253+
let state = iter.next()?;
254+
let mut iter = state.unused_externs.keys();
255+
let unit = iter.next()?;
256+
Some(&unit.pkg)
257+
}
258+
}
259+
260+
/// Track a package's [`DepKind`]
261+
#[derive(Default)]
262+
struct DependenciesState {
263+
/// All declared dependencies
264+
externs: IndexMap<InternedString, Option<Vec<Dependency>>>,
265+
/// Expected [`Self::unused_externs`] entries to know we've received them all
266+
///
267+
/// To avoid warning in cases where we didn't,
268+
/// e.g. if a [`Unit`] errored and didn't report unused externs.
269+
needed_units: usize,
270+
/// As reported by rustc
271+
unused_externs: IndexMap<Unit, Vec<InternedString>>,
272+
}
273+
274+
fn dep_kind_of(unit: &Unit) -> DepKind {
275+
match unit.target.kind() {
276+
TargetKind::Lib(_) => match unit.mode {
277+
// To support lib.rs with #[cfg(test)] use foo_crate as _;
278+
CompileMode::Test => DepKind::Development,
279+
_ => DepKind::Normal,
280+
},
281+
TargetKind::Bin => DepKind::Normal,
282+
TargetKind::Test => DepKind::Development,
283+
TargetKind::Bench => DepKind::Development,
284+
TargetKind::ExampleLib(_) => DepKind::Development,
285+
TargetKind::ExampleBin => DepKind::Development,
286+
TargetKind::CustomBuild => DepKind::Build,
287+
}
288+
}
289+
290+
fn unit_desc(unit: &Unit) -> String {
291+
format!(
292+
"{}/{}+{:?}",
293+
unit.target.name(),
294+
unit.target.kind().description(),
295+
unit.mode,
296+
)
297+
}

0 commit comments

Comments
 (0)