Skip to content

Commit d84505b

Browse files
committed
Post lint on the type HIR node
Out of the 5 lints in the `derive` directory, only two of them (`unsafe_derive_deserialize` and `derive_partial_eq_without_eq`) posted the lint on the ADT node. This fixes the situation for the three other lints: - `derive_hash_with_manual_eq` - `derive_ord_xor_partial_ord` - `expl_impl_clone_on_copy` This allows `#[expect]` to be used on the ADT to silence the lint. Also, this makes `expl_impl_clone_on_copy` properly use an "help" message instead of a "note" one when suggesting a fix.
1 parent 8a5dc7c commit d84505b

10 files changed

+101
-31
lines changed

clippy_lints/src/derive/derive_ord_xor_partial_ord.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use clippy_utils::diagnostics::span_lint_and_then;
2-
use rustc_hir as hir;
1+
use clippy_utils::diagnostics::span_lint_hir_and_then;
2+
use rustc_hir::{self as hir, HirId};
33
use rustc_lint::LateContext;
44
use rustc_middle::ty::Ty;
55
use rustc_span::{Span, sym};
@@ -12,6 +12,7 @@ pub(super) fn check<'tcx>(
1212
span: Span,
1313
trait_ref: &hir::TraitRef<'_>,
1414
ty: Ty<'tcx>,
15+
adt_hir_id: HirId,
1516
ord_is_automatically_derived: bool,
1617
) {
1718
if let Some(ord_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Ord)
@@ -38,7 +39,7 @@ pub(super) fn check<'tcx>(
3839
"you are deriving `Ord` but have implemented `PartialOrd` explicitly"
3940
};
4041

