Skip to content

Commit

Permalink
rename lint and disallow clone_from
Browse files Browse the repository at this point in the history
  • Loading branch information
Centri3 committed Jun 15, 2023
1 parent 67d5e6e commit 10cc168
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 39 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/clone_on_copy_impl.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![allow(clippy::needless_clone_impl)]
#![allow(clippy::incorrect_clone_impl_on_copy_type)]

use std::fmt;
use std::marker::PhantomData;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/derive.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@run-rustfix
#![allow(unused)]
#![allow(clippy::clone_on_copy, unused)]
#![no_main]

// lint
Expand All @@ -8,6 +8,8 @@ struct A(u32);

impl Clone for A {
fn clone(&self) -> Self { *self }


}

impl Copy for A {}
Expand Down Expand Up @@ -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 {}
Expand All @@ -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)]
Expand All @@ -64,6 +87,11 @@ impl<A: Copy> Clone for Uwu<A> {
fn clone(&self) -> Self {
Self(self.0)
}

fn clone_from(&mut self, source: &Self) {
source.clone();
*self = source.clone();
}
}

impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//@run-rustfix
#![allow(unused)]
#![allow(clippy::clone_on_copy, unused)]
#![no_main]

// lint
Expand All @@ -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 {}
Expand Down Expand Up @@ -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 {}
Expand All @@ -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)]
Expand All @@ -66,6 +97,11 @@ impl<A: Copy> Clone for Uwu<A> {
fn clone(&self) -> Self {
Self(self.0)
}

fn clone_from(&mut self, source: &Self) {
source.clone();
*self = source.clone();
}
}

impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {}
40 changes: 40 additions & 0 deletions tests/ui/incorrect_clone_impl_on_copy_type.stderr
Original file line number Diff line number Diff line change
@@ -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

13 changes: 0 additions & 13 deletions tests/ui/needless_clone_impl.stderr

This file was deleted.

2 changes: 1 addition & 1 deletion tests/ui/unnecessary_struct_initialization.fixed
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/unnecessary_struct_initialization.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down

0 comments on commit 10cc168

Please sign in to comment.