diff --git a/src/cargo/core/compiler/job_queue/job_state.rs b/src/cargo/core/compiler/job_queue/job_state.rs index 05b9627e4b8..378bca1fa07 100644 --- a/src/cargo/core/compiler/job_queue/job_state.rs +++ b/src/cargo/core/compiler/job_queue/job_state.rs @@ -9,6 +9,7 @@ use crate::core::compiler::locking::LockKey; use crate::core::compiler::timings::SectionTiming; use crate::util::Queue; use crate::util::context::WarningHandling; +use crate::util::interning::InternedString; use crate::{CargoResult, core::compiler::locking::LockManager}; use super::{Artifact, DiagDedupe, Job, JobId, Message}; @@ -224,7 +225,7 @@ impl<'a, 'gctx> JobState<'a, 'gctx> { /// /// This is useful for checking unused dependencies. /// Should only be called once, as the compiler only emits it once per compilation. - pub fn unused_externs(&self, unused_externs: Vec) { + pub fn unused_externs(&self, unused_externs: std::collections::BTreeSet) { self.messages .push(Message::UnusedExterns(self.id, unused_externs)); } diff --git a/src/cargo/core/compiler/job_queue/mod.rs b/src/cargo/core/compiler/job_queue/mod.rs index 203d886a00c..521df8bb919 100644 --- a/src/cargo/core/compiler/job_queue/mod.rs +++ b/src/cargo/core/compiler/job_queue/mod.rs @@ -149,6 +149,7 @@ use crate::util::CargoResult; use crate::util::context::WarningHandling; use crate::util::diagnostic_server::{self, DiagnosticPrinter}; use crate::util::errors::AlreadyPrintedError; +use crate::util::interning::InternedString; use crate::util::machine_message::{self, Message as _}; use crate::util::{self, internal}; use crate::util::{DependencyQueue, GlobalContext, Progress, ProgressStyle, Queue}; @@ -387,7 +388,7 @@ enum Message { Finish(JobId, Artifact, CargoResult<()>), FutureIncompatReport(JobId, Vec), SectionTiming(JobId, SectionTiming), - UnusedExterns(JobId, Vec), + UnusedExterns(JobId, std::collections::BTreeSet), } impl<'gctx> JobQueue<'gctx> { diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index d7446a3bc3d..5529652b6cc 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -2399,7 +2399,7 @@ fn on_stderr_line_inner( #[derive(serde::Deserialize)] struct UnusedExterns { - unused_extern_names: Vec, + unused_extern_names: std::collections::BTreeSet, } if let Ok(uext) = serde_json::from_str::(compiler_message.get()) { trace!( diff --git a/src/cargo/core/compiler/unused_deps.rs b/src/cargo/core/compiler/unused_deps.rs index 889f6f1a49f..212f203d990 100644 --- a/src/cargo/core/compiler/unused_deps.rs +++ b/src/cargo/core/compiler/unused_deps.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeSet; + use cargo_util_schemas::manifest; use cargo_util_terminal::report::AnnotationKind; use cargo_util_terminal::report::Group; @@ -84,7 +86,7 @@ impl UnusedDepState { .or_default() .entry(dep_kind) .or_default(); - *state.needed_units.get_or_insert_default() += 1; + state.needed_units += 1; for dep in build_runner.unit_deps(root).iter() { trace!( " => {} (deps={})", @@ -111,7 +113,11 @@ impl UnusedDepState { Self { states } } - pub fn record_unused_externs_for_unit(&mut self, unit: &Unit, unused_externs: Vec) { + pub fn record_unused_externs_for_unit( + &mut self, + unit: &Unit, + unused_externs: BTreeSet, + ) { let pkg_id = unit.pkg.package_id(); let dep_kind = dep_kind_of(unit); trace!( @@ -125,11 +131,12 @@ impl UnusedDepState { .or_default() .entry(dep_kind) .or_default(); - state - .unused_externs - .entry(unit.clone()) - .or_default() - .extend(unused_externs.into_iter().map(|s| InternedString::new(&s))); + state.seen_units.push(unit.clone()); + if let Some(existing) = state.unused_externs.as_mut() { + existing.retain(|ext| unused_externs.contains(ext)); + } else { + state.unused_externs = Some(unused_externs); + } } #[instrument(skip_all)] @@ -162,7 +169,7 @@ impl UnusedDepState { if lint_level == LintLevel::Allow { for (dep_kind, state) in states.iter() { - for ext in state.unused_externs.values().flatten() { + for ext in state.unused_externs.iter().flatten() { debug!( "pkg {} v{} ({dep_kind:?}): ignoring unused extern `{ext}`, lint is allowed", pkg_id.name(), @@ -176,47 +183,60 @@ impl UnusedDepState { let manifest_path = rel_cwd_manifest_path(pkg.manifest_path(), build_runner.bcx.gctx); let mut lint_count = 0; for (dep_kind, state) in states.iter() { - let Some(needed_units) = state.needed_units else { - // not one we care to report - for ext in state.unused_externs.values().flatten() { + for ext in state.unused_externs.iter().flatten() { + let mut used_in_dev = false; + match dep_kind { + DepKind::Normal => { + if let Some(state) = states.get(&DepKind::Development) + && state + .unused_externs + .as_ref() + .is_some_and(|ue| !ue.contains(ext)) + { + used_in_dev = true; + } + } + DepKind::Development => { + if let Some(state) = states.get(&DepKind::Normal) + && state.externs.contains_key(ext) + { + trace!( + "pkg {} v{} ({dep_kind:?}): ignoring unused extern `{ext}`, inherited from normal dependency", + pkg_id.name(), + pkg_id.version(), + ); + continue; + } + } + DepKind::Build => {} + } + let Some(extern_state) = state.externs.get(ext) else { + // not one we care to report debug!( "pkg {} v{} ({dep_kind:?}): ignoring unused extern `{ext}`, untracked dependent", pkg_id.name(), pkg_id.version(), ); - } - continue; - }; - if state.unused_externs.len() != needed_units { - // Some compilations errored without printing the unused externs. - // Don't print the warning in order to reduce false positive - // spam during errors. - for ext in state.unused_externs.values().flatten() { + continue; + }; + if state.seen_units.len() != state.needed_units { + debug_assert_ne!( + state.externs.len(), + 0, + "assumes tracked is checked first" + ); + // Some compilations errored without printing the unused externs. + // Don't print the warning in order to reduce false positive + // spam during errors. debug!( "pkg {} v{} ({dep_kind:?}): ignoring unused extern `{ext}`, {} outstanding units", pkg_id.name(), pkg_id.version(), - needed_units - state.unused_externs.len() - ); - } - continue; - } - - for (ext, extern_state) in &state.externs { - if state - .unused_externs - .values() - .any(|unused| !unused.contains(ext)) - { - trace!( - "pkg {} v{} ({dep_kind:?}): extern {} is used", - pkg_id.name(), - pkg_id.version(), - ext + state.needed_units - state.seen_units.len() ); continue; } - if is_transitive_dep(&extern_state.unit, &state.unused_externs, build_runner) { + if is_transitive_dep(&extern_state.unit, &state.seen_units, build_runner) { debug!( "pkg {} v{} ({dep_kind:?}): ignoring unused extern `{ext}`, may be activating features", pkg_id.name(), @@ -273,6 +293,12 @@ impl UnusedDepState { ); report.push(help); } + if used_in_dev { + let help = Group::with_title(Level::HELP.secondary_title( + "to still use for development builds, move to `dev-dependencies`", + )); + report.push(help); + } if lint_level.is_warn() { *warn_count += 1; @@ -296,7 +322,7 @@ impl UnusedDepState { let state = self.states.get(pkg_id)?; let mut iter = state.values(); let state = iter.next()?; - let mut iter = state.unused_externs.keys(); + let mut iter = state.seen_units.iter(); let unit = iter.next()?; Some(&unit.pkg) } @@ -307,13 +333,15 @@ impl UnusedDepState { struct DependenciesState { /// All declared dependencies externs: IndexMap, - /// Expected [`Self::unused_externs`] entries to know we've received them all + /// Expected [`Self::seen_units`] entries to know we've received them all /// /// To avoid warning in cases where we didn't, /// e.g. if a [`Unit`] errored and didn't report unused externs. - needed_units: Option, - /// As reported by rustc - unused_externs: IndexMap>, + needed_units: usize, + /// Units that have reported their unused externs + seen_units: Vec, + /// Intersection of unused externs across all [`Self::seen_units`] + unused_externs: Option>, } #[derive(Clone)] @@ -350,11 +378,11 @@ fn unit_desc(unit: &Unit) -> String { #[instrument(skip_all)] fn is_transitive_dep( direct_dep_unit: &Unit, - unused_externs: &IndexMap>, + seen_units: &Vec, build_runner: &mut BuildRunner<'_, '_>, ) -> bool { let mut queue = std::collections::VecDeque::new(); - for root_unit in unused_externs.keys() { + for root_unit in seen_units { for unit_dep in build_runner.unit_deps(root_unit) { if root_unit.pkg.package_id() == unit_dep.unit.pkg.package_id() { continue; diff --git a/tests/testsuite/lints/unused_dependencies.rs b/tests/testsuite/lints/unused_dependencies.rs index 76226d4d1d2..f141c8ad64d 100644 --- a/tests/testsuite/lints/unused_dependencies.rs +++ b/tests/testsuite/lints/unused_dependencies.rs @@ -373,7 +373,7 @@ fn unused_dep_normal_but_implicit_used_dep_dev() { "tests/foo.rs", r#" #[test] - fn foo { + fn foo() { use used_dev as _; } "#, @@ -402,6 +402,27 @@ fn unused_dep_normal_but_implicit_used_dep_dev() { | [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +"#]]) + .run(); + + p.cargo("check -Zcargo-lints --all-targets") + .masquerade_as_nightly_cargo(&["cargo-lints"]) + .with_stderr_data(str![[r#" +[CHECKING] foo v0.1.0 ([ROOT]/foo) +[WARNING] unused dependency + --> Cargo.toml:9:13 + | +9 | used_dev = "0.1.0" + | ^^^^^^^^^^^^^^^^^^ + | + = [NOTE] `cargo::unused_dependencies` is set to `warn` in `[lints]` +[HELP] remove the dependency + | +9 - used_dev = "0.1.0" + | +[HELP] to still use for development builds, move to `dev-dependencies` +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s + "#]]) .run(); }