Investigate using the unused crate rustc warning

[est31]

rustc PR https://github.com/rust-lang/rust/pull/72342

Since the PR, rustc can be configured to emit unused crate warnings.

Suggested by ehuss here.

[joshtriplett]

@est31 I've tried that lint, and it seems to work well. I'd love to get a comparison of what cargo-udeps does above and beyond that lint.

[est31]

The lint is enough for cases like buck where for each compilation unit you must specify the entire list of dependencies. For the cargo use case, one fundamentally can't implement something rustc side that works well: there is multiple compilation units that share a possible list of dependencies. A dependency can only be used by one of the compilation units.

For the main [dependencies]:

• the library target in lib.rs
• main.rs and all the other binary targets in the bin directory
• all the compilation units in the [dev-dependencies] section below also have access to the [dependencies]. so it is possible that some dependency foobar from [dependencies] is not used by lib.rs but it is used by tests/with_foobar.rs. Ideally, the lint would recognize that and suggest the move to [dev-dependencies] instead of saying that foobar is unused, which is not an accurate statement. Admittedly, we already have this for the dead code lint on the rust side, when some code is used, but only by some #[cfg()] that isn't active in the current compilation.

For the [build-dependencies]:

• build.rs - this is the only place where there is a 1:1 correspondence. in theory, the lint can be enabled for build.rs

For the [dev-dependencies]:

• lib.rs with #[cfg(test)] -- this is a different compilation unit from the normal lib.rs for usage as library
• lib.rs ran by rustdoc that enables #[cfg(doc)]
• each doctest in lib.rs as its own compilation unit -- this goes with the extra layer of first going through rustdoc which then invokes rustc internally
• each file in examples/ as their own compilation unit
• each file in benches/ as their own compilation unit
• each file in tests/ as their own compilation unit
• probably more

cargo-udeps works in the way that it takes the output of all compilation units in a given compilation run and taking the intersection of all unused dependency warnings (or the de morgan equivalent for dependency usages).

cargo-udeps also performs a mapping from the crate name as it appears in the rust program to the name as it appears in Cargo.toml. Personally I think this should be the name that's exposed to the user, as the user needs to remove that name. Most of the time the two names match, but not always.

There is other questions like where the lint should point to: it should ideally point to the place in Cargo.toml that specifies the dependency.

I think if one wants there to be a good check together with cargo, there is two alternatives:

• move cargo to a 1:1 model like buck and turn on the lint. this will hurt [dev-dependencies] a lot, I'm sure there is libraries that are commonly used with some other library so a large fraction of their examples use them but not all of them.
• have cargo collect the info from all compilation units and then combine it -- this is what cargo-udeps is doing. still, one can't upstream it as there is not a single way to compile all of a package's compilation units in a single cargo invocation, so you always miss information from some compilation unit. cargo-udeps simply punts on the issue, it basically has false positives, which is something an inofficial tool can do, but is below the quality bar of cargo.

Middle forms are possible, like adding [bin-dependencies] and enabling the lint for the main lib.rs compilation unit, but keeping [dev-dependencies] without linting. In general, dependencies only used by binaries are highly useful anyways.

There is the further complication that target.'cfg(foobar)'.dependencies is not as expressive as #[cfg(foobar)] from what I remember (maybe this has now changed? idk). So in theory some cfg'd out code could use a dependency but one can't make the dependency also gated behind that cfg, at least not in all instances. I think it's okay to live with this limitation and just silence the lint for that crate, but it's an important thing to keep in mind.

[joshtriplett]

have cargo collect the info from all compilation units and then combine it -- this is what cargo-udeps is doing. still, one can't upstream it as there is not a single way to compile all of a package's compilation units in a single cargo invocation, so you always miss information from some compilation unit.

Does --all-targets not suffice for this?

[joshtriplett]

move cargo to a 1:1 model like buck

Frankly, I'd love to have this as an option someday. It'd be a massive tradeoff, but a huge win for a number of common cases. I'd love to have built-in udeps support before that, though.

[est31]

Does --all-targets not suffice for this?

It doesn't, neither cargo check --all-targets nor cargo test --all-targets. I tried out all possible combinations once, and this was the resulting table: https://github.com/rust-lang/cargo/pull/8437#issuecomment-653842297 . The cargo test --all-targets command gets closest, but that one still doesn't check doctests.

[joshtriplett]

@est31 Ah, I didn't realize that. We should fix that; cargo check --all-targets / cargo build --all-targets seems like it should build everything. If we can't do that for backwards-compatibility reasons, then we should add a new option that does.

Would you consider filing (and nominating) an issue on cargo for building everything, with a link to the table you provided above?

[est31]

I've just recreated the table on the latest rustc, and it is like before (I've added some extra lines):

cargo command	lib.rs	main.rs	lib.rs cfg test	tests/	benches/	doctests
c	yes	yes	no	no	no	no
c --all-targets	yes	yes	yes	yes	yes	no
t --no-run	yes	yes	yes	yes	no	no
t	yes	yes	yes	yes	no	yes
t --all-targets	yes	yes	yes	yes	yes	no
t --no-run --all-targets	yes	yes	yes	yes	yes	no
t --doc	yes	no	no	no	no	yes
clippy --all-targets	yes	yes	yes	yes	yes	no

So it seems that --all-targets works really well both for cargo check as well as for cargo test. The only problem is doctests, where there has been no change since I made the linked comment in 2020.

For missing doctest support, there is already two issues in cargo, one for cargo check and one for cargo test. I've linked them in the original table but they were behind the no so not easy to spot. Do you think it makes sense to file separate issues given these two already exist?

Maybe one could unify the two issues into one: --all-targets should always include all targets, regardless of whether it's cargo test, cargo clippy or cargo check.

If the cargo team could make --all-targets also check and run the docests, I think one could restart efforts from https://github.com/rust-lang/cargo/pull/8437 to upstream the unused crate warning.