Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c711242

Browse files
Centri3Centri3
committedJun 12, 2023
add lint [needless_clone_impl]
1 parent 903fe3b commit c711242

11 files changed

+275
-3
lines changed
 

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5003,6 +5003,7 @@ Released 2018-09-13
50035003
[`needless_bool_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool_assign
50045004
[`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
50055005
[`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
5006+
[`needless_clone_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_clone_impl
50065007
[`needless_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
50075008
[`needless_continue`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_continue
50085009
[`needless_doctest_main`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_doctest_main

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
460460
crate::needless_else::NEEDLESS_ELSE_INFO,
461461
crate::needless_for_each::NEEDLESS_FOR_EACH_INFO,
462462
crate::needless_if::NEEDLESS_IF_INFO,
463+
crate::needless_impls::NEEDLESS_CLONE_IMPL_INFO,
463464
crate::needless_late_init::NEEDLESS_LATE_INIT_INFO,
464465
crate::needless_parens_on_range_literals::NEEDLESS_PARENS_ON_RANGE_LITERALS_INFO,
465466
crate::needless_pass_by_value::NEEDLESS_PASS_BY_VALUE_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ mod needless_continue;
224224
mod needless_else;
225225
mod needless_for_each;
226226
mod needless_if;
227+
mod needless_impls;
227228
mod needless_late_init;
228229
mod needless_parens_on_range_literals;
229230
mod needless_pass_by_value;
@@ -1033,6 +1034,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10331034
store.register_late_pass(|_| Box::new(redundant_type_annotations::RedundantTypeAnnotations));
10341035
store.register_late_pass(|_| Box::new(arc_with_non_send_sync::ArcWithNonSendSync));
10351036
store.register_late_pass(|_| Box::new(needless_if::NeedlessIf));
1037+
store.register_late_pass(|_| Box::new(needless_impls::NeedlessImpls));
10361038
// add lints here, do not remove this comment, it's used in `new_lint`
10371039
}
10381040

‎clippy_lints/src/needless_impls.rs

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use clippy_utils::{diagnostics::span_lint_and_sugg, get_parent_node, last_path_segment, ty::implements_trait};
2+
use rustc_errors::Applicability;
3+
use rustc_hir::{ExprKind, ImplItem, ImplItemKind, ItemKind, Node, UnOp};
4+
use rustc_hir_analysis::hir_ty_to_ty;
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty::EarlyBinder;
7+
use rustc_session::{declare_lint_pass, declare_tool_lint};
8+
use rustc_span::{sym, symbol};
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Checks for manual implementations of `Clone` when `Copy` is already implemented.
13+
///
14+
/// ### Why is this bad?
15+
/// If both `Clone` and `Copy` are implemented, they must agree. This is done by dereferencing
16+
/// `self` in `Clone`'s implementation. Anything else is incorrect.
17+
///
18+
/// ### Example
19+
/// ```rust,ignore
20+
/// #[derive(Eq, PartialEq)]
21+
/// struct A(u32);
22+
///
23+
/// impl Clone for A {
24+
/// fn clone(&self) -> Self {
25+
/// Self(self.0)
26+
/// }
27+
/// }
28+
///
29+
/// impl Copy for A {}
30+
/// ```
31+
/// Use instead:
32+
/// ```rust,ignore
33+
/// #[derive(Eq, PartialEq)]
34+
/// struct A(u32);
35+
///
36+
/// impl Clone for A {
37+
/// fn clone(&self) -> Self {
38+
/// *self
39+
/// }
40+
/// }
41+
///
42+
/// impl Copy for A {}
43+
/// ```
44+
#[clippy::version = "1.72.0"]
45+
pub NEEDLESS_CLONE_IMPL,
46+
correctness,
47+
"manual implementation of `Clone` on a `Copy` type"
48+
}
49+
declare_lint_pass!(NeedlessImpls => [NEEDLESS_CLONE_IMPL]);
50+
51+
impl LateLintPass<'_> for NeedlessImpls {
52+
#[expect(clippy::needless_return)]
53+
fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) {
54+
let node = get_parent_node(cx.tcx, impl_item.hir_id());
55+
let Some(Node::Item(item)) = node else {
56+
return;
57+
};
58+
let ItemKind::Impl(imp) = item.kind else {
59+
return;
60+
};
61+
let Some(trait_impl) = cx.tcx.impl_trait_ref(item.owner_id).map(EarlyBinder::skip_binder) else {
62+
return;
63+
};
64+
let trait_impl_def_id = trait_impl.def_id;
65+
if cx.tcx.is_automatically_derived(item.owner_id.to_def_id()) {
66+
return;
67+
}
68+
let ImplItemKind::Fn(_, impl_item_id) = cx.tcx.hir().impl_item(impl_item.impl_item_id()).kind else {
69+
return;
70+
};
71+
let body = cx.tcx.hir().body(impl_item_id);
72+
let ExprKind::Block(block, ..) = body.value.kind else {
73+
return;
74+
};
75+
// Above is duplicated from the `duplicate_manual_partial_ord_impl` branch.
76+
// Remove it while solving conflicts once that PR is merged.
77+
78+
// Actual implementation; remove this comment once aforementioned PR is merged
79+
if cx.tcx.is_diagnostic_item(sym::Clone, trait_impl_def_id)
80+
&& impl_item.ident.name == sym::clone
81+
&& let Some(copy_def_id) = cx
82+
.tcx
83+
.diagnostic_items(trait_impl.def_id.krate)
84+
.name_to_id
85+
.get(&sym::Copy)
86+
&& implements_trait(
87+
cx,
88+
hir_ty_to_ty(cx.tcx, imp.self_ty),
89+
*copy_def_id,
90+
trait_impl.substs,
91+
)
92+
{
93+
if block.stmts.is_empty()
94+
&& let Some(expr) = block.expr
95+
&& let ExprKind::Unary(UnOp::Deref, inner) = expr.kind
96+
&& let ExprKind::Path(qpath) = inner.kind
97+
&& last_path_segment(&qpath).ident.name == symbol::kw::SelfLower
98+
{} else {
99+
span_lint_and_sugg(
100+
cx,
101+
NEEDLESS_CLONE_IMPL,
102+
block.span,
103+
"manual implementation of `Clone` on a `Copy` type",
104+
"change this to",
105+
"{ *self }".to_owned(),
106+
Applicability::Unspecified,
107+
);
108+
109+
return;
110+
}
111+
}
112+
}
113+
}

