feat(lints): Emit unused_dependencies lint#16600

Draft

epage wants to merge 10 commits intorust-lang:masterfrom

Contributor

epage commented Feb 6, 2026 •

edited

Loading

What does this PR try to resolve?

Fixes #15813

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)

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

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.

rustbot assigned ehuss

rustbot added A-build-execution A-cargo-targets A-crate-dependencies A-workspaces S-waiting-on-review labels

Collaborator

rustbot commented Feb 6, 2026

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @ehuss, @epage, @weihanglo
@ehuss, @epage, @weihanglo expanded to ehuss, epage, weihanglo
Random selection from ehuss, weihanglo

epage marked this pull request as draft

February 6, 2026 22:02

rustbot removed the S-waiting-on-review label

epage commented

View reviewed changes

tests/testsuite/lints/unused_workspace_dependencies.rs

Comment on lines +102 to +112

+              [WARNING] unused dependency
+               --> bar/Cargo.toml:9:1
+                |
+| build-dep.workspace = true
+                | ^^^^^^^^^
+                |
+                = [NOTE] `cargo::unused_dependencies` is set to `warn` by default
+              [HELP] remove the dependency
+                |
+- build-dep.workspace = true
++ .workspace = true

Contributor Author

epage Feb 6, 2026

Our selecting of key-values that use workspace inheritance is not great

epage commented

View reviewed changes

tests/testsuite/lints/unused_dependencies.rs

Comment on lines +45 to +56

