rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.44k stars 1.54k forks source link

A lint for potential name clashes with the Rust Prelude #13301

Open LikeLakers2 opened 2 months ago

LikeLakers2 commented 2 months ago

What it does

This lint would check for potential name clashes in a crate with the Rust Prelude.

Name clashes with the Rust Standard Library are not too big of a problem - in those cases, you simply rename one import or the other. However, the Rust Prelude is special in that it is included everywhere - and its imports can be overridden.

This allows for name clashes to happen - and because Rust allows the rust prelude's imports to be overridden, it's not immediately obvious that it will cause problems.

As a real world example of this happening, see https://github.com/bevyengine/bevy/issues/14902, where the Bevy Game Engine accidentally introduced a name clash with std::ops::Drop - and nobody noticed until they tried to impl Drop for SomeStruct in the same file as importing the Bevy Prelude.

Advantage

Drawbacks

Example

I do not imagine this lint could be automatically fixed, so this will be an example of when this lint will show, and how a user might fix it.

I imagine it should lint on code such as this:

pub struct Drop;

The lint would suggest renaming the offending struct. Afterwards, it might look like this, which should not trigger this lint:

pub struct DropEvent;

Alternatively, if a user is okay with the name clash (i.e. they don't use std::ops::Drop), they can edit their code to show that they're okay with that:

#[expect(clippy::rust_prelude_name_clash)]
pub struct Drop;
BD103 commented 2 months ago
#[expect(clippy::rust_prelude_name_clash)]
pub struct Drop;

Is #[expect(...)] stable Rust? I didn't think it got out of nightly yet.

LikeLakers2 commented 2 months ago

@BD103 I don't remember. If it's not, just pretend expect is replaced with allow. The point of that code block was to say "Here's what they can do to show that they acknowledge the clashing name."

BD103 commented 2 months ago

Ok! I was hoping to use it in my own projects, which is why I was asking.