Skip to content

Commit 84a6e0b

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 8957483 commit 84a6e0b

11 files changed

Lines changed: 802 additions & 5 deletions

File tree

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

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

src/cargo/core/dependency.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,17 @@ impl Dependency {
293293
self.inner.explicit_name_in_toml
294294
}
295295

296+
pub fn toml_path(&self) -> Vec<String> {
297+
let mut path = Vec::new();
298+
if let Some(platform) = self.platform() {
299+
path.push("target".to_owned());
300+
path.push(platform.to_string());
301+
}
302+
path.push(self.kind().kind_table().to_owned());
303+
path.push((&*self.name_in_toml()).to_owned());
304+
path
305+
}
306+
296307
pub fn set_kind(&mut self, kind: DepKind) -> &mut Dependency {
297308
if self.is_public() {
298309
// Setting 'public' only makes sense for normal dependencies

0 commit comments

Comments
 (0)