From 67d5e6ec39a38cd4f2976b7a201d142ad6a0d09e Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Sun, 11 Jun 2023 02:46:28 -0500 Subject: [PATCH 1/2] add lint [`needless_clone_impl`] Update needless_impls.rs --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/needless_impls.rs | 109 ++++++++++++++++++ tests/ui/clone_on_copy_impl.rs | 2 + tests/ui/derive.rs | 2 +- tests/ui/needless_clone_impl.fixed | 69 +++++++++++ tests/ui/needless_clone_impl.rs | 71 ++++++++++++ tests/ui/needless_clone_impl.stderr | 13 +++ .../unnecessary_struct_initialization.fixed | 2 +- tests/ui/unnecessary_struct_initialization.rs | 2 +- 11 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 clippy_lints/src/needless_impls.rs create mode 100644 tests/ui/needless_clone_impl.fixed create mode 100644 tests/ui/needless_clone_impl.rs create mode 100644 tests/ui/needless_clone_impl.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 218bca40cb0..c0925988ca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5005,6 +5005,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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9509e6d016a..1fd70fce14f 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -463,6 +463,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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cf6499b9616..e07959849a3 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -226,6 +226,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; @@ -1047,6 +1048,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let stack_size_threshold = conf.stack_size_threshold; store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold))); store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit)); + store.register_late_pass(|_| Box::new(needless_impls::NeedlessImpls)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/needless_impls.rs b/clippy_lints/src/needless_impls.rs new file mode 100644 index 00000000000..ccfd55886ec --- /dev/null +++ b/clippy_lints/src/needless_impls.rs @@ -0,0 +1,109 @@ +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.get_diagnostic_item(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; + } + } + } +} diff --git a/tests/ui/clone_on_copy_impl.rs b/tests/ui/clone_on_copy_impl.rs index 8f9f2a0db8c..06d8ad48446 100644 --- a/tests/ui/clone_on_copy_impl.rs +++ b/tests/ui/clone_on_copy_impl.rs @@ -1,3 +1,5 @@ +#![allow(clippy::needless_clone_impl)] + use std::fmt; use std::marker::PhantomData; diff --git a/tests/ui/derive.rs b/tests/ui/derive.rs index 843e1df8bc6..857cd379fe2 100644 --- a/tests/ui/derive.rs +++ b/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)] diff --git a/tests/ui/needless_clone_impl.fixed b/tests/ui/needless_clone_impl.fixed new file mode 100644 index 00000000000..05caf485db6 --- /dev/null +++ b/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); + +impl Clone for Uwu { + fn clone(&self) -> Self { + Self(self.0) + } +} + +impl Copy for Uwu {} diff --git a/tests/ui/needless_clone_impl.rs b/tests/ui/needless_clone_impl.rs new file mode 100644 index 00000000000..e30469e09e8 --- /dev/null +++ b/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); + +impl Clone for Uwu { + fn clone(&self) -> Self { + Self(self.0) + } +} + +impl Copy for Uwu {} diff --git a/tests/ui/needless_clone_impl.stderr b/tests/ui/needless_clone_impl.stderr new file mode 100644 index 00000000000..51e2ef4f741 --- /dev/null +++ b/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 + diff --git a/tests/ui/unnecessary_struct_initialization.fixed b/tests/ui/unnecessary_struct_initialization.fixed index bdf746cf2c4..aec9725c4b9 100644 --- a/tests/ui/unnecessary_struct_initialization.fixed +++ b/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 { diff --git a/tests/ui/unnecessary_struct_initialization.rs b/tests/ui/unnecessary_struct_initialization.rs index 7271e2f957a..a3b9b88dbc2 100644 --- a/tests/ui/unnecessary_struct_initialization.rs +++ b/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 { From 10cc1684ceca90f84a42099f09cf0737a7aca35b Mon Sep 17 00:00:00 2001 From: Centri3 <114838443+Centri3@users.noreply.github.com> Date: Tue, 13 Jun 2023 05:25:49 -0500 Subject: [PATCH 2/2] rename lint and disallow `clone_from` --- CHANGELOG.md | 2 +- clippy_lints/src/declared_lints.rs | 2 +- .../{needless_impls.rs => incorrect_impls.rs} | 47 ++++++++++++------- clippy_lints/src/lib.rs | 4 +- tests/ui/clone_on_copy_impl.rs | 2 +- tests/ui/derive.rs | 2 +- ...> incorrect_clone_impl_on_copy_type.fixed} | 30 +++++++++++- ...s => incorrect_clone_impl_on_copy_type.rs} | 38 ++++++++++++++- .../incorrect_clone_impl_on_copy_type.stderr | 40 ++++++++++++++++ tests/ui/needless_clone_impl.stderr | 13 ----- .../unnecessary_struct_initialization.fixed | 2 +- tests/ui/unnecessary_struct_initialization.rs | 2 +- 12 files changed, 145 insertions(+), 39 deletions(-) rename clippy_lints/src/{needless_impls.rs => incorrect_impls.rs} (69%) rename tests/ui/{needless_clone_impl.fixed => incorrect_clone_impl_on_copy_type.fixed} (66%) rename tests/ui/{needless_clone_impl.rs => incorrect_clone_impl_on_copy_type.rs} (58%) create mode 100644 tests/ui/incorrect_clone_impl_on_copy_type.stderr delete mode 100644 tests/ui/needless_clone_impl.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index c0925988ca1..445e2366cc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4833,6 +4833,7 @@ Released 2018-09-13 [`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping [`inconsistent_struct_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor +[`incorrect_clone_impl_on_copy_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_clone_impl_on_copy_type [`index_refutable_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#index_refutable_slice [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask @@ -5005,7 +5006,6 @@ 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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1fd70fce14f..2690336648d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -206,6 +206,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::implicit_saturating_add::IMPLICIT_SATURATING_ADD_INFO, crate::implicit_saturating_sub::IMPLICIT_SATURATING_SUB_INFO, crate::inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR_INFO, + crate::incorrect_impls::INCORRECT_CLONE_IMPL_ON_COPY_TYPE_INFO, crate::index_refutable_slice::INDEX_REFUTABLE_SLICE_INFO, crate::indexing_slicing::INDEXING_SLICING_INFO, crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO, @@ -463,7 +464,6 @@ 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, diff --git a/clippy_lints/src/needless_impls.rs b/clippy_lints/src/incorrect_impls.rs similarity index 69% rename from clippy_lints/src/needless_impls.rs rename to clippy_lints/src/incorrect_impls.rs index ccfd55886ec..13cc0b23ba3 100644 --- a/clippy_lints/src/needless_impls.rs +++ b/clippy_lints/src/incorrect_impls.rs @@ -42,13 +42,13 @@ declare_clippy_lint! { /// impl Copy for A {} /// ``` #[clippy::version = "1.72.0"] - pub NEEDLESS_CLONE_IMPL, + pub INCORRECT_CLONE_IMPL_ON_COPY_TYPE, correctness, "manual implementation of `Clone` on a `Copy` type" } -declare_lint_pass!(NeedlessImpls => [NEEDLESS_CLONE_IMPL]); +declare_lint_pass!(IncorrectImpls => [INCORRECT_CLONE_IMPL_ON_COPY_TYPE]); -impl LateLintPass<'_> for NeedlessImpls { +impl LateLintPass<'_> for IncorrectImpls { #[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()); @@ -77,7 +77,6 @@ impl LateLintPass<'_> for NeedlessImpls { // 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.get_diagnostic_item(sym::Copy) && implements_trait( cx, @@ -86,20 +85,36 @@ impl LateLintPass<'_> for NeedlessImpls { 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 { + if impl_item.ident.name == sym::clone { + 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, + INCORRECT_CLONE_IMPL_ON_COPY_TYPE, + block.span, + "incorrect implementation of `clone` on a `Copy` type", + "change this to", + "{ *self }".to_owned(), + Applicability::MaybeIncorrect, + ); + + return; + } + } + + if impl_item.ident.name == sym::clone_from { 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, + INCORRECT_CLONE_IMPL_ON_COPY_TYPE, + impl_item.span, + "incorrect implementation of `clone_from` on a `Copy` type", + "remove this", + String::new(), + Applicability::MaybeIncorrect, ); return; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e07959849a3..b254c05d48b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -150,6 +150,7 @@ mod implicit_return; mod implicit_saturating_add; mod implicit_saturating_sub; mod inconsistent_struct_constructor; +mod incorrect_impls; mod index_refutable_slice; mod indexing_slicing; mod infinite_iter; @@ -226,7 +227,6 @@ 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; @@ -1048,7 +1048,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: let stack_size_threshold = conf.stack_size_threshold; store.register_late_pass(move |_| Box::new(large_stack_frames::LargeStackFrames::new(stack_size_threshold))); store.register_late_pass(|_| Box::new(single_range_in_vec_init::SingleRangeInVecInit)); - store.register_late_pass(|_| Box::new(needless_impls::NeedlessImpls)); + store.register_late_pass(|_| Box::new(incorrect_impls::IncorrectImpls)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/clone_on_copy_impl.rs b/tests/ui/clone_on_copy_impl.rs index 06d8ad48446..b7c186bef77 100644 --- a/tests/ui/clone_on_copy_impl.rs +++ b/tests/ui/clone_on_copy_impl.rs @@ -1,4 +1,4 @@ -#![allow(clippy::needless_clone_impl)] +#![allow(clippy::incorrect_clone_impl_on_copy_type)] use std::fmt; use std::marker::PhantomData; diff --git a/tests/ui/derive.rs b/tests/ui/derive.rs index 857cd379fe2..44e18bff83f 100644 --- a/tests/ui/derive.rs +++ b/tests/ui/derive.rs @@ -1,4 +1,4 @@ -#![allow(clippy::needless_clone_impl, dead_code)] +#![allow(clippy::incorrect_clone_impl_on_copy_type, dead_code)] #![warn(clippy::expl_impl_clone_on_copy)] #[derive(Copy)] diff --git a/tests/ui/needless_clone_impl.fixed b/tests/ui/incorrect_clone_impl_on_copy_type.fixed similarity index 66% rename from tests/ui/needless_clone_impl.fixed rename to tests/ui/incorrect_clone_impl_on_copy_type.fixed index 05caf485db6..ac482dcda1e 100644 --- a/tests/ui/needless_clone_impl.fixed +++ b/tests/ui/incorrect_clone_impl_on_copy_type.fixed @@ -1,5 +1,5 @@ //@run-rustfix -#![allow(unused)] +#![allow(clippy::clone_on_copy, unused)] #![no_main] // lint @@ -8,6 +8,8 @@ struct A(u32); impl Clone for A { fn clone(&self) -> Self { *self } + + } impl Copy for A {} @@ -38,6 +40,11 @@ impl Clone for D { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for D {} @@ -51,10 +58,26 @@ impl Clone for E { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for E {} +// lint since clone is not derived + +#[derive(Copy)] +struct F(u32); + +impl Clone for F { + fn clone(&self) -> Self { *self } + + +} + // do not lint since copy has more restrictive bounds #[derive(Eq, PartialEq)] @@ -64,6 +87,11 @@ impl Clone for Uwu { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for Uwu {} diff --git a/tests/ui/needless_clone_impl.rs b/tests/ui/incorrect_clone_impl_on_copy_type.rs similarity index 58% rename from tests/ui/needless_clone_impl.rs rename to tests/ui/incorrect_clone_impl_on_copy_type.rs index e30469e09e8..00775874ff5 100644 --- a/tests/ui/needless_clone_impl.rs +++ b/tests/ui/incorrect_clone_impl_on_copy_type.rs @@ -1,5 +1,5 @@ //@run-rustfix -#![allow(unused)] +#![allow(clippy::clone_on_copy, unused)] #![no_main] // lint @@ -10,6 +10,11 @@ impl Clone for A { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for A {} @@ -40,6 +45,11 @@ impl Clone for D { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for D {} @@ -53,10 +63,31 @@ impl Clone for E { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for E {} +// lint since clone is not derived + +#[derive(Copy)] +struct F(u32); + +impl Clone for F { + fn clone(&self) -> Self { + Self(self.0) + } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } +} + // do not lint since copy has more restrictive bounds #[derive(Eq, PartialEq)] @@ -66,6 +97,11 @@ impl Clone for Uwu { fn clone(&self) -> Self { Self(self.0) } + + fn clone_from(&mut self, source: &Self) { + source.clone(); + *self = source.clone(); + } } impl Copy for Uwu {} diff --git a/tests/ui/incorrect_clone_impl_on_copy_type.stderr b/tests/ui/incorrect_clone_impl_on_copy_type.stderr new file mode 100644 index 00000000000..0021841aa86 --- /dev/null +++ b/tests/ui/incorrect_clone_impl_on_copy_type.stderr @@ -0,0 +1,40 @@ +error: incorrect implementation of `clone` on a `Copy` type + --> $DIR/incorrect_clone_impl_on_copy_type.rs:10:29 + | +LL | fn clone(&self) -> Self { + | _____________________________^ +LL | | Self(self.0) +LL | | } + | |_____^ help: change this to: `{ *self }` + | + = note: `#[deny(clippy::incorrect_clone_impl_on_copy_type)]` on by default + +error: incorrect implementation of `clone_from` on a `Copy` type + --> $DIR/incorrect_clone_impl_on_copy_type.rs:14:5 + | +LL | / fn clone_from(&mut self, source: &Self) { +LL | | source.clone(); +LL | | *self = source.clone(); +LL | | } + | |_____^ help: remove this + +error: incorrect implementation of `clone` on a `Copy` type + --> $DIR/incorrect_clone_impl_on_copy_type.rs:81:29 + | +LL | fn clone(&self) -> Self { + | _____________________________^ +LL | | Self(self.0) +LL | | } + | |_____^ help: change this to: `{ *self }` + +error: incorrect implementation of `clone_from` on a `Copy` type + --> $DIR/incorrect_clone_impl_on_copy_type.rs:85:5 + | +LL | / fn clone_from(&mut self, source: &Self) { +LL | | source.clone(); +LL | | *self = source.clone(); +LL | | } + | |_____^ help: remove this + +error: aborting due to 4 previous errors + diff --git a/tests/ui/needless_clone_impl.stderr b/tests/ui/needless_clone_impl.stderr deleted file mode 100644 index 51e2ef4f741..00000000000 --- a/tests/ui/needless_clone_impl.stderr +++ /dev/null @@ -1,13 +0,0 @@ -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 - diff --git a/tests/ui/unnecessary_struct_initialization.fixed b/tests/ui/unnecessary_struct_initialization.fixed index aec9725c4b9..eae1271d1aa 100644 --- a/tests/ui/unnecessary_struct_initialization.fixed +++ b/tests/ui/unnecessary_struct_initialization.fixed @@ -1,6 +1,6 @@ //@run-rustfix -#![allow(clippy::needless_clone_impl, unused)] +#![allow(clippy::incorrect_clone_impl_on_copy_type, unused)] #![warn(clippy::unnecessary_struct_initialization)] struct S { diff --git a/tests/ui/unnecessary_struct_initialization.rs b/tests/ui/unnecessary_struct_initialization.rs index a3b9b88dbc2..4abd560f84b 100644 --- a/tests/ui/unnecessary_struct_initialization.rs +++ b/tests/ui/unnecessary_struct_initialization.rs @@ -1,6 +1,6 @@ //@run-rustfix -#![allow(clippy::needless_clone_impl, unused)] +#![allow(clippy::incorrect_clone_impl_on_copy_type, unused)] #![warn(clippy::unnecessary_struct_initialization)] struct S {