`Clone` should be `*self` for types which are always `Copy`

[scottmcm]

What it does

If a type's Clone impl has the same where bounds as its Copy impl, then this gives a warning unless the Clone impl body is exactly

*self

Inspired by the incorrect code in https://users.rust-lang.org/t/why-is-the-value-expression-with-the-type-implemented-copy-not-desugared-to-e-clone/92921?u=scottmcm, which currently gets no clippy warnings https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=3fa61e0abb79c6a444d2ef77a5c6033c.

Lint Name

overcomplicated_clone_impl

Category

suspicious, complexity

Advantage

It's shorter to write than other options, and it always correct -- anything with different behaviour would be a violation of the Copy+Clone contract.

Thus using any other implementation is just asking for that more-complicated implementation to have a logic error.

Drawbacks

Macros might not want to special-case their generation to meet this.

Example

use std::marker::PhantomData;
struct Id<T>(u32, PhantomData<T>);

impl<T> Clone for Id<T> {
    fn clone(&self) -> Self {
        Id(self.0.clone(), PhantomData)
    }
}
impl<T> Copy for Id<T> {}

Could be written as:

use std::marker::PhantomData;
struct Id<T>(u32, PhantomData<T>);

impl<T> Clone for Id<T> {
    fn clone(&self) -> Self {
        *self
    }
}
impl<T> Copy for Id<T> {}

(Which, notably, can't just use #[derive(Copy, Clone)].)

[joseph-gio]

Here's an example of a place in the wild where this lint is would have been applicable: bevyengine/bevy#8247. This one is made especially nicer by dereferencing, since it allows us to skip referring to a #[cfg] field.

[Centri3]

This is very similar to #10788, so likely we can group these two lints together in correctness (or complexity), I quite like this idea!

@rustbot claim