Skip to content

Commit f6e223c

Browse files
ada4aada4a
committed
clean-up
1 parent 3e218d1 commit f6e223c

File tree

2 files changed

+97
-120
lines changed

2 files changed

+97
-120
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 85 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -149,13 +149,12 @@ use clippy_utils::consts::{ConstEvalCtxt, Constant};
149149
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
150150
use clippy_utils::macros::FormatArgsStorage;
151151
use clippy_utils::msrvs::{self, Msrv};
152-
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
152+
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy};
153153
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty, sym};
154154
pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES;
155155
use rustc_abi::ExternAbi;
156156
use rustc_data_structures::fx::FxHashSet;
157-
use rustc_hir as hir;
158-
use rustc_hir::{Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
157+
use rustc_hir::{self as hir, Expr, ExprKind, Node, Stmt, StmtKind, TraitItem, TraitItemKind};
159158
use rustc_lint::{LateContext, LateLintPass, LintContext};
160159
use rustc_middle::ty::{self, TraitRef, Ty};
161160
use rustc_session::impl_lint_pass;
@@ -4889,13 +4888,13 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
48894888
if impl_item.span.in_external_macro(cx.sess().source_map()) {
48904889
return;
48914890
}
4892-
let name = impl_item.ident.name;
4893-
let parent = cx.tcx.hir_get_parent_item(impl_item.hir_id()).def_id;
4894-
let item = cx.tcx.hir_expect_item(parent);
4895-
let self_ty = cx.tcx.type_of(item.owner_id).instantiate_identity();
4896-
4897-
let implements_trait = matches!(item.kind, hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }));
48984891
if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind {
4892+
let name = impl_item.ident.name;
4893+
let parent = cx.tcx.hir_get_parent_item(impl_item.hir_id()).def_id;
4894+
let item = cx.tcx.hir_expect_item(parent);
4895+
let self_ty = cx.tcx.type_of(item.owner_id).instantiate_identity();
4896+
4897+
let implements_trait = matches!(item.kind, hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }));
48994898
let method_sig = cx.tcx.fn_sig(impl_item.owner_id).instantiate_identity();
49004899
let method_sig = cx.tcx.instantiate_bound_regions_with_erased(method_sig);
49014900
let first_arg_ty_opt = method_sig.inputs().iter().next().copied();
@@ -4908,9 +4907,8 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
49084907
&& method_config.output_type.matches(&sig.decl.output)
49094908
// in case there is no first arg, since we already have checked the number of arguments
49104909
// it's should be always true
4911-
&& first_arg_ty_opt.is_none_or(|first_arg_ty| method_config
4912-
.self_kind.matches(cx, self_ty, first_arg_ty)
4913-
)
4910+
&& first_arg_ty_opt
4911+
.is_none_or(|first_arg_ty| method_config.self_kind.matches(cx, self_ty, first_arg_ty))
49144912
&& fn_header_equals(method_config.fn_header, sig.header)
49154913
&& method_config.lifetime_param_cond(impl_item)
49164914
{
@@ -4948,21 +4946,14 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
49484946
false,
49494947
);
49504948
}
4951-
}
4952-
4953-
// if this impl block implements a trait, lint in trait definition instead
4954-
if implements_trait {
4955-
return;
4956-
}
49574949