+              [WARNING] unused dependency
+               --> Cargo.toml:9:13
+                |
+|             unused = "0.1.0"
+                |             ^^^^^^^^^^^^^^^^
+                |
+                = [NOTE] `cargo::unused_dependencies` is set to `warn` by default
+              [HELP] remove the dependency
+                |
+-             unused = "0.1.0"
+                |
+              [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Contributor Author

epage Feb 6, 2026

As this happens during compilation, ideally we tie into the existing warning/error count and work with build.warnings but haven't gotten all of that to work yet.

epage commented

View reviewed changes

tests/testsuite/lints/unused_dependencies.rs

Comment on lines +153 to +169

+              [WARNING] unused dependency
+               --> Cargo.toml:9:13
+                |
+|             unused = "0.1.0"
+                |             ^^^^^^^^^^^^^^^^
+                |
+                = [NOTE] `cargo::unused_dependencies` is set to `warn` by default
+              [HELP] remove the dependency
+                |
+-             unused = "0.1.0"
+                |
+              [UPDATING] `dummy-registry` index
+              [LOCKING] 1 package to latest compatible version
+              [DOWNLOADING] crates ...
+              [DOWNLOADED] unused v0.1.0 (registry `dummy-registry`)
+              [CHECKING] foo v0.1.0 ([ROOT]/foo)
+              [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s

Contributor Author

epage Feb 6, 2026

Note that this looks different than the others because there is no build.rs so there is no Unit to report the message, so we lint this during the regular lint pass.

epage commented

View reviewed changes

tests/testsuite/lints/unused_dependencies.rs

Comment on lines +192 to +195

+                          [dependencies]
+                          unused = "0.1.0"
+                          lib_used = "0.1.0"
+                          bins_used = "0.1.0"

Contributor Author

epage Feb 6, 2026

side note: I would love to find a way to tell that we should lint that bins_used should be moved to a feature that the bins require so we can help people cut down on accidentally including bin deps with their libs.

epage commented

View reviewed changes

tests/testsuite/lints/unused_dependencies.rs

Comment on lines +359 to +379

+                          [dependencies]
+                          used_dev = "0.1.0"
+                          [lints.cargo]
+                          unused_dependencies = "warn"
+                      "#,
+                      )
+                      .file(
+                          "src/main.rs",
+                          r#"
+                          fn main() {}
+                          "#,
+                      )
+                      .file(
+                          "tests/foo.rs",
+                          r#"
+                          #[test]
+                          fn foo {
+                              use used_dev as _;
+                          }
+                          "#,

Contributor Author

epage Feb 6, 2026

Ideally this would provide a hint that used_dev should be moved to dev-dependencies. Based on the code, it seems like #8437 supported that but haven't dug into what all it takes to report it.

epage commented

View reviewed changes

tests/testsuite/lints/unused_dependencies.rs

Comment on lines +1017 to +1021

+              [HELP] remove the dependency
+                 |
+-             bar.path = "../bar"
++             .path = "../bar"
+                 |

Contributor Author

epage Feb 6, 2026

More problems with how we get spans

epage commented

View reviewed changes

tests/testsuite/lints/unused_dependencies.rs Show resolved Hide resolved

Contributor Author

epage commented Feb 6, 2026

epage force-pushed the unused branch from de9e431 to 84a6e0b Compare

February 6, 2026 22:16

epage commented

View reviewed changes

src/cargo/lints/rules/unused_dependencies.rs Outdated

+              use crate::lints::get_key_value_span;
+              use crate::lints::rel_cwd_manifest_path;
+              pub const LINT: Lint = Lint {

Contributor Author

epage Feb 6, 2026

I didn't do anything in particular for artifact dependencies. I assume that I would just put it on the artifact-deps tracking issue as something to investigate.

epage commented

View reviewed changes

src/cargo/lints/rules/unused_dependencies.rs Outdated

+              use crate::lints::get_key_value_span;
+              use crate::lints::rel_cwd_manifest_path;
+              pub const LINT: Lint = Lint {

Contributor Author

epage Feb 6, 2026

One false positive I found is with cargo itself: we don't directly use libgit2-sys but we indirectly use it and activate a feature of it. We do that with our features table but that could just as well be done by just setting a feature on the dependency

Contributor Author

epage Feb 6, 2026

For a potential workaround, see https://github.com/rust-lang/cargo/pull/16600/changes#r2776252783

Contributor Author

epage Feb 10, 2026 •

edited

Loading

We talked about this in the cargo team meeting.

Options

A lint config
- Doesn't compose well with workspace inheritance without the ability to extend what we inherit (Packages overriding inherited lints in Cargo.toml + adding lints #13157)
- distant from the affected data
_ prefix
- Matches rust
- We were concerned about using _ to mark hidden features (Don't display "hidden" features in cargo add #10794)
- Gets weird with kebab-case dependencies
- Affects sort order
A used = true field with maybe used.reason = "feature activation"
- Similar to #[allow] or #[expect] in keeping it local
- TOML syntax however doesn't allow general lint control
- Extending our schema for general lint control could be messy
- Question is whether having one-off lint control for this is justified

Contributor Author

epage Feb 10, 2026

https://github.com/est31/cargo-udeps?tab=readme-ov-file#ignoring-some-of-the-dependencies

[package.metadata.cargo-udeps.ignore]
normal = ["if_chain"]
#development = []
#build = []

[dependencies]
if_chain = "1.0.0" # Used only in doc-tests, which `cargo-udeps` cannot check.

Contributor Author

epage Feb 10, 2026 •

edited

Loading

https://github.com/bnjbvr/cargo-machete?tab=readme-ov-file#false-positives

[dependencies]
prost = "0.10" # Used in code generated by build.rs output, which cargo-machete cannot check

# in an individual package Cargo.toml
[package.metadata.cargo-machete]
ignored = ["prost"]

# in a workspace Cargo.toml
[workspace.metadata.cargo-machete]
ignored = ["prost"]

Contributor Author

epage Feb 10, 2026

https://github.com/Boshen/cargo-shear

[package.metadata.cargo-shear]
ignored = ["crate-name"]

Contributor Author

epage Feb 13, 2026

Moved this to rust-lang/rfcs#3920

epage commented

View reviewed changes

src/cargo/core/compiler/unused_deps.rs

+                                  continue;
+                              }
+                              for (ext, dependency) in &state.externs {

Contributor Author

epage Feb 6, 2026

#8437 would automatically skip any dependency of the form:

_foo = { package = "foo", ...}

I removed that but we can consider adding it back in.

Contributor Author

epage Feb 6, 2026

See #16600 (comment) for a potential use case

epage force-pushed the unused branch from 84a6e0b to 960acbc Compare

February 6, 2026 22:22

rustbot added the A-documenting-cargo-itself label

epage force-pushed the unused branch from 960acbc to 4ce3794 Compare

February 6, 2026 22:35

epage commented

View reviewed changes

src/cargo/core/compiler/unused_deps.rs

Comment on lines +130 to +140

+                          let toml_lints = pkg
+                              .manifest()
+                              .normalized_toml()
+                              .lints
+                              .clone()
+                              .map(|lints| lints.lints)
+                              .unwrap_or(manifest::TomlLints::default());
+                          let cargo_lints = toml_lints
+                              .get("cargo")
+                              .cloned()
+                              .unwrap_or(manifest::TomlToolLints::default());

Contributor Author

epage Feb 6, 2026

During our regular lint pass, we calculate this and then pass it into each lint rule. Here, we have to re-calculate it and would likely need to do it again for each additional lint.

T-256 mentioned this pull request

Linting public reexport of private dependencies rust-lang/rust#143856

Merged

T-256 commented Feb 9, 2026

side-question (if is acceptable): rust-lang/rust#143856 (comment)

Member

weihanglo commented Feb 9, 2026

@T-256 see rust-lang/rust#143856 (comment).

epage force-pushed the unused branch 2 times, most recently from b60a278 to bb24b5e Compare

February 9, 2026 18:39

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-author label

ehuss reviewed

View reviewed changes

Contributor

ehuss left a comment •

edited by rustbot

Loading

Just a partial review for now, still working through this.

It would be helpful to have some doc comments on some of the new types and pub functions just to give them some context.

View changes since this review

src/cargo/core/compiler/mod.rs

                   }
+                  if build_runner.bcx.gctx.cli_unstable().cargo_lints {
+                      base.arg("-Wunused_crate_dependencies");

Contributor

ehuss Feb 10, 2026

Is this intentionally placed after the call to extra_args_for? I'm just wondering about the interaction of things like cargo rustc -- -Dwarnings or other such flags that could conflict with this.

Contributor Author

epage Feb 10, 2026

My intention was to have this very last so that any user provided modification of unused_crate_dependencies would be overridden unless it was in source code which we can't do anything about.

If there are some particular cases you'd like covered to see how they behave, I can add them.

est31 and others added 10 commits

February 10, 2026 13:35


          refactor(compile): Return (Result<(),E>,) in drain_the_queue

30c2ee5

This refactors the drain_the_queue function to return a
tuple containing Result. That way, it's still not
possible to use ? or try! to handle errors,
but for readers of the function declaration it's
clearer now that the error actually indicates one.

Bonus: it makes the calling code of drain_the_queue simpler.


          refactor(compile): Logically group json error format

fffd415


          test(lints): Add cases for unused_dependencies

2a3144e


          feat(lints): Add unused_dependencies lint definition

ccb8c49


          feat(lints): Check for unused build-dependencies without build.rs

74f23f3


          refactor(compile): Track selected dep kinds


          refactor(compile): Observe unused externs

20c6cbc


          refactor(utils): Add an Unhashed

307b363


          refactor(compile): Track manifest deps in unit graph

4aeaba3


          feat(lints): Emit the rest of unused_dependencies

db80b90

Fixes rust-lang#15813

Built-on rust-lang#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

epage force-pushed the unused branch from bb24b5e to db80b90 Compare

February 10, 2026 20:15

Contributor Author

epage commented Feb 10, 2026

It would be helpful to have some doc comments on some of the new types and pub functions just to give them some context.

I've added doc comments where I thought they may be helpful

epage changed the title ~~feat(lints): Emit the rest of unused_dependencies~~ feat(lints): Emit unused_dependencies lint

ehuss reviewed

View reviewed changes

Contributor

ehuss left a comment •

edited by rustbot

Loading

Looks good! I think this will be really useful. I don't have any particularly helpful review comments. I imagine as we get more people to test it out we can learn more.

I don't know if you want someone else to take a better look?

View changes since this review

tests/testsuite/lints/implicit_minimum_version_req.rs

Contributor

ehuss Feb 11, 2026

I'm assuming the changes in this file are mostly unrelated. Do we want to alter these tests so they don't trigger cargo::unused_dependencies?

src/cargo/lints/rules/unused_dependencies.rs

Comment on lines +151 to +154

+                      if lint_level.is_error() {
+                          *error_count += 1;
+                      }
+                      gctx.shell().print_report(&report, lint_level.force())?;

Contributor

ehuss Feb 11, 2026

Just a side note, I'm not sure if ya'll have considered having the emitter responsible for tracking the error_count so that it wouldn't be necessary to manually manage the count (similar to DiagCtxt in rustc).

Contributor Author

epage Feb 11, 2026

That is something worth exploring as we evolve the linting systec. I'd prefer it not to be on GlobalContext. As we'd need to track which package we are in, we can also use it to determine when the emitted reason should be shown.

Contributor Author

epage commented Feb 11, 2026

Looks good! I think this will be really useful. I don't have any particularly helpful review comments. I imagine as we get more people to test it out we can learn more.

Thanks for looking! I appreciate the extra eyes on the compilation process and do expect hands on experience will help with what we want the experience to be. Tempted to start this in the nursery group just so we don't have to wait for it to be fully polished (e.g. dealing with false positives).

Member

est31 commented Feb 15, 2026 •

edited

Loading

Thanks @epage for doing this. Nice work, especially love that it prints spans in Cargo.toml.

epage mentioned this pull request

Input on unused-deps lint and related RFC Boshen/cargo-shear#432

Open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-build-execution A-cargo-targets A-crate-dependencies A-documenting-cargo-itself A-workspaces S-waiting-on-author