41-
span_lint_and_then(cx, DERIVE_ORD_XOR_PARTIAL_ORD, span, mess, |diag| {
42+
span_lint_hir_and_then(cx, DERIVE_ORD_XOR_PARTIAL_ORD, adt_hir_id, span, mess, |diag| {
4243
if let Some(local_def_id) = impl_id.as_local() {
4344
let hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id);
4445
diag.span_note(cx.tcx.hir_span(hir_id), "`PartialOrd` implemented here");

clippy_lints/src/derive/derive_partial_eq_without_eq.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,22 @@ use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::has_non_exhaustive_attr;
33
use clippy_utils::ty::implements_trait_with_env;
44
use rustc_errors::Applicability;
5-
use rustc_hir as hir;
65
use rustc_hir::def_id::DefId;
6+
use rustc_hir::{self as hir, HirId};
77
use rustc_lint::LateContext;
88
use rustc_middle::ty::{self, ClauseKind, GenericParamDefKind, ParamEnv, TraitPredicate, Ty, TyCtxt, Upcast};
99
use rustc_span::{Span, sym};
1010

1111
use super::DERIVE_PARTIAL_EQ_WITHOUT_EQ;
1212

1313
/// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint.
14-
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) {
14+
pub(super) fn check<'tcx>(
15+
cx: &LateContext<'tcx>,
16+
span: Span,
17+
trait_ref: &hir::TraitRef<'_>,
18+
ty: Ty<'tcx>,
19+
adt_hir_id: HirId,
20+
) {
1521
if let ty::Adt(adt, args) = ty.kind()
1622
&& cx.tcx.visibility(adt.did()).is_public()
1723
&& let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq)
@@ -20,7 +26,6 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::T
2026
&& !has_non_exhaustive_attr(cx.tcx, *adt)
2127
&& !ty_implements_eq_trait(cx.tcx, ty, eq_trait_def_id)
2228
&& let typing_env = typing_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id)
23-
&& let Some(local_def_id) = adt.did().as_local()
2429
// If all of our fields implement `Eq`, we can implement `Eq` too
2530
&& adt
2631
.all_fields()
@@ -30,7 +35,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::T
3035
span_lint_hir_and_then(
3136
cx,
3237
DERIVE_PARTIAL_EQ_WITHOUT_EQ,
33-
cx.tcx.local_def_id_to_hir_id(local_def_id),
38+
adt_hir_id,
3439
span.ctxt().outer_expn_data().call_site,
3540
"you are deriving `PartialEq` and can implement `Eq`",
3641
|diag| {

clippy_lints/src/derive/derived_hash_with_manual_eq.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use clippy_utils::diagnostics::span_lint_and_then;
2-
use rustc_hir as hir;
1+
use clippy_utils::diagnostics::span_lint_hir_and_then;
2+
use rustc_hir::{HirId, TraitRef};
33
use rustc_lint::LateContext;
44
use rustc_middle::ty::Ty;
55
use rustc_span::{Span, sym};
@@ -10,8 +10,9 @@ use super::DERIVED_HASH_WITH_MANUAL_EQ;
1010
pub(super) fn check<'tcx>(
1111
cx: &LateContext<'tcx>,
1212
span: Span,
13-
trait_ref: &hir::TraitRef<'_>,
13+
trait_ref: &TraitRef<'_>,
1414
ty: Ty<'tcx>,
15+
adt_hir_id: HirId,
1516
hash_is_automatically_derived: bool,
1617
) {
1718
if let Some(peq_trait_def_id) = cx.tcx.lang_items().eq_trait()
@@ -31,9 +32,10 @@ pub(super) fn check<'tcx>(
3132
// Only care about `impl PartialEq<Foo> for Foo`
3233
// For `impl PartialEq<B> for A, input_types is [A, B]
3334
if trait_ref.instantiate_identity().args.type_at(1) == ty {
34-
span_lint_and_then(
35+
span_lint_hir_and_then(
3536
cx,
3637
DERIVED_HASH_WITH_MANUAL_EQ,
38+
adt_hir_id,
3739
span,
3840
"you are deriving `Hash` but have implemented `PartialEq` explicitly",
3941
|diag| {

clippy_lints/src/derive/expl_impl_clone_on_copy.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
1-
use clippy_utils::diagnostics::span_lint_and_note;
1+
use clippy_utils::diagnostics::span_lint_hir_and_then;
22
use clippy_utils::ty::{implements_trait, is_copy};
3-
use rustc_hir::{self as hir, Item};
3+
use rustc_hir::{self as hir, HirId, Item};
44
use rustc_lint::LateContext;
55
use rustc_middle::ty::{self, GenericArgKind, Ty};
66

77
use super::EXPL_IMPL_CLONE_ON_COPY;
88

99
/// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
10-
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) {
10+
pub(super) fn check<'tcx>(
11+
cx: &LateContext<'tcx>,
12+
item: &Item<'_>,
13+
trait_ref: &hir::TraitRef<'_>,
14+
ty: Ty<'tcx>,
15+
adt_hir_id: HirId,
16+
) {
1117
let clone_id = match cx.tcx.lang_items().clone_trait() {
1218
Some(id) if trait_ref.trait_def_id() == Some(id) => id,
1319
_ => return,
@@ -54,12 +60,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &h
5460
return;
5561
}
5662

57-
span_lint_and_note(
63+
span_lint_hir_and_then(
5864
cx,
5965
EXPL_IMPL_CLONE_ON_COPY,
66+
adt_hir_id,
6067
item.span,
6168
"you are implementing `Clone` explicitly on a `Copy` type",
62-
Some(item.span),
63-
"consider deriving `Clone` or removing `Copy`",
69+
|diag| {
70+
diag.span_help(item.span, "consider deriving `Clone` or removing `Copy`");
71+
},
6472
);
6573
}

clippy_lints/src/derive/mod.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use clippy_utils::path_res;
2+
use rustc_hir::def::Res;
13
use rustc_hir::{Impl, Item, ItemKind};
24
use rustc_lint::{LateContext, LateLintPass};
35
use rustc_session::declare_lint_pass;
@@ -194,21 +196,25 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
194196
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
195197
if let ItemKind::Impl(Impl {
196198
of_trait: Some(of_trait),
199+
self_ty,
197200
..
198201
}) = item.kind
202+
&& let Res::Def(_, def_id) = path_res(cx, self_ty)
203+
&& let Some(local_def_id) = def_id.as_local()
199204
{
205+
let adt_hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id);
200206
let trait_ref = &of_trait.trait_ref;
201207
let ty = cx.tcx.type_of(item.owner_id).instantiate_identity();
202208
let is_automatically_derived = cx.tcx.is_automatically_derived(item.owner_id.to_def_id());
203209

204-
derived_hash_with_manual_eq::check(cx, item.span, trait_ref, ty, is_automatically_derived);
205-
derive_ord_xor_partial_ord::check(cx, item.span, trait_ref, ty, is_automatically_derived);
210+
derived_hash_with_manual_eq::check(cx, item.span, trait_ref, ty, adt_hir_id, is_automatically_derived);
211+
derive_ord_xor_partial_ord::check(cx, item.span, trait_ref, ty, adt_hir_id, is_automatically_derived);
206212

207213
if is_automatically_derived {
208-
unsafe_derive_deserialize::check(cx, item, trait_ref, ty);
209-
derive_partial_eq_without_eq::check(cx, item.span, trait_ref, ty);
214+
unsafe_derive_deserialize::check(cx, item, trait_ref, ty, adt_hir_id);
215+
derive_partial_eq_without_eq::check(cx, item.span, trait_ref, ty, adt_hir_id);
210216
} else {
211-
expl_impl_clone_on_copy::check(cx, item, trait_ref, ty);
217+
expl_impl_clone_on_copy::check(cx, item, trait_ref, ty, adt_hir_id);
212218
}
213219
}
214220
}

clippy_lints/src/derive/unsafe_derive_deserialize.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use clippy_utils::diagnostics::span_lint_hir_and_then;
44
use clippy_utils::{is_lint_allowed, paths};
55
use rustc_hir::def_id::LocalDefId;
66
use rustc_hir::intravisit::{FnKind, Visitor, walk_expr, walk_fn, walk_item};
7-
use rustc_hir::{self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, Item, UnsafeSource};
7+
use rustc_hir::{self as hir, BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, HirId, Item, UnsafeSource};
88
use rustc_lint::LateContext;
99
use rustc_middle::hir::nested_filter;
1010
use rustc_middle::ty::{self, Ty};
@@ -13,7 +13,13 @@ use rustc_span::{Span, sym};
1313
use super::UNSAFE_DERIVE_DESERIALIZE;
1414

1515
/// Implementation of the `UNSAFE_DERIVE_DESERIALIZE` lint.
16-
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) {
16+
pub(super) fn check<'tcx>(
17+
cx: &LateContext<'tcx>,
18+
item: &Item<'_>,
19+
trait_ref: &hir::TraitRef<'_>,
20+
ty: Ty<'tcx>,
21+
adt_hir_id: HirId,
22+
) {
1723
fn has_unsafe<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
1824
let mut visitor = UnsafeVisitor { cx };
1925
walk_item(&mut visitor, item).is_break()
@@ -22,8 +28,6 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &h
2228
if let Some(trait_def_id) = trait_ref.trait_def_id()
2329
&& paths::SERDE_DESERIALIZE.matches(cx, trait_def_id)
2430
&& let ty::Adt(def, _) = ty.kind()
25-
&& let Some(local_def_id) = def.did().as_local()
26-
&& let adt_hir_id = cx.tcx.local_def_id_to_hir_id(local_def_id)
2731
&& !is_lint_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id)
2832
&& cx
2933
.tcx

tests/ui/derive.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,3 +130,16 @@ fn issue14558() {
130130
}
131131

132132
fn main() {}
133+
134+
mod issue15708 {
135+
// Check that the lint posts on the type definition node
136+
#[expect(clippy::expl_impl_clone_on_copy)]
137+
#[derive(Copy)]
138+
struct S;
139+
140+
impl Clone for S {
141+
fn clone(&self) -> Self {
142+
S
143+
}
144+
}
145+
}

tests/ui/derive.stderr

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ LL | | fn clone(&self) -> Self {
99
LL | | }
1010
| |_^
1111
|
12-
note: consider deriving `Clone` or removing `Copy`
12+
help: consider deriving `Clone` or removing `Copy`
1313
--> tests/ui/derive.rs:15:1
1414
|
1515
LL | / impl Clone for Qux {
@@ -33,7 +33,7 @@ LL | | fn clone(&self) -> Self {
3333
LL | | }
3434
| |_^
3535
|
36-
note: consider deriving `Clone` or removing `Copy`
36+
help: consider deriving `Clone` or removing `Copy`
3737
--> tests/ui/derive.rs:41:1
3838
|
3939
LL | / impl<'a> Clone for Lt<'a> {
@@ -55,7 +55,7 @@ LL | | fn clone(&self) -> Self {
5555
LL | | }
5656
| |_^
5757
|
58-
note: consider deriving `Clone` or removing `Copy`
58+
help: consider deriving `Clone` or removing `Copy`
5959
--> tests/ui/derive.rs:54:1
6060
|
6161
LL | / impl Clone for BigArray {
@@ -77,7 +77,7 @@ LL | | fn clone(&self) -> Self {
7777
LL | | }
7878
| |_^
7979
|
80-
note: consider deriving `Clone` or removing `Copy`
80+
help: consider deriving `Clone` or removing `Copy`
8181
--> tests/ui/derive.rs:67:1
8282
|
8383
LL | / impl Clone for FnPtr {
@@ -99,7 +99,7 @@ LL | | fn clone(&self) -> Self {
9999
LL | | }
100100
| |_^
101101
|
102-
note: consider deriving `Clone` or removing `Copy`
102+
help: consider deriving `Clone` or removing `Copy`
103103
--> tests/ui/derive.rs:89:1
104104
|
105105
LL | / impl<T: Clone> Clone for Generic2<T> {

tests/ui/derive_ord_xor_partial_ord.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,18 @@ mod use_ord {
7676
}
7777

7878
fn main() {}
79+
80+
mod issue15708 {
81+
use std::cmp::{Ord, Ordering};
82+
83+
// Check that the lint posts on the type definition node
84+
#[expect(clippy::derive_ord_xor_partial_ord)]
85+
#[derive(PartialOrd, PartialEq, Eq)]
86+
struct DerivePartialOrdInUseOrd;
87+
88+
impl Ord for DerivePartialOrdInUseOrd {
89+
fn cmp(&self, other: &Self) -> Ordering {
90+
Ordering::Less
91+
}
92+
}
93+
}

tests/ui/derived_hash_with_manual_eq.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,19 @@ impl std::hash::Hash for Bah {
4141
}
4242

4343
fn main() {}
44+
45+
mod issue15708 {
46+
// Check that the lint posts on the type definition node
47+
#[expect(clippy::derived_hash_with_manual_eq)]
48+
#[derive(Debug, Clone, Copy, Eq, PartialOrd, Ord, Hash)]
49+
pub struct Span {
50+
start: usize,
51+
end: usize,
52+
}
53+
54+
impl PartialEq for Span {
55+
fn eq(&self, other: &Self) -> bool {
56+
self.start.cmp(&other.start).then(self.end.cmp(&other.end)).is_eq()
57+
}
58+
}
59+
}

0 commit comments

Comments
 (0)