feat(lints): Emit unused_dependencies lint (#16600)

*[View all
comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/cargo/pull/16600)*

### What does this PR try to resolve?

Fixes #15813

```toml
[lints.cargo]
unused_dependencies = "warn"
```

This checks only build and normal dependencies (see below for more
details). I would expect we'd open a new issue to track
dev-dependencies. I don't consider this a false-positive because there
technically isn't a way to ask for everything that uses dev-dependencies
to be built.

This only checks selected packages, and not all local packages, and only
if the build target selection flags are exhaustive without consideration
for the selected packages. See
#16600 (comment)

Known false positives:
- activating a feature on a transitive dependency
- pinning a transitive dependency (instead use
`target.cfg(false).dependencies`)

To handle false positives,
```toml
[lints.cargo]
unused_dependencies = { level = "warn", ignore = ["dep_name"] }
```
- Some prior art use `ignored` but I felt like `ignore` fit in better
with `allow`
- Per-dep-table is more complicated for users, tricky to get the design
right (`udeps` cares about dep kinds but not target cfgs), these
shouldn't be too common, and using dep names gives users control over
uniqueness
- For more background on alternatives, see
rust-lang/rfcs#3920

### How to test and review this PR?

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

As for the commits, I did something unusual for myself in that I made
changes with dead code so it is easier to understand what goes into one
step in this process without seeing the entire process at once and
getting confused.

Author: ehuss

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

@@ -7,6 +7,7 @@ use crate::core::compiler::CompileKind;
 use crate::core::compiler::Unit;
 use crate::core::compiler::UnitIndex;
 use crate::core::compiler::unit_graph::UnitGraph;
+use crate::core::dependency::DepKind;
 use crate::core::profiles::Profiles;
 use crate::util::Rustc;
 use crate::util::context::GlobalContext;
@@ -64,6 +65,9 @@ pub struct BuildContext<'a, 'gctx> {
     /// Configuration information for a rustc build.
     pub build_config: &'a BuildConfig,
 
+    /// Associated [`DepKind`]s for root targets
+    pub selected_dep_kinds: DepKindSet,
+
     /// Extra compiler args for either `rustc` or `rustdoc`.
     pub extra_compiler_args: HashMap<Unit, Vec<String>>,
 
@@ -97,6 +101,7 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
         logger: Option<&'a BuildLogger>,
         packages: PackageSet<'gctx>,
         build_config: &'a BuildConfig,
+        selected_dep_kinds: DepKindSet,
         profiles: Profiles,
         extra_compiler_args: HashMap<Unit, Vec<String>>,
         target_data: RustcTargetData<'gctx>,
@@ -118,6 +123,7 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
             logger,
             packages,
             build_config,
+            selected_dep_kinds,
             profiles,
             extra_compiler_args,
             target_data,
@@ -157,3 +163,20 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
         self.extra_compiler_args.get(unit)
     }
 }
+
+#[derive(Copy, Clone, Default, Debug)]
+pub struct DepKindSet {
+    pub build: bool,
+    pub normal: bool,
+    pub dev: bool,
+}
+
+impl DepKindSet {
+    pub fn contains(&self, kind: DepKind) -> bool {
+        match kind {
+            DepKind::Build => self.build,
+            DepKind::Normal => self.normal,
+            DepKind::Development => self.dev,
+        }
+    }
+}

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

@@ -219,4 +219,13 @@ impl<'a, 'gctx> JobState<'a, 'gctx> {
         self.messages
             .push(Message::FutureIncompatReport(self.id, report));
     }
+
+    /// The rustc emitted the list of unused `--extern` args.
+    ///
+    /// 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<String>) {
+        self.messages
+            .push(Message::UnusedExterns(self.id, unused_externs));
+    }
 }

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

@@ -138,6 +138,7 @@ use super::UnitIndex;
 use super::custom_build::Severity;
 use super::timings::SectionTiming;
 use super::timings::Timings;
+use super::unused_deps::UnusedDepState;
 use crate::core::compiler::descriptive_pkg_name;
 use crate::core::compiler::future_incompat::{
     self, FutureBreakageItem, FutureIncompatReportPackage,
@@ -187,6 +188,7 @@ struct DrainState<'gctx> {
     progress: Progress<'gctx>,
     next_id: u32,
     timings: Timings<'gctx>,
+    unused_dep_state: UnusedDepState,
 
     /// Map from unit index to unit, for looking up dependency information.
     index_to_unit: HashMap<UnitIndex, Unit>,
@@ -385,6 +387,7 @@ enum Message {
     Finish(JobId, Artifact, CargoResult<()>),
     FutureIncompatReport(JobId, Vec<FutureBreakageItem>),
     SectionTiming(JobId, SectionTiming),
+    UnusedExterns(JobId, Vec<String>),
 }
 
 impl<'gctx> JobQueue<'gctx> {
@@ -504,6 +507,7 @@ impl<'gctx> JobQueue<'gctx> {
             progress,
             next_id: 0,
             timings: self.timings,
+            unused_dep_state: UnusedDepState::new(build_runner),
             index_to_unit: build_runner
                 .bcx
                 .unit_to_index
@@ -544,12 +548,10 @@ impl<'gctx> JobQueue<'gctx> {
             .take()
             .map(move |srv| srv.start(move |msg| messages.push(Message::FixDiagnostic(msg))));
 
-        thread::scope(
-            move |scope| match state.drain_the_queue(build_runner, scope, &helper) {
-                Some(err) => Err(err),
-                None => Ok(()),
-            },
-        )
+        thread::scope(move |scope| {
+            let (result,) = state.drain_the_queue(build_runner, scope, &helper);
+            result
+        })
     }
 }
 
@@ -743,6 +745,11 @@ impl<'gctx> DrainState<'gctx> {
                         items,
                     });
             }
