Add lint disallowed_modules

[lmeller-git]

Implement new lint disallowed_modules, which flags the usage of banned modules. Based on lint disallowed_types.

changelog: new lint: [`disallowed_modules`]

Closes #9489

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @dswij (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 8 candidates
• 8 candidates expanded to 8 candidates
• Random selection from Jarcho, dswij, llogiq, samueltardieu

[lmeller-git]

@rustbot ready

[samueltardieu]

Could you include more tests, with complex use, such as use std::{sync::{self, mpsc}, io::Error as IoError}?

Also, it looks like adding use std::sync::Arc; to your test file doesn't trigger the lint while it should.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[lmeller-git]

Reexports, such as std::sync::Arc should now be caught properly. While adding more tests, i realized a limitation of my current approach though:

When using reexported items that are themselves reexported in the banned module (or in other words not defined in a banned module), those items are not flagged. I documented this as a limitation for now, as I am not sure if/how this should be fixed.

mod inner {
    // allow import via this module
    #![allow(clippy::disallowed_modules)]
    pub use std::sync::{Mutex, Weak};
}

// Weak does not get flagged because it is defined in alloc::sync, which is not banned, not
// std::sync and its path is inner::Weak, which is also not banned, not std::sync::Weak.
// Note that importing Mutex still fails, since it is defined in std::sync, which is banned.
use inner::{Mutex as StdMutex, Weak as InnerWeak};

[lmeller-git]

@rustbot ready

[dswij]

r? clippy

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[lmeller-git]

After some consideration, I realized that checking only paths, rather than checking the definition modules of items, is a better approach here, because it more accurately captures the aim of the lint, allows for easier usage to lint things like internal reexport shims and can be implemented much more simply.

This means that my previous comment does not apply anymore, as we do not care about definition modules.

Currently the lint emits at most one diagnostic per checked path, with only the outermost disallowed module in a path being reported.
Macros are currently not handled specially, because I think that just linting all generated code is a good baseline.