‎tests/ui/clone_on_copy_impl.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![allow(clippy::needless_clone_impl)]
2+
13
use std::fmt;
24
use std::marker::PhantomData;
35

‎tests/ui/derive.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#![allow(dead_code)]
1+
#![allow(clippy::needless_clone_impl, dead_code)]
22
#![warn(clippy::expl_impl_clone_on_copy)]
33

44
#[derive(Copy)]

‎tests/ui/needless_clone_impl.fixed

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
//@run-rustfix
2+
#![allow(unused)]
3+
#![no_main]
4+
5+
// lint
6+
7+
struct A(u32);
8+
9+
impl Clone for A {
10+
fn clone(&self) -> Self { *self }
11+
}
12+
13+
impl Copy for A {}
14+
15+
// do not lint
16+
17+
struct B(u32);
18+
19+
impl Clone for B {
20+
fn clone(&self) -> Self {
21+
*self
22+
}
23+
}
24+
25+
impl Copy for B {}
26+
27+
// do not lint derived (clone's implementation is `*self` here anyway)
28+
29+
#[derive(Clone, Copy)]
30+
struct C(u32);
31+
32+
// do not lint derived (fr this time)
33+
34+
struct D(u32);
35+
36+
#[automatically_derived]
37+
impl Clone for D {
38+
fn clone(&self) -> Self {
39+
Self(self.0)
40+
}
41+
}
42+
43+
impl Copy for D {}
44+
45+
// do not lint if clone is not manually implemented
46+
47+
struct E(u32);
48+
49+
#[automatically_derived]
50+
impl Clone for E {
51+
fn clone(&self) -> Self {
52+
Self(self.0)
53+
}
54+
}
55+
56+
impl Copy for E {}
57+
58+
// do not lint since copy has more restrictive bounds
59+
60+
#[derive(Eq, PartialEq)]
61+
struct Uwu<A: Copy>(A);
62+
63+
impl<A: Copy> Clone for Uwu<A> {
64+
fn clone(&self) -> Self {
65+
Self(self.0)
66+
}
67+
}
68+
69+
impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {}

‎tests/ui/needless_clone_impl.rs

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
//@run-rustfix
2+
#![allow(unused)]
3+
#![no_main]
4+
5+
// lint
6+
7+
struct A(u32);
8+
9+
impl Clone for A {
10+
fn clone(&self) -> Self {
11+
Self(self.0)
12+
}
13+
}
14+
15+
impl Copy for A {}
16+
17+
// do not lint
18+
19+
struct B(u32);
20+
21+
impl Clone for B {
22+
fn clone(&self) -> Self {
23+
*self
24+
}
25+
}
26+
27+
impl Copy for B {}
28+
29+
// do not lint derived (clone's implementation is `*self` here anyway)
30+
31+
#[derive(Clone, Copy)]
32+
struct C(u32);
33+
34+
// do not lint derived (fr this time)
35+
36+
struct D(u32);
37+
38+
#[automatically_derived]
39+
impl Clone for D {
40+
fn clone(&self) -> Self {
41+
Self(self.0)
42+
}
43+
}
44+
45+
impl Copy for D {}
46+
47+
// do not lint if clone is not manually implemented
48+
49+
struct E(u32);
50+
51+
#[automatically_derived]
52+
impl Clone for E {
53+
fn clone(&self) -> Self {
54+
Self(self.0)
55+
}
56+
}
57+
58+
impl Copy for E {}
59+
60+
// do not lint since copy has more restrictive bounds
61+
62+
#[derive(Eq, PartialEq)]
63+
struct Uwu<A: Copy>(A);
64+
65+
impl<A: Copy> Clone for Uwu<A> {
66+
fn clone(&self) -> Self {
67+
Self(self.0)
68+
}
69+
}
70+
71+
impl<A: std::fmt::Debug + Copy + Clone> Copy for Uwu<A> {}

‎tests/ui/needless_clone_impl.stderr

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

‎tests/ui/unnecessary_struct_initialization.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@run-rustfix
22

3-
#![allow(unused)]
3+
#![allow(clippy::needless_clone_impl, unused)]
44
#![warn(clippy::unnecessary_struct_initialization)]
55

66
struct S {

‎tests/ui/unnecessary_struct_initialization.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@run-rustfix
22

3-
#![allow(unused)]
3+
#![allow(clippy::needless_clone_impl, unused)]
44
#![warn(clippy::unnecessary_struct_initialization)]
55

66
struct S {

0 commit comments

Comments
 (0)
Please sign in to comment.