From a4a88ea592a1a43ab7d2ece3928702f3c8d7e803 Mon Sep 17 00:00:00 2001 From: Ada Alakbarova Date: Sat, 1 Nov 2025 17:11:32 +0100 Subject: [PATCH] refactor(needless_arbitrary_self_type): give suggestions with finer diffs --- .../src/needless_arbitrary_self_type.rs | 108 ++++++++---------- tests/ui/needless_arbitrary_self_type.stderr | 63 ++++++++-- ...dless_arbitrary_self_type_unfixable.stderr | 7 +- 3 files changed, 109 insertions(+), 69 deletions(-) diff --git a/clippy_lints/src/needless_arbitrary_self_type.rs b/clippy_lints/src/needless_arbitrary_self_type.rs index 5f7fde30f03f..691d9035d02c 100644 --- a/clippy_lints/src/needless_arbitrary_self_type.rs +++ b/clippy_lints/src/needless_arbitrary_self_type.rs @@ -1,10 +1,9 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_with_applicability; -use rustc_ast::ast::{BindingMode, ByRef, Lifetime, Mutability, Param, PatKind, Path, TyKind}; +use rustc_ast::ast::{BindingMode, ByRef, Lifetime, Param, PatKind, TyKind}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::Span; use rustc_span::symbol::kw; declare_clippy_lint! { @@ -65,52 +64,6 @@ enum Mode { Value, } -fn check_param_inner(cx: &EarlyContext<'_>, path: &Path, span: Span, binding_mode: &Mode, mutbl: Mutability) { - if let [segment] = &path.segments[..] - && segment.ident.name == kw::SelfUpper - { - // In case we have a named lifetime, we check if the name comes from expansion. - // If it does, at this point we know the rest of the parameter was written by the user, - // so let them decide what the name of the lifetime should be. - // See #6089 for more details. - let mut applicability = Applicability::MachineApplicable; - let self_param = match (binding_mode, mutbl) { - (Mode::Ref(None), Mutability::Mut) => "&mut self".to_string(), - (Mode::Ref(Some(lifetime)), Mutability::Mut) => { - if lifetime.ident.span.from_expansion() { - applicability = Applicability::HasPlaceholders; - "&'_ mut self".to_string() - } else { - let lt_name = snippet_with_applicability(cx, lifetime.ident.span, "..", &mut applicability); - format!("&{lt_name} mut self") - } - }, - (Mode::Ref(None), Mutability::Not) => "&self".to_string(), - (Mode::Ref(Some(lifetime)), Mutability::Not) => { - if lifetime.ident.span.from_expansion() { - applicability = Applicability::HasPlaceholders; - "&'_ self".to_string() - } else { - let lt_name = snippet_with_applicability(cx, lifetime.ident.span, "..", &mut applicability); - format!("&{lt_name} self") - } - }, - (Mode::Value, Mutability::Mut) => "mut self".to_string(), - (Mode::Value, Mutability::Not) => "self".to_string(), - }; - - span_lint_and_sugg( - cx, - NEEDLESS_ARBITRARY_SELF_TYPE, - span, - "the type of the `self` parameter does not need to be arbitrary", - "consider to change this parameter to", - self_param, - applicability, - ); - } -} - impl EarlyLintPass for NeedlessArbitrarySelfType { fn check_param(&mut self, cx: &EarlyContext<'_>, p: &Param) { // Bail out if the parameter it's not a receiver or was not written by the user @@ -118,20 +71,55 @@ impl EarlyLintPass for NeedlessArbitrarySelfType { return; } - match &p.ty.kind { - TyKind::Path(None, path) => { - if let PatKind::Ident(BindingMode(ByRef::No, mutbl), _, _) = p.pat.kind { - check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Value, mutbl); - } + let (path, binding_mode, mutbl) = match &p.ty.kind { + TyKind::Path(None, path) if let PatKind::Ident(BindingMode(ByRef::No, mutbl), _, _) = p.pat.kind => { + (path, Mode::Value, mutbl) }, - TyKind::Ref(lifetime, mut_ty) => { + TyKind::Ref(lifetime, mut_ty) if let TyKind::Path(None, path) = &mut_ty.ty.kind - && let PatKind::Ident(BindingMode::NONE, _, _) = p.pat.kind - { - check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Ref(*lifetime), mut_ty.mutbl); - } + && let PatKind::Ident(BindingMode::NONE, _, _) = p.pat.kind => + { + (path, Mode::Ref(*lifetime), mut_ty.mutbl) }, - _ => {}, + _ => return, + }; + + let span = p.span.to(p.ty.span); + if let [segment] = &path.segments[..] + && segment.ident.name == kw::SelfUpper + { + span_lint_and_then( + cx, + NEEDLESS_ARBITRARY_SELF_TYPE, + span, + "the type of the `self` parameter does not need to be arbitrary", + |diag| { + let mut applicability = Applicability::MachineApplicable; + let add = match binding_mode { + Mode::Value => String::new(), + Mode::Ref(None) => mutbl.ref_prefix_str().to_string(), + Mode::Ref(Some(lifetime)) => { + // In case we have a named lifetime, we check if the name comes from expansion. + // If it does, at this point we know the rest of the parameter was written by the user, + // so let them decide what the name of the lifetime should be. + // See #6089 for more details. + let lt_name = if lifetime.ident.span.from_expansion() { + applicability = Applicability::HasPlaceholders; + "'_".into() + } else { + snippet_with_applicability(cx, lifetime.ident.span, "'_", &mut applicability) + }; + format!("&{lt_name} {mut_}", mut_ = mutbl.prefix_str()) + }, + }; + + let mut sugg = vec![(p.ty.span.with_lo(p.span.hi()), String::new())]; + if !add.is_empty() { + sugg.push((p.span.shrink_to_lo(), add)); + } + diag.multipart_suggestion_verbose("remove the type", sugg, applicability); + }, + ); } } } diff --git a/tests/ui/needless_arbitrary_self_type.stderr b/tests/ui/needless_arbitrary_self_type.stderr index b5c0aae8310f..bb42e5ea63f5 100644 --- a/tests/ui/needless_arbitrary_self_type.stderr +++ b/tests/ui/needless_arbitrary_self_type.stderr @@ -2,52 +2,99 @@ error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:10:16 | LL | pub fn bad(self: Self) { - | ^^^^^^^^^^ help: consider to change this parameter to: `self` + | ^^^^^^^^^^ | = note: `-D clippy::needless-arbitrary-self-type` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::needless_arbitrary_self_type)]` +help: remove the type + | +LL - pub fn bad(self: Self) { +LL + pub fn bad(self) { + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:19:20 | LL | pub fn mut_bad(mut self: Self) { - | ^^^^^^^^^^^^^^ help: consider to change this parameter to: `mut self` + | ^^^^^^^^^^^^^^ + | +help: remove the type + | +LL - pub fn mut_bad(mut self: Self) { +LL + pub fn mut_bad(mut self) { + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:28:20 | LL | pub fn ref_bad(self: &Self) { - | ^^^^^^^^^^^ help: consider to change this parameter to: `&self` + | ^^^^^^^^^^^ + | +help: remove the type + | +LL - pub fn ref_bad(self: &Self) { +LL + pub fn ref_bad(&self) { + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:37:38 | LL | pub fn ref_bad_with_lifetime<'a>(self: &'a Self) { - | ^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a self` + | ^^^^^^^^^^^^^^ + | +help: remove the type + | +LL - pub fn ref_bad_with_lifetime<'a>(self: &'a Self) { +LL + pub fn ref_bad_with_lifetime<'a>(&'a self) { + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:46:24 | LL | pub fn mut_ref_bad(self: &mut Self) { - | ^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self` + | ^^^^^^^^^^^^^^^ + | +help: remove the type + | +LL - pub fn mut_ref_bad(self: &mut Self) { +LL + pub fn mut_ref_bad(&mut self) { + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:55:42 | LL | pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) { - | ^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a mut self` + | ^^^^^^^^^^^^^^^^^^ + | +help: remove the type + | +LL - pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) { +LL + pub fn mut_ref_bad_with_lifetime<'a>(&'a mut self) { + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:74:11 | LL | fn f1(self: &'r#struct Self) {} - | ^^^^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'r#struct self` + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the type + | +LL - fn f1(self: &'r#struct Self) {} +LL + fn f1(&'r#struct self) {} + | error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type.rs:76:11 | LL | fn f2(self: &'r#struct mut Self) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'r#struct mut self` + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: remove the type + | +LL - fn f2(self: &'r#struct mut Self) {} +LL + fn f2(&'r#struct mut self) {} + | error: aborting due to 8 previous errors diff --git a/tests/ui/needless_arbitrary_self_type_unfixable.stderr b/tests/ui/needless_arbitrary_self_type_unfixable.stderr index b50e00575629..4f8f001fc5e4 100644 --- a/tests/ui/needless_arbitrary_self_type_unfixable.stderr +++ b/tests/ui/needless_arbitrary_self_type_unfixable.stderr @@ -2,10 +2,15 @@ error: the type of the `self` parameter does not need to be arbitrary --> tests/ui/needless_arbitrary_self_type_unfixable.rs:42:31 | LL | fn call_with_mut_self(self: &mut Self) {} - | ^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self` + | ^^^^^^^^^^^^^^^ | = note: `-D clippy::needless-arbitrary-self-type` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::needless_arbitrary_self_type)]` +help: remove the type + | +LL - fn call_with_mut_self(self: &mut Self) {} +LL + fn call_with_mut_self(&mut self) {} + | error: aborting due to 1 previous error