Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trait_duplication_in_bounds lint doesn't consider generics #8757

Closed
aldhsu opened this issue Apr 28, 2022 · 3 comments
Closed

trait_duplication_in_bounds lint doesn't consider generics #8757

aldhsu opened this issue Apr 28, 2022 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@aldhsu
Copy link
Contributor

aldhsu commented Apr 28, 2022

Summary

@giraffate caught this issue in #8703 PR which is based on the TRAIT_DUPLICATION_IN_BOUNDS lint.

Lint Name

TRAIT_DUPLICATION_IN_BOUNDS

Reproducer

I tried this code:

trait BoundWithGeneric<T> {}

fn good_bounds_with_generic<T: BoundWithGeneric<u32>>(arg0: T, arg1: T)
where
    T: BoundWithGeneric<u64>,
{
    unimplemented!();
}

I saw this happen:

 --> $DIR/trait_duplication_in_bounds.rs:16:32
+   |
+LL | fn good_bounds_with_generic<T: BoundWithGeneric<u32>>(arg0: T, arg1: T)
+   |                                ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider removing this trait bound

I expected to see this happen:

No linting error as while the same trait is used, the concrete type argument it is applied to is different.

Version

rustc 1.62.0-nightly (8f36334ca 2022-04-06)
binary: rustc
commit-hash: 8f36334ca939a67cce3f37f24953ff6f2d3f3d33
commit-date: 2022-04-06
host: x86_64-apple-darwin
release: 1.62.0-nightly
LLVM version: 14.0.0
@aldhsu aldhsu added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Apr 28, 2022
@aldhsu aldhsu changed the title TRAIT_DUPLICATION_IN_BOUNDS doesn't consider generics trait_duplication_in_bounds lint doesn't consider generics Apr 29, 2022
dtolnay added a commit to serde-rs/json that referenced this issue May 1, 2022
rust-lang/rust-clippy#8757

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:13:8
       |
    13 |     T: TryFrom<u64> + TryFrom<i64> + FromStr,
       |        ^^^^^^^^^^^^
       |
       = note: `-D clippy::trait-duplication-in-bounds` implied by `-D clippy::pedantic`
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:14:33
       |
    14 |     <T as TryFrom<u64>>::Error: Display,
       |                                 ^^^^^^^
       |
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:49:8
       |
    49 |     T: TryFrom<u64> + TryFrom<i64> + FromStr,
       |        ^^^^^^^^^^^^
       |
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:50:33
       |
    50 |     <T as TryFrom<u64>>::Error: Display,
       |                                 ^^^^^^^
       |
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds
@digama0
Copy link
Contributor

digama0 commented May 8, 2022

Here are some more test cases for this issue:

#![warn(clippy::trait_duplication_in_bounds)]

trait Trait<A> {}
struct Foo;

// to avoid trivial bound lint
impl Trait<u8> for Foo {}
impl Trait<i8> for Foo {}
impl Trait<u8> for () {}
impl Trait<i8> for () {}

fn ok1() where Foo: Trait<u8> + Trait<i8> {}
fn bad2<A>() where Foo: Trait<u8> + Trait<i8> {} // bad
fn ok3<A>() where (): Trait<u8> + Trait<i8> {}
fn bad3<A>() where Foo: Trait<A> + Trait<i8> {} // bad
fn bad4<A, B>() where Foo: Trait<A> + Trait<B> {} // bad
impl<A> Trait<(A,)> for Foo where Foo: Trait<A> + Trait<u8> {} // bad
impl<A, B> Trait<(A, B)> for Foo where Foo: Trait<A> + Trait<B> {} // bad

fn main() {}

@aldhsu
Copy link
Contributor Author

aldhsu commented May 17, 2022

I've been poking at this problem since it is related to my PR.

Reproducer

I tried this code:

fn duplicate_trait<T: Clone + Clone + Default, Z: Copy>(arg0: T, arg1: Z) {}

I saw this happen:

error: this trait bound is already specified in the where clause
  --> $DIR/trait_duplication_in_bounds.rs:110:16
   |
LL | fn duplicate_trait<T: Clone + Clone + Default, Z: Copy>(arg0: T, arg1: Z) {}
   |                       ^^^^^
   |
   = help: consider removing this trait bound

The error message always says this is trait bound is already specified in the where clause.

Two interpretations.

  1. The message is correct and the behaviour of the lint should only compare trait bounds to where clauses for the same generic, not within trait bounds or where clauses
  2. The message is incorrect and behaviour is expected.

@dswij
Copy link
Member

dswij commented May 17, 2022

The error message always says this is trait bound is already specified in the where clause.

Two interpretations.

1. The message is correct and the behaviour of the lint should only compare trait bounds to where clauses for the same generic, not within trait bounds or where clauses

2. The message is incorrect and behaviour is expected.

Based on the example in the lint's docs, I think the behavior is unexpected. Point 1 makes more sense, imo

aldhsu pushed a commit to aldhsu/rust-clippy that referenced this issue May 20, 2022
aldhsu pushed a commit to aldhsu/rust-clippy that referenced this issue May 20, 2022
aldhsu added a commit to aldhsu/rust-clippy that referenced this issue Jul 12, 2022
- only compare where predicates to trait bounds when generating where
  clause specific message to fix rust-lang#9151
- use comparable_trait_ref to account for trait bound generics to fix rust-lang#8757
aldhsu added a commit to aldhsu/rust-clippy that referenced this issue Jul 13, 2022
- only compare where predicates to trait bounds when generating where
  clause specific message to fix rust-lang#9151
- use comparable_trait_ref to account for trait bound generics to fix rust-lang#8757
aldhsu added a commit to aldhsu/rust-clippy that referenced this issue Jul 13, 2022
aldhsu added a commit to aldhsu/rust-clippy that referenced this issue Jul 13, 2022
- only compare where predicates to trait bounds when generating where
  clause specific message to fix rust-lang#9151
- use comparable_trait_ref to account for trait bound generics to fix rust-lang#8757
aldhsu added a commit to aldhsu/rust-clippy that referenced this issue Jul 13, 2022
aldhsu added a commit to aldhsu/rust-clippy that referenced this issue Aug 2, 2022
bors added a commit that referenced this issue Aug 14, 2022
Fixes [`trait_duplication_in_bounds`] false positives

Fixes #9076 #9151 #8757.
Partially fixes #8771.

changelog: [`trait_duplication_in_bounds`]: Reduce number of false positives.
@bors bors closed this as completed in 171d082 Aug 14, 2022
ronaldslc pushed a commit to ronaldslc/serde-json that referenced this issue Dec 3, 2022
rust-lang/rust-clippy#8757

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:13:8
       |
    13 |     T: TryFrom<u64> + TryFrom<i64> + FromStr,
       |        ^^^^^^^^^^^^
       |
       = note: `-D clippy::trait-duplication-in-bounds` implied by `-D clippy::pedantic`
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:14:33
       |
    14 |     <T as TryFrom<u64>>::Error: Display,
       |                                 ^^^^^^^
       |
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:49:8
       |
    49 |     T: TryFrom<u64> + TryFrom<i64> + FromStr,
       |        ^^^^^^^^^^^^
       |
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds

    error: this trait bound is already specified in the where clause
      --> tests/regression/issue845.rs:50:33
       |
    50 |     <T as TryFrom<u64>>::Error: Display,
       |                                 ^^^^^^^
       |
       = help: consider removing this trait bound
       = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

3 participants