False positive from type_repetition_in_bounds

[toku-sa-n]

Lint name: type_repetition_in_bounds

I tried this code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1107f6f365f12139ee07287fa5d80f88.

#![deny(clippy::type_repetition_in_bounds)]
use core::fmt;

pub struct Foo<T: ?Sized> {
    _bar: T,
}
impl<T> fmt::Display for Foo<[T]>
where
    T: Clone,
    [T]: fmt::Display,
{
    fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
        todo!()
    }
}

I expected to see this happen: No errors.

Instead, this happened:

    Checking playground v0.0.1 (/playground)
error: this type has already been used as a bound predicate
  --> src/lib.rs:10:5
   |
10 |     [T]: fmt::Display,
   |     ^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> src/lib.rs:1:9
   |
1  | #![deny(clippy::type_repetition_in_bounds)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: consider combining the bounds: `[T]: Clone + fmt::Display`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds

error: aborting due to previous error

error: could not compile `playground`

To learn more, run the command again with --verbose.

Meta

• cargo clippy -V: clippy 0.1.54 (607d6b0 2021-06-15)
• rustc -Vv:

rustc 1.55.0-nightly (607d6b00d 2021-06-15)
binary: rustc
commit-hash: 607d6b00d4e0e0475b8de9d0c870b7126fdcdf6b
commit-date: 2021-06-15
host: x86_64-unknown-linux-gnu
release: 1.55.0-nightly
LLVM version: 12.0.1

If I apply the suggestion by Clippy, I get another wrong error. code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=cd7648cbd5b5c16c3ba19593abef321f

#![deny(clippy::type_repetition_in_bounds)]
use core::fmt;

pub struct Foo<T: ?Sized> {
    _bar: T,
}
impl<T> fmt::Display for Foo<[T]>
where
    T: Clone,
    [T]: Clone + fmt::Display,
{
    fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
        todo!()
    }
}

errors:

    Checking playground v0.0.1 (/playground)
error: this type has already been used as a bound predicate
  --> src/lib.rs:10:5
   |
10 |     [T]: Clone + fmt::Display,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> src/lib.rs:1:9
   |
1  | #![deny(clippy::type_repetition_in_bounds)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: consider combining the bounds: `[T]: Clone + Clone + fmt::Display`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds

error: aborting due to previous error

error: could not compile `playground`

To learn more, run the command again with --verbose.

[kugiyasan]

Seems like you need to remove the T: Clone line, like explained here

[T] and [T; 1] to [T; 32] implements Clone if T implements Clone, so you need to specify the Clone trait for one of the types (source)

code: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=8ae99166e0231ea610302f0ef38052be

#![deny(clippy::type_repetition_in_bounds)]
use core::fmt;

pub struct Foo<T: ?Sized> {
    _bar: T,
}
impl<T> fmt::Display for Foo<[T]>
where
    [T]: Clone + fmt::Display,
{
    fn fmt(&self, _: &mut fmt::Formatter<'_>) -> fmt::Result {
        unreachable!()
    }
}

[toku-sa-n]

Sorry, I overlooked that. I thought that this lint only detected bounds of the same types and didn't expect that it warns such as T and [T]. It'd be helpful to add a note about this to the documentation or add a message like help: remove this type bound to the error messages.

I think suggesting Clone + Clone + fmt::Display is confusing, though.

[Arnavion]

Seems like you need to remove the T: Clone line, like explained here

This reasoning is wrong.

First, [T] cannot be Clone regardless of whether T is Clone or not, because Clone requires Sized.

Secondly, even if T: Clone somehow implied [T]: Clone, that would not mean that [T]: Clone + fmt::Display is a valid way to resolve this error. That does not allow the body of the impl block to use T as if it was Clone, which the original code with its T: Clone bound allowed. Compare https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=2aebafa7ee8b99335b1af1308bdf651f with https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=6bcb809cecbbd4b969ca8a115622bb81

So yes, this is a bug in the lint, not the code.

[opeik]

I've been having a similar issue with the following trait boundaries:

A: Service<Request<Body>, Response = Response<BoxBody>> + Clone + Send + 'static,
B: Service<Request<Body>, Response = Response<BoxBody>> + Clone + Send + 'static,
A::Future: Send,
B::Future: Send,
A::Error: Into<BoxError> + Send,
B::Error: Into<BoxError> + Send,

warning: this type has already been used as a bound predicate
   |
17 |     B::Future: Send,
   |     ^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
   |
3  | #![warn(missing_docs, clippy::cargo, clippy::nursery, clippy::pedantic)]
   |                                                       ^^^^^^^^^^^^^^^^
   = note: `#[warn(clippy::type_repetition_in_bounds)]` implied by `#[warn(clippy::pedantic)]`
   = help: consider combining the bounds: `B::Future: Send + Send`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds

warning: this type has already been used as a bound predicate
   |
19 |     B::Error: Into<BoxError> + Send,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: consider combining the bounds: `B::Error: Into<BoxError> + Send + Into<BoxError> + Send`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds

Both {A, B}::{Error, Future} requires Send. The suggestion of B::Future: Send + Send is very confusing too.

After speaking with Yandros on the Rust Discord, they mentioned:

If I had to guess, I suspect the issue is with https://github1s.com/rust-lang/rust-clippy/blob/master/clippy_utils/src/hir_utils.rs#L757-L758
Since it disregards the Ty first field of TypeRelative, it's as if it were hashing _::Future for both A::Future and B::Future
Which is not a problem per se (hashes don't have to be unique), except for the lint using hash unicity to detect repetitions
Feel free to report these observations to that issue, and suggest that SpanLessEq be used to further disambiguate hash collisions

[danielhenrymantilla]

Permalinks:

• SpanLessHash::hash_qpath

• Used here (by the way, the link to the source of the lint in the documentation is quite neat!)