Skip to content

Fallout from expansion of redundant import checking #121708

Open

Description

@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.

Activity

  1. added
    needs-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
    on Feb 27, 2024
  2. tmandry commented on Feb 27, 2024

    @tmandry
    MemberAuthor

    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.

  3. added
    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.
    A-resolveArea: Name/path resolution done by `rustc_resolve` specifically
    on Feb 27, 2024
  4. tmandry commented on Feb 27, 2024

    @tmandry
    Author
  5. craterbot commented on Feb 27, 2024

    @craterbot
  6. tmandry commented on Feb 27, 2024

    @tmandry
    MemberAuthor

    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

  7. craterbot commented on Feb 27, 2024

    @craterbot
    Collaborator

    👌 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

  8. tmandry commented on Feb 27, 2024

    @tmandry
    MemberAuthor

    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

  9. craterbot commented on Feb 27, 2024

    @craterbot
    Collaborator

    📝 Configuration of the redundant-imports experiment changed.

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

  10. surechen commented on Feb 28, 2024

    @surechen
    Contributor

    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.

  11. Robbepop commented on Feb 28, 2024

    @Robbepop
    Contributor

    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
    
  12. craterbot commented on Feb 28, 2024

    @craterbot
    Collaborator

    🚧 Experiment redundant-imports is now running

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

  13. 72 remaining items

  14. added a commit that references this issue on Jan 17, 2025
  15. added
    I-lang-radarItems that are on lang's radar and will need eventual work or consideration.
    and removed
    I-lang-nominatedNominated for discussion during a lang team meeting.
    on Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.A-resolveArea: Name/path resolution done by `rustc_resolve` specificallyI-lang-radarItems that are on lang's radar and will need eventual work or consideration.L-redundant_importsLint: redundant_importsS-waiting-on-reviewStatus: Awaiting review from the assignee but also interested parties.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-langRelevant to the language team

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions