Auto merge of #9546 - kraktus:default_not_default_trait, r=xFrednet

[`should_implement_trait`] Also lint `default` method

close https://github.com/rust-lang/rust-clippy/issues/8550

changelog: FP: [`should_implement_trait`]: Now also works for `default` methods
This commit is contained in:
bors 2022-09-28 11:00:37 +00:00
commit 0f6932a1f7
2 changed files with 49 additions and 46 deletions

View File

@ -3255,65 +3255,59 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
let self_ty = cx.tcx.type_of(item.def_id);
let implements_trait = matches!(item.kind, hir::ItemKind::Impl(hir::Impl { of_trait: Some(_), .. }));
if_chain! {
if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind;
if let Some(first_arg) = iter_input_pats(sig.decl, cx.tcx.hir().body(id)).next();
if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind {
let method_sig = cx.tcx.fn_sig(impl_item.def_id);
let method_sig = cx.tcx.erase_late_bound_regions(method_sig);
let first_arg_ty = method_sig.inputs().iter().next();
// check conventions w.r.t. conversion method names and predicates
if let Some(first_arg_ty) = first_arg_ty;
then {
// if this impl block implements a trait, lint in trait definition instead
if !implements_trait && cx.access_levels.is_exported(impl_item.def_id) {
// check missing trait implementations
for method_config in &TRAIT_METHODS {
if name == method_config.method_name &&
sig.decl.inputs.len() == method_config.param_count &&
method_config.output_type.matches(&sig.decl.output) &&
method_config.self_kind.matches(cx, self_ty, *first_arg_ty) &&
fn_header_equals(method_config.fn_header, sig.header) &&
method_config.lifetime_param_cond(impl_item)
{
span_lint_and_help(
cx,
SHOULD_IMPLEMENT_TRAIT,
impl_item.span,
&format!(
"method `{}` can be confused for the standard trait method `{}::{}`",
method_config.method_name,
method_config.trait_name,
method_config.method_name
),
None,
&format!(
"consider implementing the trait `{}` or choosing a less ambiguous method name",
method_config.trait_name
)
);
}
let first_arg_ty_opt = method_sig.inputs().iter().next().copied();
// if this impl block implements a trait, lint in trait definition instead
if !implements_trait && cx.access_levels.is_exported(impl_item.def_id) {
// check missing trait implementations
for method_config in &TRAIT_METHODS {
if name == method_config.method_name
&& sig.decl.inputs.len() == method_config.param_count
&& method_config.output_type.matches(&sig.decl.output)
// in case there is no first arg, since we already have checked the number of arguments
// it's should be always true
&& first_arg_ty_opt.map_or(true, |first_arg_ty| method_config
.self_kind.matches(cx, self_ty, first_arg_ty)
)
&& fn_header_equals(method_config.fn_header, sig.header)
&& method_config.lifetime_param_cond(impl_item)
{
span_lint_and_help(
cx,
SHOULD_IMPLEMENT_TRAIT,
impl_item.span,
&format!(
"method `{}` can be confused for the standard trait method `{}::{}`",
method_config.method_name, method_config.trait_name, method_config.method_name
),
None,
&format!(
"consider implementing the trait `{}` or choosing a less ambiguous method name",
method_config.trait_name
),
);
}
}
}
if sig.decl.implicit_self.has_implicit_self()
if sig.decl.implicit_self.has_implicit_self()
&& !(self.avoid_breaking_exported_api
&& cx.access_levels.is_exported(impl_item.def_id))
&& cx.access_levels.is_exported(impl_item.def_id))
&& let Some(first_arg) = iter_input_pats(sig.decl, cx.tcx.hir().body(id)).next()
&& let Some(first_arg_ty) = first_arg_ty_opt
{
wrong_self_convention::check(
cx,
name,
self_ty,
*first_arg_ty,
first_arg_ty,
first_arg.pat.span,
implements_trait,
false
);
}
}
}
// if this impl block implements a trait, lint in trait definition instead
@ -3799,7 +3793,6 @@ const TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
ShouldImplTraitCase::new("std::borrow::BorrowMut", "borrow_mut", 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
ShouldImplTraitCase::new("std::clone::Clone", "clone", 1, FN_HEADER, SelfKind::Ref, OutType::Any, true),
ShouldImplTraitCase::new("std::cmp::Ord", "cmp", 2, FN_HEADER, SelfKind::Ref, OutType::Any, true),
// FIXME: default doesn't work
ShouldImplTraitCase::new("std::default::Default", "default", 0, FN_HEADER, SelfKind::No, OutType::Any, true),
ShouldImplTraitCase::new("std::ops::Deref", "deref", 1, FN_HEADER, SelfKind::Ref, OutType::Ref, true),
ShouldImplTraitCase::new("std::ops::DerefMut", "deref_mut", 1, FN_HEADER, SelfKind::RefMut, OutType::Ref, true),
@ -3827,7 +3820,7 @@ enum SelfKind {
Value,
Ref,
RefMut,
No,
No, // When we want the first argument type to be different than `Self`
}
impl SelfKind {

View File

@ -99,6 +99,16 @@ LL | | }
|
= help: consider implementing the trait `std::cmp::Ord` or choosing a less ambiguous method name
error: method `default` can be confused for the standard trait method `std::default::Default::default`
--> $DIR/method_list_1.rs:65:5
|
LL | / pub fn default() -> Self {
LL | | unimplemented!()
LL | | }
| |_____^
|
= help: consider implementing the trait `std::default::Default` or choosing a less ambiguous method name
error: method `deref` can be confused for the standard trait method `std::ops::Deref::deref`
--> $DIR/method_list_1.rs:69:5
|
@ -139,5 +149,5 @@ LL | | }
|
= help: consider implementing the trait `std::ops::Drop` or choosing a less ambiguous method name
error: aborting due to 14 previous errors
error: aborting due to 15 previous errors