Skip to content

Commit b698bc2

Browse files
committed
give suggestions with finer diffs
This also makes it feasible to collapse cases with different mutabilities
1 parent 84fd155 commit b698bc2

File tree

3 files changed

+90
-45
lines changed

3 files changed

+90
-45
lines changed

clippy_lints/src/needless_arbitrary_self_type.rs

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
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, 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;
@@ -88,44 +88,37 @@ impl EarlyLintPass for NeedlessArbitrarySelfType {
8888
if let [segment] = &path.segments[..]
8989
&& segment.ident.name == kw::SelfUpper
9090
{
91-
// In case we have a named lifetime, we check if the name comes from expansion.
92-
// If it does, at this point we know the rest of the parameter was written by the user,
93-
// so let them decide what the name of the lifetime should be.
94-
// See #6089 for more details.
95-
let mut applicability = Applicability::MachineApplicable;
96-
let self_param = match (binding_mode, mutbl) {
97-
(Mode::Ref(None), Mutability::Mut) => "&mut self".to_string(),
98-
(Mode::Ref(Some(lifetime)), Mutability::Mut) => {
99-
if lifetime.ident.span.from_expansion() {
100-
applicability = Applicability::HasPlaceholders;
101-
"&'_ mut self".to_string()
102-
} else {
103-
let lt_name = snippet_with_applicability(cx, lifetime.ident.span, "'_", &mut applicability);
104-
format!("&{lt_name} mut self")
105-
}
106-
},
107-
(Mode::Ref(None), Mutability::Not) => "&self".to_string(),
108-
(Mode::Ref(Some(lifetime)), Mutability::Not) => {
109-
if lifetime.ident.span.from_expansion() {
110-
applicability = Applicability::HasPlaceholders;
111-
"&'_ self".to_string()
112-
} else {
113-
let lt_name = snippet_with_applicability(cx, lifetime.ident.span, "'_", &mut applicability);
114-
format!("&{lt_name} self")
115-
}
116-
},
117-
(Mode::Value, Mutability::Mut) => "mut self".to_string(),
118-
(Mode::Value, Mutability::Not) => "self".to_string(),
119-
};
120-
121-
span_lint_and_sugg(
91+
span_lint_and_then(
12292
cx,
12393
NEEDLESS_ARBITRARY_SELF_TYPE,
12494
span,
12595
"the type of the `self` parameter does not need to be arbitrary",
126-
"consider to change this parameter to",
127-
self_param,
128-
applicability,
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+
},
129122
);
130123
}
131124
}

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)