+            Message::UnusedExterns(id, unused_externs) => {
+                let unit = &self.active[&id];
+                self.unused_dep_state
+                    .record_unused_externs_for_unit(unit, unused_externs);
+            }
             Message::Token(acquired_token) => {
                 let token = acquired_token.context("failed to acquire jobserver token")?;
                 self.tokens.push(token);
@@ -783,14 +790,15 @@ impl<'gctx> DrainState<'gctx> {
     /// This is the "main" loop, where Cargo does all work to run the
     /// compiler.
     ///
-    /// This returns an Option to prevent the use of `?` on `Result` types
-    /// because it is important for the loop to carefully handle errors.
+    /// This returns a tuple of `Result` to prevent the use of `?` on
+    /// `Result` types because it is important for the loop to
+    /// carefully handle errors.
     fn drain_the_queue<'s>(
         mut self,
         build_runner: &mut BuildRunner<'_, '_>,
         scope: &'s Scope<'s, '_>,
         jobserver_helper: &HelperThread,
-    ) -> Option<anyhow::Error> {
+    ) -> (Result<(), anyhow::Error>,) {
         trace!("queue: {:#?}", self.queue);
 
         // Iteratively execute the entire dependency graph. Each turn of the
@@ -834,6 +842,18 @@ impl<'gctx> DrainState<'gctx> {
         }
         self.progress.clear();
 
+        if build_runner.bcx.gctx.cli_unstable().cargo_lints {
+            let mut warn_count = 0;
+            let mut error_count = 0;
+            drop(self.unused_dep_state.emit_unused_warnings(
+                &mut warn_count,
+                &mut error_count,
+                build_runner,
+            ));
+            errors.count += error_count;
+            build_runner.compilation.lint_warning_count += warn_count;
+        }
+
         let profile_name = build_runner.bcx.build_config.requested_profile;
         // NOTE: this may be a bit inaccurate, since this may not display the
         // profile for what was actually built. Profile overrides can change
@@ -874,7 +894,7 @@ impl<'gctx> DrainState<'gctx> {
         if let Some(error) = errors.to_error() {
             // Any errors up to this point have already been printed via the
             // `display_error` inside `handle_error`.
-            Some(anyhow::Error::new(AlreadyPrintedError::new(error)))
+            (Err(anyhow::Error::new(AlreadyPrintedError::new(error))),)
         } else if self.queue.is_empty() && self.pending_queue.is_empty() {
             let profile_link = build_runner.bcx.gctx.shell().err_hyperlink(
                 "https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles",
@@ -889,10 +909,10 @@ impl<'gctx> DrainState<'gctx> {
                 &self.per_package_future_incompat_reports,
             );
 
-            None
+            (Ok(()),)
         } else {
             debug!("queue: {:#?}", self.queue);
-            Some(internal("finished with jobs still left in the queue"))
+            (Err(internal("finished with jobs still left in the queue")),)
         }
     }
 

src/cargo/core/compiler/mod.rs

@@ -51,6 +51,7 @@ pub mod timings;
 mod unit;
 pub mod unit_dependencies;
 pub mod unit_graph;
+mod unused_deps;
 
 use std::borrow::Cow;
 use std::cell::OnceCell;
@@ -74,6 +75,7 @@ use tracing::{debug, instrument, trace};
 pub use self::build_config::UserIntent;
 pub use self::build_config::{BuildConfig, CompileMode, MessageFormat};
 pub use self::build_context::BuildContext;
+pub use self::build_context::DepKindSet;
 pub use self::build_context::FileFlavor;
 pub use self::build_context::FileType;
 pub use self::build_context::RustcTargetData;
@@ -832,6 +834,12 @@ fn prepare_rustc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResult
         base.env("CARGO_TARGET_TMPDIR", tmp.display().to_string());
     }
 
+    if build_runner.bcx.gctx.cli_unstable().cargo_lints {
+        // Added last to reduce the risk of RUSTFLAGS or `[lints]` from interfering with
+        // `unused_dependencies` tracking
+        base.arg("-Wunused_crate_dependencies");
+    }
+
     Ok(base)
 }
 
@@ -1175,8 +1183,11 @@ fn add_error_format_and_color(build_runner: &BuildRunner<'_, '_>, cmd: &mut Proc
     }
 
     cmd.arg("--error-format=json");
-    let mut json = String::from("--json=diagnostic-rendered-ansi,artifacts,future-incompat");
 
+    let mut json = String::from("--json=diagnostic-rendered-ansi,artifacts,future-incompat");
+    if build_runner.bcx.gctx.cli_unstable().cargo_lints {
+        json.push_str(",unused-externs-silent");
+    }
     if let MessageFormat::Short | MessageFormat::Json { short: true, .. } =
         build_runner.bcx.build_config.message_format
     {
@@ -1186,11 +1197,9 @@ fn add_error_format_and_color(build_runner: &BuildRunner<'_, '_>, cmd: &mut Proc
     {
         json.push_str(",diagnostic-unicode");
     }
-
     if enable_timings {
         json.push_str(",timings");
     }
-
     cmd.arg(json);
 
     let gctx = build_runner.bcx.gctx;
@@ -2376,6 +2385,19 @@ fn on_stderr_line_inner(
         return Ok(false);
     }
 
+    #[derive(serde::Deserialize)]
+    struct UnusedExterns {
+        unused_extern_names: Vec<String>,
+    }
+    if let Ok(uext) = serde_json::from_str::<UnusedExterns>(compiler_message.get()) {
+        trace!(
+            "obtained unused externs list from rustc: `{:?}`",
+            uext.unused_extern_names
+        );
+        state.unused_externs(uext.unused_extern_names);
+        return Ok(true);
+    }
+
     // And failing all that above we should have a legitimate JSON diagnostic
     // from the compiler, so wrap it in an external Cargo JSON message
     // indicating which package it came from and then emit it.