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] 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 {