Skip to content

Commit a1182d2

Browse files
samueltardieusamueltardieu
committed
Lookup record through a hash map instead of a linear scan
1 parent 12e0c4c commit a1182d2

File tree

1 file changed

+67
-75
lines changed

1 file changed

+67
-75
lines changed

‎clippy_lints/src/methods/should_implement_trait.rs‎

Lines changed: 67 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
use std::sync::OnceLock;
2+
13
use clippy_utils::diagnostics::span_lint_and_help;
24
use clippy_utils::{is_bool, sym};
35
use rustc_abi::ExternAbi;
4-
use rustc_hir as hir;
5-
use rustc_hir::{FnSig, ImplItem};
6+
use rustc_data_structures::fx::FxHashMap;
7+
use rustc_hir::{self as hir, FnRetTy, FnSig, GenericParamKind, ImplItem, LifetimeParamKind};
68
use rustc_lint::LateContext;
79
use rustc_middle::ty::Ty;
810
use rustc_span::edition::Edition::{self, Edition2015, Edition2021};
@@ -22,49 +24,45 @@ pub(super) fn check_impl_item<'tcx>(
2224
// if this impl block implements a trait, lint in trait definition instead
2325
if !impl_implements_trait && cx.effective_visibilities.is_exported(impl_item.owner_id.def_id) {
2426
// check missing trait implementations
25-
for method_config in &TRAIT_METHODS {
26-
if impl_item.ident.name == method_config.method_name
27+
static TRAIT_METHODS: OnceLock<FxHashMap<Symbol, ShouldImplTraitCase>> = OnceLock::new();
28+
let trait_methods =
29+
TRAIT_METHODS.get_or_init(|| TRAIT_METHODS_DEFS.into_iter().map(|s| (s.method_name, s)).collect());
30+
if let Some(method_config) = trait_methods.get(&impl_item.ident.name)
2731
&& sig.decl.inputs.len() == method_config.param_count
2832
&& method_config.output_type.matches(&sig.decl.output)
2933
// in case there is no first arg, since we already have checked the number of arguments
3034
// it's should be always true
3135
&& first_arg_ty_opt
3236
.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)
37+
&& sig.header.is_safe()
38+
&& !sig.header.is_const()
39+
&& !sig.header.is_async()
40+
&& sig.header.abi == ExternAbi::Rust
3441
&& method_config.lifetime_param_cond(impl_item)
3542
&& 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-
}
43+
{
44+
span_lint_and_help(
45+
cx,
46+
SHOULD_IMPLEMENT_TRAIT,
47+
impl_item.span,
48+
format!(
49+
"method `{}` can be confused for the standard trait method `{}::{}`",
50+
method_config.method_name, method_config.trait_name, method_config.method_name
51+
),
52+
None,
53+
format!(
54+
"consider implementing the trait `{}` or choosing a less ambiguous method name",
55+
method_config.trait_name
56+
),
57+
);
5258
}
5359
}
5460
}
5561

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-
6362
struct ShouldImplTraitCase {
6463
trait_name: &'static str,
6564
method_name: Symbol,
6665
param_count: usize,
67-
fn_header: hir::FnHeader,
6866
// implicit self kind expected (none, self, &self, ...)
6967
self_kind: SelfKind,
7068
// checks against the output type
@@ -73,13 +71,12 @@ struct ShouldImplTraitCase {
7371
lint_explicit_lifetime: bool,
7472
in_prelude_since: Edition,
7573
}
74+
7675
impl ShouldImplTraitCase {
77-
#[expect(clippy::too_many_arguments)]
7876
const fn new(
7977
trait_name: &'static str,
8078
method_name: Symbol,
8179
param_count: usize,
82-
fn_header: hir::FnHeader,
8380
self_kind: SelfKind,
8481
output_type: OutType,
8582
lint_explicit_lifetime: bool,
@@ -89,7 +86,6 @@ impl ShouldImplTraitCase {
8986
trait_name,
9087
method_name,
9188
param_count,
92-
fn_header,
9389
self_kind,
9490
output_type,
9591
lint_explicit_lifetime,
@@ -102,46 +98,46 @@ impl ShouldImplTraitCase {
10298
|| !impl_item.generics.params.iter().any(|p| {
10399
matches!(
104100
p.kind,
105-
hir::GenericParamKind::Lifetime {
106-
kind: hir::LifetimeParamKind::Explicit
101+
GenericParamKind::Lifetime {
102+
kind: LifetimeParamKind::Explicit
107103
}
108104
)
109105
})
110106
}
111107
}
112108

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

147143
#[derive(Clone, Copy)]
@@ -153,19 +149,15 @@ enum OutType {
153149
}
154150

155151
impl OutType {
156-
fn matches(self, ty: &hir::FnRetTy<'_>) -> bool {
152+
fn matches(self, ty: &FnRetTy<'_>) -> bool {
157153
let is_unit = |ty: &hir::Ty<'_>| matches!(ty.kind, hir::TyKind::Tup(&[]));
158154
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(_, _)),
155+
(Self::Unit, &FnRetTy::DefaultReturn(_)) => true,
156+
(Self::Unit, &FnRetTy::Return(ty)) if is_unit(ty) => true,
157+
(Self::Bool, &FnRetTy::Return(ty)) if is_bool(ty) => true,
158+
(Self::Any, &FnRetTy::Return(ty)) if !is_unit(ty) => true,
159+
(Self::Ref, &FnRetTy::Return(ty)) => matches!(ty.kind, hir::TyKind::Ref(_, _)),
164160
_ => false,
165161
}
166162
}
167163
}
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)