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.32k stars 1.53k forks source link

Lint suggestion: duplicate or redundant trait bounds #12919

Open alice-i-cecile opened 3 months ago

alice-i-cecile commented 3 months ago

What it does

Checks for traits that are bounded by the same trait more than once.

If you want to get fancy with it, you could add a check for implied duplicate bounds, e.g. Hash implies PartialEq, so trait Foo: Hash + PartialEq is pointless.

This was detected in https://github.com/Leafwing-Studios/leafwing-input-manager/pull/545.

Playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=e298f6d998b87fe37663e1df073297f4

Advantage

These duplicate bounds are pointless and confusing.

Drawbacks

The more advanced implied redundant trait bound detection could reduce clarity of the code, and is less robust to future refactoring of the traits being relied on, as bounds may be removed in the future.

Example

use core::fmt::Debug;

trait SuperDebug: Debug + Debug + Debug { }

Could be written as:

use core::fmt::Debug;

trait SuperDebug: Debug { }
y21 commented 3 months ago

If you want to get fancy with it, you could add a check for implied duplicate bounds, e.g. Hash implies PartialEq, so trait Foo: Hash + PartialEq is pointless.

We might be able to use some of the logic in the implied_bounds_in_impls lint for this. I don't think Hash implies PartialEq, but for example that lint already understands & warns on fn f() -> impl Copy + Clone { 1 } because trait Copy: Clone {}, so this feels like it could share a lot of logic.

alice-i-cecile commented 3 months ago

Ah right, it's not actually implied, just used by the derive macro 😅