4958-
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
4959-
let ret_ty = return_ty(cx, impl_item.owner_id);
4960-
4961-
if contains_ty_adt_constructor_opaque(cx, ret_ty, self_ty) {
4962-
return;
4963-
}
4964-
4965-
if name == sym::new && ret_ty != self_ty {
4950+
// if this impl block implements a trait, lint in trait definition instead
4951+
if !implements_trait
4952+
&& impl_item.ident.name == sym::new
4953+
&& let ret_ty = return_ty(cx, impl_item.owner_id)
4954+
&& ret_ty != self_ty
4955+
&& !contains_ty_adt_constructor_opaque(cx, ret_ty, self_ty)
4956+
{
49664957
span_lint(
49674958
cx,
49684959
NEW_RET_NO_SELF,
@@ -4978,41 +4969,41 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
49784969
return;
49794970
}
49804971

4981-
if let TraitItemKind::Fn(ref sig, _) = item.kind
4982-
&& sig.decl.implicit_self.has_implicit_self()
4983-
&& let Some(first_arg_hir_ty) = sig.decl.inputs.first()
4984-
&& let Some(&first_arg_ty) = cx
4985-
.tcx
4986-
.fn_sig(item.owner_id)
4987-
.instantiate_identity()
4988-
.inputs()
4989-
.skip_binder()
4990-
.first()
4991-
{
4992-
let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id()).self_ty();
4993-
wrong_self_convention::check(
4994-
cx,
4995-
item.ident.name,
4996-
self_ty,
4997-
first_arg_ty,
4998-
first_arg_hir_ty.span,
4999-
false,
5000-
true,
5001-
);
5002-
}
4972+
if let TraitItemKind::Fn(ref sig, _) = item.kind {
4973+
if sig.decl.implicit_self.has_implicit_self()
4974+
&& let Some(first_arg_hir_ty) = sig.decl.inputs.first()
4975+
&& let Some(&first_arg_ty) = cx
4976+
.tcx
4977+
.fn_sig(item.owner_id)
4978+
.instantiate_identity()
4979+
.inputs()
4980+
.skip_binder()
4981+
.first()
4982+
{
4983+
let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id()).self_ty();
4984+
wrong_self_convention::check(
4985+
cx,
4986+
item.ident.name,
4987+
self_ty,
4988+
first_arg_ty,
4989+
first_arg_hir_ty.span,
4990+
false,
4991+
true,
4992+
);
4993+
}
50034994

5004-
if item.ident.name == sym::new
5005-
&& let TraitItemKind::Fn(_, _) = item.kind
5006-
&& let ret_ty = return_ty(cx, item.owner_id)
5007-
&& let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id()).self_ty()
5008-
&& !ret_ty.contains(self_ty)
5009-
{
5010-
span_lint(
5011-
cx,
5012-
NEW_RET_NO_SELF,
5013-
item.span,
5014-
"methods called `new` usually return `Self`",
5015-
);
4995+
if item.ident.name == sym::new
4996+
&& let ret_ty = return_ty(cx, item.owner_id)
4997+
&& let self_ty = TraitRef::identity(cx.tcx, item.owner_id.to_def_id()).self_ty()
4998+
&& !ret_ty.contains(self_ty)
4999+
{
5000+
span_lint(
5001+
cx,
5002+
NEW_RET_NO_SELF,
5003+
item.span,
5004+
"methods called `new` usually return `Self`",
5005+
);
5006+
}
50165007
}
50175008
}
50185009
}
@@ -5776,36 +5767,36 @@ impl ShouldImplTraitCase {
57765767

57775768
#[rustfmt::skip]
57785769
const TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
5779-
ShouldImplTraitCase::new("std::ops::Add", sym::add, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5780-
ShouldImplTraitCase::new("std::convert::AsMut", sym::as_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
5781-
ShouldImplTraitCase::new("std::convert::AsRef", sym::as_ref, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true),
5782-
ShouldImplTraitCase::new("std::ops::BitAnd", sym::bitand, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5783-
ShouldImplTraitCase::new("std::ops::BitOr", sym::bitor, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5784-
ShouldImplTraitCase::new("std::ops::BitXor", sym::bitxor, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5785-
ShouldImplTraitCase::new("std::borrow::Borrow", sym::borrow, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true),
5786-
ShouldImplTraitCase::new("std::borrow::BorrowMut", sym::borrow_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
5787-
ShouldImplTraitCase::new("std::clone::Clone", sym::clone, 1, FN_HEADER, SelfKind::Ref, OutType::Any, true),
5788-
ShouldImplTraitCase::new("std::cmp::Ord", sym::cmp, 2, FN_HEADER, SelfKind::Ref, OutType::Any, true),
5789-
ShouldImplTraitCase::new("std::default::Default", kw::Default, 0, FN_HEADER, SelfKind::No, OutType::Any, true),
5790-
ShouldImplTraitCase::new("std::ops::Deref", sym::deref, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true),
5791-
ShouldImplTraitCase::new("std::ops::DerefMut", sym::deref_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
5792-
ShouldImplTraitCase::new("std::ops::Div", sym::div, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5793-
ShouldImplTraitCase::new("std::ops::Drop", sym::drop, 1, FN_HEADER, SelfKind::RefMut, OutType::Unit, true),
5794-
ShouldImplTraitCase::new("std::cmp::PartialEq", sym::eq, 2, FN_HEADER, SelfKind::Ref, OutType::Bool, true),
5795-
ShouldImplTraitCase::new("std::iter::FromIterator", sym::from_iter, 1, FN_HEADER, SelfKind::No, OutType::Any, true),
5796-
ShouldImplTraitCase::new("std::str::FromStr", sym::from_str, 1, FN_HEADER, SelfKind::No, OutType::Any, true),
5797-
ShouldImplTraitCase::new("std::hash::Hash", sym::hash, 2, FN_HEADER, SelfKind::Ref, OutType::Unit, true),
5798-
ShouldImplTraitCase::new("std::ops::Index", sym::index, 2, FN_HEADER, SelfKind::Ref, OutType::Ref, true),
5799-
ShouldImplTraitCase::new("std::ops::IndexMut", sym::index_mut, 2, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
5800-
ShouldImplTraitCase::new("std::iter::IntoIterator", sym::into_iter, 1, FN_HEADER, SelfKind::Value, OutType::Any, true),
5801-
ShouldImplTraitCase::new("std::ops::Mul", sym::mul, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5802-
ShouldImplTraitCase::new("std::ops::Neg", sym::neg, 1, FN_HEADER, SelfKind::Value, OutType::Any, true),
5803-
ShouldImplTraitCase::new("std::iter::Iterator", sym::next, 1, FN_HEADER, SelfKind::RefMut, OutType::Any, false),
5804-
ShouldImplTraitCase::new("std::ops::Not", sym::not, 1, FN_HEADER, SelfKind::Value, OutType::Any, true),
5805-
ShouldImplTraitCase::new("std::ops::Rem", sym::rem, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5806-
ShouldImplTraitCase::new("std::ops::Shl", sym::shl, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5807-
ShouldImplTraitCase::new("std::ops::Shr", sym::shr, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5808-
ShouldImplTraitCase::new("std::ops::Sub", sym::sub, 2, FN_HEADER, SelfKind::Value, OutType::Any, true),
5770+
ShouldImplTraitCase::new("std::ops::Add", sym::add, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5771+
ShouldImplTraitCase::new("std::convert::AsMut", sym::as_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true ),
5772+
ShouldImplTraitCase::new("std::convert::AsRef", sym::as_ref, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true ),
5773+
ShouldImplTraitCase::new("std::ops::BitAnd", sym::bitand, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5774+
ShouldImplTraitCase::new("std::ops::BitOr", sym::bitor, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5775+
ShouldImplTraitCase::new("std::ops::BitXor", sym::bitxor, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5776+
ShouldImplTraitCase::new("std::borrow::Borrow", sym::borrow, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true ),
5777+
ShouldImplTraitCase::new("std::borrow::BorrowMut", sym::borrow_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true ),
5778+
ShouldImplTraitCase::new("std::clone::Clone", sym::clone, 1, FN_HEADER, SelfKind::Ref, OutType::Any, true ),
5779+
ShouldImplTraitCase::new("std::cmp::Ord", sym::cmp, 2, FN_HEADER, SelfKind::Ref, OutType::Any, true ),
5780+
ShouldImplTraitCase::new("std::default::Default", kw::Default, 0, FN_HEADER, SelfKind::No, OutType::Any, true ),
5781+
ShouldImplTraitCase::new("std::ops::Deref", sym::deref, 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true ),
5782+
ShouldImplTraitCase::new("std::ops::DerefMut", sym::deref_mut, 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true ),
5783+
ShouldImplTraitCase::new("std::ops::Div", sym::div, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5784+
ShouldImplTraitCase::new("std::ops::Drop", sym::drop, 1, FN_HEADER, SelfKind::RefMut, OutType::Unit, true ),
5785+
ShouldImplTraitCase::new("std::cmp::PartialEq", sym::eq, 2, FN_HEADER, SelfKind::Ref, OutType::Bool, true ),
5786+
ShouldImplTraitCase::new("std::iter::FromIterator", sym::from_iter, 1, FN_HEADER, SelfKind::No, OutType::Any, true ),
5787+
ShouldImplTraitCase::new("std::str::FromStr", sym::from_str, 1, FN_HEADER, SelfKind::No, OutType::Any, true ),
5788+
ShouldImplTraitCase::new("std::hash::Hash", sym::hash, 2, FN_HEADER, SelfKind::Ref, OutType::Unit, true ),
5789+
ShouldImplTraitCase::new("std::ops::Index", sym::index, 2, FN_HEADER, SelfKind::Ref, OutType::Ref, true ),
5790+
ShouldImplTraitCase::new("std::ops::IndexMut", sym::index_mut, 2, FN_HEADER, SelfKind::RefMut, OutType::Ref, true ),
5791+
ShouldImplTraitCase::new("std::iter::IntoIterator", sym::into_iter, 1, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5792+
ShouldImplTraitCase::new("std::ops::Mul", sym::mul, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5793+
ShouldImplTraitCase::new("std::ops::Neg", sym::neg, 1, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5794+
ShouldImplTraitCase::new("std::iter::Iterator", sym::next, 1, FN_HEADER, SelfKind::RefMut, OutType::Any, false),
5795+
ShouldImplTraitCase::new("std::ops::Not", sym::not, 1, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5796+
ShouldImplTraitCase::new("std::ops::Rem", sym::rem, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5797+
ShouldImplTraitCase::new("std::ops::Shl", sym::shl, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5798+
ShouldImplTraitCase::new("std::ops::Shr", sym::shr, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
5799+
ShouldImplTraitCase::new("std::ops::Sub", sym::sub, 2, FN_HEADER, SelfKind::Value, OutType::Any, true ),
58095800
];
58105801

58115802
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
@@ -5823,12 +5814,10 @@ impl SelfKind {
58235814
true
58245815
} else if let Some(boxed_ty) = ty.boxed_ty() {
58255816
boxed_ty == parent_ty
5826-
} else if is_type_diagnostic_item(cx, ty, sym::Rc) || is_type_diagnostic_item(cx, ty, sym::Arc) {
5827-
if let ty::Adt(_, args) = ty.kind() {
5828-
args.types().next() == Some(parent_ty)
5829-
} else {
5830-
false
5831-
}
5817+
} else if let ty::Adt(adt, args) = ty.kind()
5818+
&& matches!(cx.tcx.get_diagnostic_name(adt.did()), Some(sym::Rc | sym::Arc))
5819+
{
5820+
args.types().next() == Some(parent_ty)
58325821
} else {
58335822
false
58345823
}

‎clippy_lints/src/methods/wrong_self_convention.rs‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use crate::methods::SelfKind;
22
use clippy_utils::diagnostics::span_lint_and_help;
33
use clippy_utils::ty::is_copy;
4+
use itertools::Itertools;
45
use rustc_lint::LateContext;
56
use rustc_middle::ty::Ty;
67
use rustc_span::{Span, Symbol};
@@ -61,20 +62,20 @@ impl Convention {
6162
impl fmt::Display for Convention {
6263
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
6364
match *self {
64-
Self::Eq(this) => format!("`{this}`").fmt(f),
65-
Self::StartsWith(this) => format!("`{this}*`").fmt(f),
66-
Self::EndsWith(this) => format!("`*{this}`").fmt(f),
67-
Self::NotEndsWith(this) => format!("`~{this}`").fmt(f),
65+
Self::Eq(this) => write!(f, "`{this}`"),
66+
Self::StartsWith(this) => write!(f, "`{this}*`"),
67+
Self::EndsWith(this) => write!(f, "`*{this}`"),
68+
Self::NotEndsWith(this) => write!(f, "`~{this}`"),
6869
Self::IsSelfTypeCopy(is_true) => {
69-
format!("`self` type is{} `Copy`", if is_true { "" } else { " not" }).fmt(f)
70+
write!(f, "`self` type is{} `Copy`", if is_true { "" } else { " not" })
7071
},
7172
Self::ImplementsTrait(is_true) => {
7273
let (negation, s_suffix) = if is_true { ("", "s") } else { (" does not", "") };
73-
format!("method{negation} implement{s_suffix} a trait").fmt(f)
74+
write!(f, "method{negation} implement{s_suffix} a trait")
7475
},
7576
Self::IsTraitItem(is_true) => {
7677
let suffix = if is_true { " is" } else { " is not" };
77-
format!("method{suffix} a trait item").fmt(f)
78+
write!(f, "method{suffix} a trait item")
7879
},
7980
}
8081
}
@@ -115,18 +116,9 @@ pub(super) fn check<'tcx>(
115116

116117
let s = conventions
117118
.iter()
118-
.filter_map(|conv| {
119-
if (cut_ends_with_conv && matches!(conv, Convention::NotEndsWith(_)))
120-
|| matches!(conv, Convention::ImplementsTrait(_))
121-
|| matches!(conv, Convention::IsTraitItem(_))
122-
{
123-
None
124-
} else {
125-
Some(conv.to_string())
126-
}
127-
})
128-
.collect::<Vec<_>>()
129-
.join(" and ");
119+
.filter(|conv| !(cut_ends_with_conv && matches!(conv, Convention::NotEndsWith(_))))
120+
.filter(|conv| !matches!(conv, Convention::ImplementsTrait(_) | Convention::IsTraitItem(_)))
121+
.format(" and ");
130122

131123
format!("methods with the following characteristics: ({s})")
132124
} else {
@@ -140,11 +132,7 @@ pub(super) fn check<'tcx>(
140132
first_arg_span,
141133
format!(
142134
"{suggestion} usually take {}",
143-
&self_kinds
144-
.iter()
145-
.map(|k| k.description())
146-
.collect::<Vec<_>>()
147-
.join(" or ")
135+
self_kinds.iter().map(|k| k.description()).format(" or ")
148136
),
149137
None,
150138
"consider choosing a less ambiguous name",

0 commit comments

Comments
 (0)