Fallout from expansion of redundant import checking

[tmandry]

I've noticed there is a lot of fallout from the changes in #117772 expanding redundant import checking. See discussion and backlinks on that issue as well as #118692 for some evidence.

I have a theory that redundant imports should be split out into their own lint group (this was discussed in the past), and that in the future as a matter of policy, disruptive expansions like this should come with a machine-applicable fix to make rollout smoother.

For now, I'm opening this issue to collect data and feedback that I want to use to inform future policy discussions around lint expansions.

[tmandry]

FYI @surechen @petrochenkov - Not trying to single you out here, but I would like your input. This is a problem I've noticed in past lint expansions, and it seems useful to focus discussion around a concrete incidence.

[tmandry]

I'm interested to see what kind of numbers crater can give us.

@craterbot run name=redundant-imports start=master#6f726205a1b7992537ddec96c83f2b054b03e04f+rustflags=-Dunused-imports end=master#8b21296b5db6d5724d6b8440dcf459fa82fd88b5+rustflags=-Dunused-imports crates=top-1000 mode=check p=2

(p=2 because top-1000 should be fast, let me know if you disagree with jumping the queue like this)

[craterbot]

🚨 Error: failed to parse the command

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[tmandry]

I'm interested to see what kind of numbers crater can give us.

@craterbot run name=redundant-imports start=master#6f726205a1b7992537ddec96c83f2b054b03e04f+rustflags=-Dunused-imports end=master#8b21296b5db6d5724d6b8440dcf459fa82fd88b5+rustflags=-Dunused-imports crates=top-1000 mode=check-only p=2

[craterbot]

👌 Experiment redundant-imports created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[tmandry]

Now I see that the only p=1 in the queue has a similar number of crates to build, so let's use the same priority.

@craterbot name=redundant-imports p=1

[craterbot]

📝 Configuration of the redundant-imports experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[surechen]

FYI @surechen @petrochenkov - Not trying to single you out here, but I would like your input. This is a problem I've noticed in past lint expansions, and it seems useful to focus discussion around a concrete incidence.

Hi, thank you for starting the discussion.

As a newbie to compilers, this PR was originally intended to address a smaller scope issue #117448, detecting redundant imports which should only be imported by prelude, petrochenkov provided a lot of help in the comments. I very much recognized his comment: #117772 (comment) and asked for help, so he patiently guided me to develop this PR. So my views here may not be professional enough and can only represent my own.

I think Rust's powerful detection capabilities make me more confident in the code I develop. In my mind, ideally, the compiler can help me find all problems, so stricter checking of redundant imports will not cause too much trouble, but it will help me.

Of course, if some lints will invalidate a large amount of existing code, it will really increase my burden, so if the community has some future policies or some measures to make the changes smoother, then I will appreciate it very much, and I will also be very happy to participate in the discussion or do some development.

[Robbepop]

Hi, today I noticed that this is causing a lot of trouble for my crates in the Wasmi workspace, particularly the wasmi_core crate. I mistakenly thought this was a clippy issue first and reported it there and got rerouted to this issue fortunately. For the sake of providing my use case that causes issues with the expanded import checking I will copy what I wrote in the clippy bug report below.

Summary

For my crate wasmi_core in https://github.com/wasmi-labs/wasmi/tree/master/crates/core I see the following warnings when running clippy rustc on nightly channel:

warning: the item `Box` is imported redundantly
   --> crates/wasmi/src/store.rs:32:5
    |
32  | use alloc::boxed::Box;
    |     ^^^^^^^^^^^^^^^^^
    |
   ::: /Users/robin/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/prelude/mod.rs:125:13
    |
125 |     pub use super::v1::*;
    |             --------- the item `Box` is already defined here

Many more warnings pop up, but they all are very similar to the one above.
This import is required for proper no_std builds.

In lib.rs we do:

#[cfg(not(feature = "std"))]
extern crate alloc;

Thus alloc is always available as crate root import.

Lint Name

unused_imports

Reproducer

• Clone the repository https://github.com/wasmi-labs/wasmi (rev = a2f299af2f97af3691e195a4473abaa93bca72f7)
• Install rustc from current nightly channel.
• Run cargo +nightly check -p wasmi_core

Version

rustc 1.78.0-nightly (ef324565d 2024-02-27)
binary: rustc
commit-hash: ef324565d071c6d7e2477a195648549e33d6a465
commit-date: 2024-02-27
host: aarch64-apple-darwin
release: 1.78.0-nightly
LLVM version: 18.1.0