Skip to content

Commit a4a88ea

Browse files
ada4aada4a
committed
refactor(needless_arbitrary_self_type): give suggestions with finer diffs
1 parent ee4390f commit a4a88ea

File tree

3 files changed

+109
-69
lines changed

3 files changed

+109
-69
lines changed

‎clippy_lints/src/needless_arbitrary_self_type.rs‎

Lines changed: 48 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::snippet_with_applicability;
3-
use rustc_ast::ast::{BindingMode, ByRef, Lifetime, Mutability, Param, PatKind, Path, TyKind};
3+
use rustc_ast::ast::{BindingMode, ByRef, Lifetime, Param, PatKind, TyKind};
44
use rustc_errors::Applicability;
55
use rustc_lint::{EarlyContext, EarlyLintPass};
66
use rustc_session::declare_lint_pass;
7-
use rustc_span::Span;
87
use rustc_span::symbol::kw;
98

109
declare_clippy_lint! {
@@ -65,73 +64,62 @@ enum Mode {
6564
Value,
6665
}
6766

68-
fn check_param_inner(cx: &EarlyContext<'_>, path: &Path, span: Span, binding_mode: &Mode, mutbl: Mutability) {
69-
if let [segment] = &path.segments[..]
70-
&& segment.ident.name == kw::SelfUpper
71-
{
72-
// In case we have a named lifetime, we check if the name comes from expansion.
73-
// If it does, at this point we know the rest of the parameter was written by the user,
74-
// so let them decide what the name of the lifetime should be.
75-
// See #6089 for more details.
76-
let mut applicability = Applicability::MachineApplicable;
77-
let self_param = match (binding_mode, mutbl) {
78-
(Mode::Ref(None), Mutability::Mut) => "&mut self".to_string(),
79-
(Mode::Ref(Some(lifetime)), Mutability::Mut) => {
80-
if lifetime.ident.span.from_expansion() {
81-
applicability = Applicability::HasPlaceholders;
82-
"&'_ mut self".to_string()
83-
} else {
84-
let lt_name = snippet_with_applicability(cx, lifetime.ident.span, "..", &mut applicability);
85-
format!("&{lt_name} mut self")
86-
}
87-
},
88-
(Mode::Ref(None), Mutability::Not) => "&self".to_string(),
89-
(Mode::Ref(Some(lifetime)), Mutability::Not) => {
90-
if lifetime.ident.span.from_expansion() {
91-
applicability = Applicability::HasPlaceholders;
92-
"&'_ self".to_string()
93-
} else {
94-
let lt_name = snippet_with_applicability(cx, lifetime.ident.span, "..", &mut applicability);
95-
format!("&{lt_name} self")
96-
}
97-
},
98-
(Mode::Value, Mutability::Mut) => "mut self".to_string(),
99-
(Mode::Value, Mutability::Not) => "self".to_string(),
100-
};
101-
102-
span_lint_and_sugg(
103-
cx,
104-
NEEDLESS_ARBITRARY_SELF_TYPE,
105-
span,
106-
"the type of the `self` parameter does not need to be arbitrary",
107-
"consider to change this parameter to",
108-
self_param,
109-
applicability,
110-
);
111-
}
112-
}
113-
11467
impl EarlyLintPass for NeedlessArbitrarySelfType {
11568
fn check_param(&mut self, cx: &EarlyContext<'_>, p: &Param) {
11669
// Bail out if the parameter it's not a receiver or was not written by the user
11770
if !p.is_self() || p.span.from_expansion() {
11871
return;
11972
}
12073

121-
match &p.ty.kind {
122-
TyKind::Path(None, path) => {
123-
if let PatKind::Ident(BindingMode(ByRef::No, mutbl), _, _) = p.pat.kind {
124-
check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Value, mutbl);
125-
}
74+
let (path, binding_mode, mutbl) = match &p.ty.kind {
75+
TyKind::Path(None, path) if let PatKind::Ident(BindingMode(ByRef::No, mutbl), _, _) = p.pat.kind => {
76+
(path, Mode::Value, mutbl)
12677
},
127-
TyKind::Ref(lifetime, mut_ty) => {
78+
TyKind::Ref(lifetime, mut_ty)
12879
if let TyKind::Path(None, path) = &mut_ty.ty.kind
129-
&& let PatKind::Ident(BindingMode::NONE, _, _) = p.pat.kind
130-
{
131-
check_param_inner(cx, path, p.span.to(p.ty.span), &Mode::Ref(*lifetime), mut_ty.mutbl);
132-
}
80+
&& let PatKind::Ident(BindingMode::NONE, _, _) = p.pat.kind =>
81+
{
82+
(path, Mode::Ref(*lifetime), mut_ty.mutbl)
13383
},
134-
_ => {},
84+
_ => return,
85+
};
86+
87+
let span = p.span.to(p.ty.span);
88+
if let [segment] = &path.segments[..]
89+
&& segment.ident.name == kw::SelfUpper
90+
{
91+
span_lint_and_then(
92+
cx,
93+
NEEDLESS_ARBITRARY_SELF_TYPE,
94+
span,
95+
"the type of the `self` parameter does not need to be arbitrary",
96+
|diag| {
97+
let mut applicability = Applicability::MachineApplicable;
98+
let add = match binding_mode {
99+
Mode::Value => String::new(),
100+
Mode::Ref(None) => mutbl.ref_prefix_str().to_string(),
101+
Mode::Ref(Some(lifetime)) => {
102+
// In case we have a named lifetime, we check if the name comes from expansion.
103+
// If it does, at this point we know the rest of the parameter was written by the user,
104+
// so let them decide what the name of the lifetime should be.
105+
// See #6089 for more details.
106+
let lt_name = if lifetime.ident.span.from_expansion() {
107+
applicability = Applicability::HasPlaceholders;
108+
"'_".into()
109+
} else {
110+
snippet_with_applicability(cx, lifetime.ident.span, "'_", &mut applicability)
111+
};
112+
format!("&{lt_name} {mut_}", mut_ = mutbl.prefix_str())
113+
},
114+
};
115+
116+
let mut sugg = vec![(p.ty.span.with_lo(p.span.hi()), String::new())];
117+
if !add.is_empty() {
118+
sugg.push((p.span.shrink_to_lo(), add));
119+
}
120+
diag.multipart_suggestion_verbose("remove the type", sugg, applicability);
121+
},
122+
);
135123
}
136124
}
137125
}

‎tests/ui/needless_arbitrary_self_type.stderr‎

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,52 +2,99 @@ error: the type of the `self` parameter does not need to be arbitrary
22
--> tests/ui/needless_arbitrary_self_type.rs:10:16
33
|
44
LL | pub fn bad(self: Self) {
5-
| ^^^^^^^^^^ help: consider to change this parameter to: `self`
5+
| ^^^^^^^^^^
66
|
77
= note: `-D clippy::needless-arbitrary-self-type` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::needless_arbitrary_self_type)]`
9+
help: remove the type
10+
|
11+
LL - pub fn bad(self: Self) {
12+
LL + pub fn bad(self) {
13+
|
914

1015
error: the type of the `self` parameter does not need to be arbitrary
1116
--> tests/ui/needless_arbitrary_self_type.rs:19:20
1217
|
1318
LL | pub fn mut_bad(mut self: Self) {
14-
| ^^^^^^^^^^^^^^ help: consider to change this parameter to: `mut self`
19+
| ^^^^^^^^^^^^^^
20+
|
21+
help: remove the type
22+
|
23+
LL - pub fn mut_bad(mut self: Self) {
24+
LL + pub fn mut_bad(mut self) {
25+
|
1526

1627
error: the type of the `self` parameter does not need to be arbitrary
1728
--> tests/ui/needless_arbitrary_self_type.rs:28:20
1829
|
1930
LL | pub fn ref_bad(self: &Self) {
20-
| ^^^^^^^^^^^ help: consider to change this parameter to: `&self`
31+
| ^^^^^^^^^^^
32+
|
33+
help: remove the type
34+
|
35+
LL - pub fn ref_bad(self: &Self) {
36+
LL + pub fn ref_bad(&self) {
37+
|
2138

2239
error: the type of the `self` parameter does not need to be arbitrary
2340
--> tests/ui/needless_arbitrary_self_type.rs:37:38
2441
|
2542
LL | pub fn ref_bad_with_lifetime<'a>(self: &'a Self) {
26-
| ^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a self`
43+
| ^^^^^^^^^^^^^^
44+
|
45+
help: remove the type
46+
|
47+
LL - pub fn ref_bad_with_lifetime<'a>(self: &'a Self) {
48+
LL + pub fn ref_bad_with_lifetime<'a>(&'a self) {
49+
|
2750

2851
error: the type of the `self` parameter does not need to be arbitrary
2952
--> tests/ui/needless_arbitrary_self_type.rs:46:24
3053
|
3154
LL | pub fn mut_ref_bad(self: &mut Self) {
32-
| ^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self`
55+
| ^^^^^^^^^^^^^^^
56+
|
57+
help: remove the type
58+
|
59+
LL - pub fn mut_ref_bad(self: &mut Self) {
60+
LL + pub fn mut_ref_bad(&mut self) {
61+
|
3362

3463
error: the type of the `self` parameter does not need to be arbitrary
3564
--> tests/ui/needless_arbitrary_self_type.rs:55:42
3665
|
3766
LL | pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
38-
| ^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'a mut self`
67+
| ^^^^^^^^^^^^^^^^^^
68+
|
69+
help: remove the type
70+
|
71+
LL - pub fn mut_ref_bad_with_lifetime<'a>(self: &'a mut Self) {
72+
LL + pub fn mut_ref_bad_with_lifetime<'a>(&'a mut self) {
73+
|
3974

4075
error: the type of the `self` parameter does not need to be arbitrary
4176
--> tests/ui/needless_arbitrary_self_type.rs:74:11
4277
|
4378
LL | fn f1(self: &'r#struct Self) {}
44-
| ^^^^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'r#struct self`
79+
| ^^^^^^^^^^^^^^^^^^^^^
80+
|
81+
help: remove the type
82+
|
83+
LL - fn f1(self: &'r#struct Self) {}
84+
LL + fn f1(&'r#struct self) {}
85+
|
4586

4687
error: the type of the `self` parameter does not need to be arbitrary
4788
--> tests/ui/needless_arbitrary_self_type.rs:76:11
4889
|
4990
LL | fn f2(self: &'r#struct mut Self) {}
50-
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&'r#struct mut self`
91+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
92+
|
93+
help: remove the type
94+
|
95+
LL - fn f2(self: &'r#struct mut Self) {}
96+
LL + fn f2(&'r#struct mut self) {}
97+
|
5198

5299
error: aborting due to 8 previous errors
53100

‎tests/ui/needless_arbitrary_self_type_unfixable.stderr‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@ error: the type of the `self` parameter does not need to be arbitrary
22
--> tests/ui/needless_arbitrary_self_type_unfixable.rs:42:31
33
|
44
LL | fn call_with_mut_self(self: &mut Self) {}
5-
| ^^^^^^^^^^^^^^^ help: consider to change this parameter to: `&mut self`
5+
| ^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::needless-arbitrary-self-type` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::needless_arbitrary_self_type)]`
9+
help: remove the type
10+
|
11+
LL - fn call_with_mut_self(self: &mut Self) {}
12+
LL + fn call_with_mut_self(&mut self) {}
13+
|
914

1015
error: aborting due to 1 previous error
1116

0 commit comments

Comments
 (0)