add lint [needless_clone_impl]

CHANGELOG.md

@@ -5004,6 +5004,7 @@ Released 2018-09-13
 [`needless_bool_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool_assign
 [`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
 [`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
+[`needless_clone_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_clone_impl
 [`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
 [`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
 [`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main

clippy_lints/src/declared_lints.rs

@@ -461,6 +461,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::needless_else::NEEDLESS_ELSE_INFO,
     crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
     crate::needless_if::NEEDLESS_IF_INFO,
+    crate::needless_impls::NEEDLESS_CLONE_IMPL_INFO,
     crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
     crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
     crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,

clippy_lints/src/lib.rs

@@ -225,6 +225,7 @@ mod needless_continue;
 mod needless_else;
 mod needless_for_each;
 mod needless_if;
+mod needless_impls;
 mod needless_late_init;
 mod needless_parens_on_range_literals;
 mod needless_pass_by_value;
@@ -1042,6 +1043,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
             min_ident_chars_threshold,
         })
     });
+    store.register_late_pass(|_| Box::new(needless_impls::NeedlessImpls));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/needless_impls.rs

@@ -0,0 +1,113 @@
+use clippy_utils::{diagnostics::span_lint_and_sugg, get_parent_node, last_path_segment, ty::implements_trait};
+use rustc_errors::Applicability;
+use rustc_hir::{ExprKind, ImplItem, ImplItemKind, ItemKind, Node, UnOp};
+use rustc_hir_analysis::hir_ty_to_ty;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty::EarlyBinder;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{sym, symbol};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for manual implementations of `Clone` when `Copy` is already implemented.
+    ///
+    /// ### Why is this bad?
+    /// If both `Clone` and `Copy` are implemented, they must agree. This is done by dereferencing
+    /// `self` in `Clone`'s implementation. Anything else is incorrect.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// #[derive(Eq, PartialEq)]
+    /// struct A(u32);
+    ///
+    /// impl Clone for A {
+    ///     fn clone(&self) -> Self {
+    ///         Self(self.0)
+    ///     }
+    /// }
+    ///
+    /// impl Copy for A {}
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// #[derive(Eq, PartialEq)]
+    /// struct A(u32);
+    ///
+    /// impl Clone for A {
+    ///     fn clone(&self) -> Self {
+    ///         *self
+    ///     }
+    /// }
+    ///
+    /// impl Copy for A {}
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub NEEDLESS_CLONE_IMPL,
+    correctness,
+    "manual implementation of `Clone` on a `Copy` type"
+}
+declare_lint_pass!(NeedlessImpls => [NEEDLESS_CLONE_IMPL]);
+
+impl LateLintPass<'_> for NeedlessImpls {
+    #[expect(clippy::needless_return)]
+    fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
+        let node = get_parent_node(cx.tcx, impl_item.hir_id());
+        let Some(Node::Item(item)) = node else {
+            return;
+        };
+        let ItemKind::Impl(imp) = item.kind else {
+            return;
+        };
+        let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder) else {
+            return;
+        };
+        let trait_impl_def_id = trait_impl.def_id;
+        if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) {
+            return;
+        }
+        let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind else {
+            return;
+        };
+        let body = cx.tcx.hir().body(impl_item_id);
+        let ExprKind::Block(block, ..) = body.value.kind else {
+            return;
+        };
+        // Above is duplicated from the `duplicate_manual_partial_ord_impl` branch.
+        // Remove it while solving conflicts once that PR is merged.
+
+        // Actual implementation; remove this comment once aforementioned PR is merged
+        if cx.tcx.is_diagnostic_item(sym::Clone, trait_impl_def_id)
+            && impl_item.ident.name == sym::clone
+            && let Some(copy_def_id) = cx
+                .tcx
+                .diagnostic_items(trait_impl.def_id.krate)
+                .name_to_id
+                .get(&sym::Copy)
+            && implements_trait(
+                    cx,
+                    hir_ty_to_ty(cx.tcx, imp.self_ty),
+                    *copy_def_id,
+                    trait_impl.substs,
+                )
+        {
+            if block.stmts.is_empty()
+                && let Some(expr) = block.expr
+                && let ExprKind::Unary(UnOp::Deref, inner) = expr.kind
+                && let ExprKind::Path(qpath) = inner.kind
+                && last_path_segment(&qpath).ident.name == symbol::kw::SelfLower
+            {} else {
+                span_lint_and_sugg(
+                    cx,
+                    NEEDLESS_CLONE_IMPL,
+                    block.span,
+                    "manual implementation of `Clone` on a `Copy` type",
+                    "change this to",
+                    "{ *self }".to_owned(),
+                    Applicability::Unspecified,
+                );
+
+                return;
+            }
+        }
+    }
+}

tests/ui/clone_on_copy_impl.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::needless_clone_impl)]
+
 use std::fmt;
 use std::marker::PhantomData;
 

tests/ui/derive.rs

@@ -1,4 +1,4 @@
-#![allow(dead_code)]
+#![allow(clippy::needless_clone_impl, dead_code)]
 #![warn(clippy::expl_impl_clone_on_copy)]
 
 #[derive(Copy)]

tests/ui/needless_clone_impl.fixed

@@ -0,0 +1,69 @@
+//@run-rustfix
+#![allow(unused)]
+#![no_main]
+
+// lint
+
+struct A(u32);
+
+impl Clone for A {
+    fn clone(&self) -> Self { *self }
+}
+
+impl Copy for A {}
+
+// do not lint
+
+struct B(u32);
+
+impl Clone for B {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl Copy for B {}
+
+// do not lint derived (clone's implementation is `*self` here anyway)
+
+#[derive(Clone, Copy)]
+struct C(u32);
+
+// do not lint derived (fr this time)
+
+struct D(u32);
+
+#[automatically_derived]
+impl Clone for D {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl Copy for D {}
+
+// do not lint if clone is not manually implemented
+
+struct E(u32);
+
+#[automatically_derived]
+impl Clone for E {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl Copy for E {}
+
+// do not lint since copy has more restrictive bounds
+
+#[derive(Eq, PartialEq)]
+struct Uwu<A: Copy>(A);
+
+impl<A: Copy> Clone for Uwu<A> {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {}

tests/ui/needless_clone_impl.rs

@@ -0,0 +1,71 @@
+//@run-rustfix
+#![allow(unused)]
+#![no_main]
+
+// lint
+
+struct A(u32);
+
+impl Clone for A {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl Copy for A {}
+
+// do not lint
+
+struct B(u32);
+
+impl Clone for B {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl Copy for B {}
+
+// do not lint derived (clone's implementation is `*self` here anyway)
+
+#[derive(Clone, Copy)]
+struct C(u32);
+
+// do not lint derived (fr this time)
+
+struct D(u32);
+
+#[automatically_derived]
+impl Clone for D {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl Copy for D {}
+
+// do not lint if clone is not manually implemented
+
+struct E(u32);
+
+#[automatically_derived]
+impl Clone for E {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl Copy for E {}
+
+// do not lint since copy has more restrictive bounds
+
+#[derive(Eq, PartialEq)]
+struct Uwu<A: Copy>(A);
+
+impl<A: Copy> Clone for Uwu<A> {
+    fn clone(&self) -> Self {
+        Self(self.0)
+    }
+}
+
+impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {}

tests/ui/needless_clone_impl.stderr

@@ -0,0 +1,13 @@
+error: manual implementation of `Clone` on a `Copy` type
+  --> $DIR/needless_clone_impl.rs:10:29
+   |
+LL |       fn clone(&self) -> Self {
+   |  _____________________________^
+LL | |         Self(self.0)
+LL | |     }
+   | |_____^ help: change this to: `{ *self }`
+   |
+   = note: `#[deny(clippy::needless_clone_impl)]` on by default
+
+error: aborting due to previous error
+

tests/ui/unnecessary_struct_initialization.fixed

@@ -1,6 +1,6 @@
 //@run-rustfix
 
-#![allow(unused)]
+#![allow(clippy::needless_clone_impl, unused)]
 #![warn(clippy::unnecessary_struct_initialization)]
 
 struct S {

tests/ui/unnecessary_struct_initialization.rs

@@ -1,6 +1,6 @@
 //@run-rustfix
 
-#![allow(unused)]
+#![allow(clippy::needless_clone_impl, unused)]
 #![warn(clippy::unnecessary_struct_initialization)]
 
 struct S {