Skip to content

Commit 87d2891

Browse files
samueltardieusamueltardieu
committed
Cleanup should_implement_trait lint code
1 parent 12e0c4c commit 87d2891

File tree

1 file changed

+69
-84
lines changed

1 file changed

+69
-84
lines changed

‎clippy_lints/src/methods/should_implement_trait.rs‎

Lines changed: 69 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::{is_bool, sym};
33
use rustc_abi::ExternAbi;
4-
use rustc_hir as hir;
5-
use rustc_hir::{FnSig, ImplItem};
4+
use rustc_hir::{self as hir, FnRetTy, FnSig, GenericParamKind, ImplItem, LifetimeParamKind};
65
use rustc_lint::LateContext;
76
use rustc_middle::ty::Ty;
87
use rustc_span::edition::Edition::{self, Edition2015, Edition2021};
@@ -20,51 +19,43 @@ pub(super) fn check_impl_item<'tcx>(
2019
sig: &FnSig<'_>,
2120
) {
2221
// if this impl block implements a trait, lint in trait definition instead
23-
if !impl_implements_trait && cx.effective_visibilities.is_exported(impl_item.owner_id.def_id) {
22+
if !impl_implements_trait && cx.effective_visibilities.is_exported(impl_item.owner_id.def_id)
2423
// check missing trait implementations
25-
for method_config in &TRAIT_METHODS {
26-
if impl_item.ident.name == method_config.method_name
27-
&& sig.decl.inputs.len() == method_config.param_count
28-
&& method_config.output_type.matches(&sig.decl.output)
29-
// in case there is no first arg, since we already have checked the number of arguments
30-
// it's should be always true
31-
&& first_arg_ty_opt
32-
.is_none_or(|first_arg_ty| method_config.self_kind.matches(cx, self_ty, first_arg_ty))
33-
&& fn_header_equals(method_config.fn_header, sig.header)
34-
&& method_config.lifetime_param_cond(impl_item)
35-
&& method_config.in_prelude_since <= cx.tcx.sess.edition()
36-
{
37-
span_lint_and_help(
38-
cx,
39-
SHOULD_IMPLEMENT_TRAIT,
40-
impl_item.span,
41-
format!(
42-
"method `{}` can be confused for the standard trait method `{}::{}`",
43-
method_config.method_name, method_config.trait_name, method_config.method_name
44-
),
45-
None,
46-
format!(
47-
"consider implementing the trait `{}` or choosing a less ambiguous method name",
48-
method_config.trait_name
49-
),
50-
);
51-
}
52-
}
24+
&& let Some(method_config) = TRAIT_METHODS.iter().find(|case| case.method_name == impl_item.ident.name)
25+
&& sig.decl.inputs.len() == method_config.param_count
26+
&& method_config.output_type.matches(&sig.decl.output)
27+
// in case there is no first arg, since we already have checked the number of arguments
28+
// it's should be always true
29+
&& first_arg_ty_opt
30+
.is_none_or(|first_arg_ty| method_config.self_kind.matches(cx, self_ty, first_arg_ty))
31+
&& sig.header.is_safe()
32+
&& !sig.header.is_const()
33+
&& !sig.header.is_async()
34+
&& sig.header.abi == ExternAbi::Rust
35+
&& method_config.lifetime_param_cond(impl_item)
36+
&& method_config.in_prelude_since <= cx.tcx.sess.edition()
37+
{
38+
span_lint_and_help(
39+
cx,
40+
SHOULD_IMPLEMENT_TRAIT,
41+
impl_item.span,
42+
format!(
43+
"method `{}` can be confused for the standard trait method `{}::{}`",
44+
method_config.method_name, method_config.trait_name, method_config.method_name
45+
),
46+
None,
47+
format!(
48+
"consider implementing the trait `{}` or choosing a less ambiguous method name",
49+
method_config.trait_name
50+
),
51+
);
5352
}
5453
}
5554

56-
const FN_HEADER: hir::FnHeader = hir::FnHeader {
57-
safety: hir::HeaderSafety::Normal(hir::Safety::Safe),
58-
constness: hir::Constness::NotConst,
59-
asyncness: hir::IsAsync::NotAsync,
60-
abi: ExternAbi::Rust,
61-
};
62-
6355
struct ShouldImplTraitCase {
6456
trait_name: &'static str,
6557
method_name: Symbol,
6658
param_count: usize,
67-
fn_header: hir::FnHeader,
6859
// implicit self kind expected (none, self, &self, ...)
6960
self_kind: SelfKind,
7061
// checks against the output type
@@ -73,13 +64,12 @@ struct ShouldImplTraitCase {
7364
lint_explicit_lifetime: bool,
7465
in_prelude_since: Edition,
7566
}
67+
7668
impl ShouldImplTraitCase {
77-
#[expect(clippy::too_many_arguments)]
7869
const fn new(
7970
trait_name: &'static str,
8071
method_name: Symbol,
8172
param_count: usize,
82-
fn_header: hir::FnHeader,
8373
self_kind: SelfKind,
8474
output_type: OutType,
8575
lint_explicit_lifetime: bool,
@@ -89,7 +79,6 @@ impl ShouldImplTraitCase {
8979
trait_name,
9080
method_name,
9181
param_count,
92-
fn_header,
9382
self_kind,
9483
output_type,
9584
lint_explicit_lifetime,
@@ -102,8 +91,8 @@ impl ShouldImplTraitCase {
10291
|| !impl_item.generics.params.iter().any(|p| {
10392
matches!(
10493
p.kind,
105-
hir::GenericParamKind::Lifetime {
106-
kind: hir::LifetimeParamKind::Explicit
94+
GenericParamKind::Lifetime {
95+
kind: LifetimeParamKind::Explicit
10796
}
10897
)
10998
})
@@ -112,36 +101,36 @@ impl ShouldImplTraitCase {
112101

113102
#[rustfmt::skip]
114103
const TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
115-
ShouldImplTraitCase::new("std::ops::Add", sym::add, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
116-
ShouldImplTraitCase::new("std::convert::AsMut", sym::as_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true, Edition2015),
117-
ShouldImplTraitCase::new("std::convert::AsRef", sym::as_ref, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true, Edition2015),
118-
ShouldImplTraitCase::new("std::ops::BitAnd", sym::bitand, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
119-
ShouldImplTraitCase::new("std::ops::BitOr", sym::bitor, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
120-
ShouldImplTraitCase::new("std::ops::BitXor", sym::bitxor, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
121-
ShouldImplTraitCase::new("std::borrow::Borrow", sym::borrow, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true, Edition2015),
122-
ShouldImplTraitCase::new("std::borrow::BorrowMut", sym::borrow_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true, Edition2015),
123-
ShouldImplTraitCase::new("std::clone::Clone", sym::clone, 1, FN_HEADER, SelfKind::Ref, OutType::Any, true, Edition2015),
124-
ShouldImplTraitCase::new("std::cmp::Ord", sym::cmp, 2, FN_HEADER, SelfKind::Ref, OutType::Any, true, Edition2015),
125-
ShouldImplTraitCase::new("std::default::Default", kw::Default, 0, FN_HEADER, SelfKind::No, OutType::Any, true, Edition2015),
126-
ShouldImplTraitCase::new("std::ops::Deref", sym::deref, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true, Edition2015),
127-
ShouldImplTraitCase::new("std::ops::DerefMut", sym::deref_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true, Edition2015),
128-
ShouldImplTraitCase::new("std::ops::Div", sym::div, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
129-
ShouldImplTraitCase::new("std::ops::Drop", sym::drop, 1, FN_HEADER, SelfKind::RefMut, OutType::Unit, true, Edition2015),
130-
ShouldImplTraitCase::new("std::cmp::PartialEq", sym::eq, 2, FN_HEADER, SelfKind::Ref, OutType::Bool, true, Edition2015),
131-
ShouldImplTraitCase::new("std::iter::FromIterator", sym::from_iter, 1, FN_HEADER, SelfKind::No, OutType::Any, true, Edition2021),
132-
ShouldImplTraitCase::new("std::str::FromStr", sym::from_str, 1, FN_HEADER, SelfKind::No, OutType::Any, true, Edition2015),
133-
ShouldImplTraitCase::new("std::hash::Hash", sym::hash, 2, FN_HEADER, SelfKind::Ref, OutType::Unit, true, Edition2015),
134-
ShouldImplTraitCase::new("std::ops::Index", sym::index, 2, FN_HEADER, SelfKind::Ref, OutType::Ref, true, Edition2015),
135-
ShouldImplTraitCase::new("std::ops::IndexMut", sym::index_mut, 2, FN_HEADER, SelfKind::RefMut, OutType::Ref, true, Edition2015),
136-
ShouldImplTraitCase::new("std::iter::IntoIterator", sym::into_iter, 1, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
137-
ShouldImplTraitCase::new("std::ops::Mul", sym::mul, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
138-
ShouldImplTraitCase::new("std::ops::Neg", sym::neg, 1, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
139-
ShouldImplTraitCase::new("std::iter::Iterator", sym::next, 1, FN_HEADER, SelfKind::RefMut, OutType::Any, false, Edition2015),
140-
ShouldImplTraitCase::new("std::ops::Not", sym::not, 1, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
141-
ShouldImplTraitCase::new("std::ops::Rem", sym::rem, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
142-
ShouldImplTraitCase::new("std::ops::Shl", sym::shl, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
143-
ShouldImplTraitCase::new("std::ops::Shr", sym::shr, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
144-
ShouldImplTraitCase::new("std::ops::Sub", sym::sub, 2, FN_HEADER, SelfKind::Value, OutType::Any, true, Edition2015),
104+
ShouldImplTraitCase::new("std::ops::Add", sym::add, 2, SelfKind::Value, OutType::Any, true, Edition2015),
105+
ShouldImplTraitCase::new("std::convert::AsMut", sym::as_mut, 1, SelfKind::RefMut, OutType::Ref, true, Edition2015),
106+
ShouldImplTraitCase::new("std::convert::AsRef", sym::as_ref, 1, SelfKind::Ref, OutType::Ref, true, Edition2015),
107+
ShouldImplTraitCase::new("std::ops::BitAnd", sym::bitand, 2, SelfKind::Value, OutType::Any, true, Edition2015),
108+
ShouldImplTraitCase::new("std::ops::BitOr", sym::bitor, 2, SelfKind::Value, OutType::Any, true, Edition2015),
109+
ShouldImplTraitCase::new("std::ops::BitXor", sym::bitxor, 2, SelfKind::Value, OutType::Any, true, Edition2015),
110+
ShouldImplTraitCase::new("std::borrow::Borrow", sym::borrow, 1, SelfKind::Ref, OutType::Ref, true, Edition2015),
111+
ShouldImplTraitCase::new("std::borrow::BorrowMut", sym::borrow_mut, 1, SelfKind::RefMut, OutType::Ref, true, Edition2015),
112+
ShouldImplTraitCase::new("std::clone::Clone", sym::clone, 1, SelfKind::Ref, OutType::Any, true, Edition2015),
113+
ShouldImplTraitCase::new("std::cmp::Ord", sym::cmp, 2, SelfKind::Ref, OutType::Any, true, Edition2015),
114+
ShouldImplTraitCase::new("std::default::Default", kw::Default, 0, SelfKind::No, OutType::Any, true, Edition2015),
115+
ShouldImplTraitCase::new("std::ops::Deref", sym::deref, 1, SelfKind::Ref, OutType::Ref, true, Edition2015),
116+
ShouldImplTraitCase::new("std::ops::DerefMut", sym::deref_mut, 1, SelfKind::RefMut, OutType::Ref, true, Edition2015),
117+
ShouldImplTraitCase::new("std::ops::Div", sym::div, 2, SelfKind::Value, OutType::Any, true, Edition2015),
118+
ShouldImplTraitCase::new("std::ops::Drop", sym::drop, 1, SelfKind::RefMut, OutType::Unit, true, Edition2015),
119+
ShouldImplTraitCase::new("std::cmp::PartialEq", sym::eq, 2, SelfKind::Ref, OutType::Bool, true, Edition2015),
120+
ShouldImplTraitCase::new("std::iter::FromIterator", sym::from_iter, 1, SelfKind::No, OutType::Any, true, Edition2021),
121+
ShouldImplTraitCase::new("std::str::FromStr", sym::from_str, 1, SelfKind::No, OutType::Any, true, Edition2015),
122+
ShouldImplTraitCase::new("std::hash::Hash", sym::hash, 2, SelfKind::Ref, OutType::Unit, true, Edition2015),
123+
ShouldImplTraitCase::new("std::ops::Index", sym::index, 2, SelfKind::Ref, OutType::Ref, true, Edition2015),
124+
ShouldImplTraitCase::new("std::ops::IndexMut", sym::index_mut, 2, SelfKind::RefMut, OutType::Ref, true, Edition2015),
125+
ShouldImplTraitCase::new("std::iter::IntoIterator", sym::into_iter, 1, SelfKind::Value, OutType::Any, true, Edition2015),
126+
ShouldImplTraitCase::new("std::ops::Mul", sym::mul, 2, SelfKind::Value, OutType::Any, true, Edition2015),
127+
ShouldImplTraitCase::new("std::ops::Neg", sym::neg, 1, SelfKind::Value, OutType::Any, true, Edition2015),
128+
ShouldImplTraitCase::new("std::iter::Iterator", sym::next, 1, SelfKind::RefMut, OutType::Any, false, Edition2015),
129+
ShouldImplTraitCase::new("std::ops::Not", sym::not, 1, SelfKind::Value, OutType::Any, true, Edition2015),
130+
ShouldImplTraitCase::new("std::ops::Rem", sym::rem, 2, SelfKind::Value, OutType::Any, true, Edition2015),
131+
ShouldImplTraitCase::new("std::ops::Shl", sym::shl, 2, SelfKind::Value, OutType::Any, true, Edition2015),
132+
ShouldImplTraitCase::new("std::ops::Shr", sym::shr, 2, SelfKind::Value, OutType::Any, true, Edition2015),
133+
ShouldImplTraitCase::new("std::ops::Sub", sym::sub, 2, SelfKind::Value, OutType::Any, true, Edition2015),
145134
];
146135

147136
#[derive(Clone, Copy)]
@@ -153,19 +142,15 @@ enum OutType {
153142
}
154143

155144
impl OutType {
156-
fn matches(self, ty: &hir::FnRetTy<'_>) -> bool {
145+
fn matches(self, ty: &FnRetTy<'_>) -> bool {
157146
let is_unit = |ty: &hir::Ty<'_>| matches!(ty.kind, hir::TyKind::Tup(&[]));
158147
match (self, ty) {
159-
(Self::Unit, &hir::FnRetTy::DefaultReturn(_)) => true,
160-
(Self::Unit, &hir::FnRetTy::Return(ty)) if is_unit(ty) => true,
161-
(Self::Bool, &hir::FnRetTy::Return(ty)) if is_bool(ty) => true,
162-
(Self::Any, &hir::FnRetTy::Return(ty)) if !is_unit(ty) => true,
163-
(Self::Ref, &hir::FnRetTy::Return(ty)) => matches!(ty.kind, hir::TyKind::Ref(_, _)),
148+
(Self::Unit, &FnRetTy::DefaultReturn(_)) => true,
149+
(Self::Unit, &FnRetTy::Return(ty)) if is_unit(ty) => true,
150+
(Self::Bool, &FnRetTy::Return(ty)) if is_bool(ty) => true,
151+
(Self::Any, &FnRetTy::Return(ty)) if !is_unit(ty) => true,
152+
(Self::Ref, &FnRetTy::Return(ty)) => matches!(ty.kind, hir::TyKind::Ref(_, _)),
164153
_ => false,
165154
}
166155
}
167156
}
168-
169-
fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool {
170-
expected.constness == actual.constness && expected.safety == actual.safety && expected.asyncness == actual.asyncness
171-
}

0 commit comments

Comments
 (0)