Skip to content

Commit 08a7157

Browse files
JarchoJarcho
committed
Improve message for match_single_arms 
1 parent 4f8f4b4 commit 08a7157

File tree

4 files changed

+249
-234
lines changed

4 files changed

+249
-234
lines changed

‎clippy_lints/src/matches/match_same_arms.rs

Lines changed: 46 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use clippy_utils::{path_to_local, search_same, SpanlessEq, SpanlessHash};
44
use core::iter;
55
use rustc_arena::DroplessArena;
66
use rustc_ast::ast::LitKind;
7+
use rustc_errors::Applicability;
78
use rustc_hir::def_id::DefId;
89
use rustc_hir::{Arm, Expr, ExprKind, HirId, HirIdMap, HirIdSet, Pat, PatKind, RangeEnd};
910
use rustc_lint::LateContext;
@@ -13,6 +14,7 @@ use std::collections::hash_map::Entry;
1314

1415
use super::MATCH_SAME_ARMS;
1516

17+
#[allow(clippy::too_many_lines)]
1618
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
1719
let hash = |&(_, arm): &(usize, &Arm<'_>)| -> u64 {
1820
let mut h = SpanlessHash::new(cx);
@@ -96,42 +98,52 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) {
9698
};
9799

98100
let indexed_arms: Vec<(usize, &Arm<'_>)> = arms.iter().enumerate().collect();
99-
for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) {
100-
span_lint_and_then(
101-
cx,
102-
MATCH_SAME_ARMS,
103-
j.body.span,
104-
"this `match` has identical arm bodies",
105-
|diag| {
106-
diag.span_note(i.body.span, "same as this");
107-
108-
// Note: this does not use `span_suggestion` on purpose:
109-
// there is no clean way
110-
// to remove the other arm. Building a span and suggest to replace it to ""
111-
// makes an even more confusing error message. Also in order not to make up a
112-
// span for the whole pattern, the suggestion is only shown when there is only
113-
// one pattern. The user should know about `|` if they are already using it…
101+
for (&(i, arm1), &(j, arm2)) in search_same(&indexed_arms, hash, eq) {
102+
if matches!(arm2.pat.kind, PatKind::Wild) {
103+
span_lint_and_then(
104+
cx,
105+
MATCH_SAME_ARMS,
106+
arm1.span,
107+
"this match arm has an identical body to the `_` wildcard arm",
108+
|diag| {
109+
diag.span_suggestion(
110+
arm1.span,
111+
"try removing the arm",
112+
String::new(),
113+
Applicability::MaybeIncorrect,
114+
)
115+
.help("or try changing either arm body")
116+
.span_note(arm2.span, "`_` wildcard arm here");
117+
},
118+
);
119+
} else {
120+
let back_block = backwards_blocking_idxs[j];
121+
let (keep_arm, move_arm) = if back_block < i || (back_block == 0 && forwards_blocking_idxs[i] <= j) {
122+
(arm1, arm2)
123+
} else {
124+
(arm2, arm1)
125+
};
114126

115-
let lhs = snippet(cx, i.pat.span, "<pat1>");
116-
let rhs = snippet(cx, j.pat.span, "<pat2>");
127+
span_lint_and_then(
128+
cx,
129+
MATCH_SAME_ARMS,
130+
keep_arm.span,
131+
"this match arm has an identical body to another arm",
132+
|diag| {
133+
let move_pat_snip = snippet(cx, move_arm.pat.span, "<pat2>");
134+
let keep_pat_snip = snippet(cx, keep_arm.pat.span, "<pat1>");
117135

118-
if let PatKind::Wild = j.pat.kind {
119-
// if the last arm is _, then i could be integrated into _
120-
// note that i.pat cannot be _, because that would mean that we're
121-
// hiding all the subsequent arms, and rust won't compile
122-
diag.span_note(
123-
i.body.span,
124-
&format!(
125-
"`{}` has the same arm body as the `_` wildcard, consider removing it",
126-
lhs
127-
),
128-
);
129-
} else {
130-
diag.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs,))
131-
.help("...or consider changing the match arm bodies");
132-
}
133-
},
134-
);
136+
diag.span_suggestion(
137+
keep_arm.pat.span,
138+
"try merging the arm patterns",
139+
format!("{} | {}", keep_pat_snip, move_pat_snip),
140+
Applicability::MaybeIncorrect,
141+
)
142+
.help("or try changing either arm body")
143+
.span_note(move_arm.span, "other arm here");
144+
},
145+
);
146+
}
135147
}
136148
}
137149

‎tests/ui/match_same_arms.stderr

Lines changed: 79 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,128 +1,121 @@
1-
error: this `match` has identical arm bodies
2-
--> $DIR/match_same_arms.rs:13:14
1+
error: this match arm has an identical body to the `_` wildcard arm
2+
--> $DIR/match_same_arms.rs:11:9
33
|
4-
LL | _ => 0, //~ ERROR match arms have same body
5-
| ^
4+
LL | Abc::A => 0,
5+
| ^^^^^^^^^^^ help: try removing the arm
66
|
77
= note: `-D clippy::match-same-arms` implied by `-D warnings`
8-
note: same as this
9-
--> $DIR/match_same_arms.rs:11:19
8+
= help: or try changing either arm body
9+
note: `_` wildcard arm here
10+
--> $DIR/match_same_arms.rs:13:9
1011
|
11-
LL | Abc::A => 0,
12-
| ^
13-
note: `Abc::A` has the same arm body as the `_` wildcard, consider removing it
14-
--> $DIR/match_same_arms.rs:11:19
15-
|
16-
LL | Abc::A => 0,
17-
| ^
12+
LL | _ => 0, //~ ERROR match arms have same body
13+
| ^^^^^^
1814

19-
error: this `match` has identical arm bodies
20-
--> $DIR/match_same_arms.rs:18:20
21-
|
22-
LL | (.., 3) => 42, //~ ERROR match arms have same body
23-
| ^^
24-
|
25-
note: same as this
26-
--> $DIR/match_same_arms.rs:17:23
27-
|
28-
LL | (1, .., 3) => 42,
29-
| ^^
30-
help: consider refactoring into `(1, .., 3) | (.., 3)`
15+
error: this match arm has an identical body to another arm
3116
--> $DIR/match_same_arms.rs:17:9
3217
|
3318
LL | (1, .., 3) => 42,
34-
| ^^^^^^^^^^
35-
= help: ...or consider changing the match arm bodies
19+
| ----------^^^^^^
20+
| |
21+
| help: try merging the arm patterns: `(1, .., 3) | (.., 3)`
22+
|
23+
= help: or try changing either arm body
24+
note: other arm here
25+
--> $DIR/match_same_arms.rs:18:9
26+
|
27+
LL | (.., 3) => 42, //~ ERROR match arms have same body
28+
| ^^^^^^^^^^^^^
3629

37-
error: this `match` has identical arm bodies
38-
--> $DIR/match_same_arms.rs:24:15
30+
error: this match arm has an identical body to another arm
31+
--> $DIR/match_same_arms.rs:24:9
3932
|
4033
LL | 51 => 1, //~ ERROR match arms have same body
41-
| ^
34+
| --^^^^^
35+
| |
36+
| help: try merging the arm patterns: `51 | 42`
4237
|
43-
note: same as this
44-
--> $DIR/match_same_arms.rs:23:15
45-
|
46-
LL | 42 => 1,
47-
| ^
48-
help: consider refactoring into `42 | 51`
38+
= help: or try changing either arm body
39+
note: other arm here
4940
--> $DIR/match_same_arms.rs:23:9
5041
|
5142
LL | 42 => 1,
52-
| ^^
53-
= help: ...or consider changing the match arm bodies
43+
| ^^^^^^^
5444

55-
error: this `match` has identical arm bodies
56-
--> $DIR/match_same_arms.rs:26:15
57-
|
58-
LL | 52 => 2, //~ ERROR match arms have same body
59-
| ^
60-
|
61-
note: same as this
62-
--> $DIR/match_same_arms.rs:25:15
63-
|
64-
LL | 41 => 2,
65-
| ^
66-
help: consider refactoring into `41 | 52`
45+
error: this match arm has an identical body to another arm
6746
--> $DIR/match_same_arms.rs:25:9
6847
|
6948
LL | 41 => 2,
70-
| ^^
71-
= help: ...or consider changing the match arm bodies
49+
| --^^^^^
50+
| |
51+
| help: try merging the arm patterns: `41 | 52`
52+
|
53+
= help: or try changing either arm body
54+
note: other arm here
55+
--> $DIR/match_same_arms.rs:26:9
56+
|
57+
LL | 52 => 2, //~ ERROR match arms have same body
58+
| ^^^^^^^
7259

73-
error: this `match` has identical arm bodies
74-
--> $DIR/match_same_arms.rs:32:14
60+
error: this match arm has an identical body to another arm
61+
--> $DIR/match_same_arms.rs:32:9
7562
|
7663
LL | 2 => 2, //~ ERROR 2nd matched arms have same body
77-
| ^
78-
|
79-
note: same as this
80-
--> $DIR/match_same_arms.rs:31:14
64+
| -^^^^^
65+
| |
66+
| help: try merging the arm patterns: `2 | 1`
8167
|
82-
LL | 1 => 2,
83-
| ^
84-
help: consider refactoring into `1 | 2`
68+
= help: or try changing either arm body
69+
note: other arm here
8570
--> $DIR/match_same_arms.rs:31:9
8671
|
8772
LL | 1 => 2,
88-
| ^
89-
= help: ...or consider changing the match arm bodies
73+
| ^^^^^^
9074

91-
error: this `match` has identical arm bodies
92-
--> $DIR/match_same_arms.rs:33:14
75+
error: this match arm has an identical body to another arm
76+
--> $DIR/match_same_arms.rs:33:9
9377
|
9478
LL | 3 => 2, //~ ERROR 3rd matched arms have same body
95-
| ^
96-
|
97-
note: same as this
98-
--> $DIR/match_same_arms.rs:31:14
79+
| -^^^^^
80+
| |
81+
| help: try merging the arm patterns: `3 | 1`
9982
|
100-
LL | 1 => 2,
101-
| ^
102-
help: consider refactoring into `1 | 3`
83+
= help: or try changing either arm body
84+
note: other arm here
10385
--> $DIR/match_same_arms.rs:31:9
10486
|
10587
LL | 1 => 2,
106-
| ^
107-
= help: ...or consider changing the match arm bodies
88+
| ^^^^^^
10889

109-
error: this `match` has identical arm bodies
110-
--> $DIR/match_same_arms.rs:50:55
90+
error: this match arm has an identical body to another arm
91+
--> $DIR/match_same_arms.rs:32:9
11192
|
112-
LL | CommandInfo::External { name, .. } => name.to_string(),
113-
| ^^^^^^^^^^^^^^^^
93+
LL | 2 => 2, //~ ERROR 2nd matched arms have same body
94+
| -^^^^^
95+
| |
96+
| help: try merging the arm patterns: `2 | 3`
11497
|
115-
note: same as this
116-
--> $DIR/match_same_arms.rs:49:54
98+
= help: or try changing either arm body
99+
note: other arm here
100+
--> $DIR/match_same_arms.rs:33:9
117101
|
118-
LL | CommandInfo::BuiltIn { name, .. } => name.to_string(),
119-
| ^^^^^^^^^^^^^^^^
120-
help: consider refactoring into `CommandInfo::BuiltIn { name, .. } | CommandInfo::External { name, .. }`
102+
LL | 3 => 2, //~ ERROR 3rd matched arms have same body
103+
| ^^^^^^
104+
105+
error: this match arm has an identical body to another arm
106+
--> $DIR/match_same_arms.rs:50:17
107+
|
108+
LL | CommandInfo::External { name, .. } => name.to_string(),
109+
| ----------------------------------^^^^^^^^^^^^^^^^^^^^
110+
| |
111+
| help: try merging the arm patterns: `CommandInfo::External { name, .. } | CommandInfo::BuiltIn { name, .. }`
112+
|
113+
= help: or try changing either arm body
114+
note: other arm here
121115
--> $DIR/match_same_arms.rs:49:17
122116
|
123117
LL | CommandInfo::BuiltIn { name, .. } => name.to_string(),
124-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
125-
= help: ...or consider changing the match arm bodies
118+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
126119

127-
error: aborting due to 7 previous errors
120+
error: aborting due to 8 previous errors
128121

‎tests/ui/match_same_arms2.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,27 @@ fn main() {
181181
Z(u32),
182182
}
183183

184+
// Don't lint. `Foo::X(0)` and `Foo::Z(_)` overlap with the arm in between.
184185
let _ = match Foo::X(0) {
185186
Foo::X(0) => 1,
186187
Foo::X(_) | Foo::Y(_) | Foo::Z(0) => 2,
187188
Foo::Z(_) => 1,
188189
_ => 0,
189190
};
191+
192+
// Suggest moving `Foo::Z(_)` up.
193+
let _ = match Foo::X(0) {
194+
Foo::X(0) => 1,
195+
Foo::X(_) | Foo::Y(_) => 2,
196+
Foo::Z(_) => 1,
197+
_ => 0,
198+
};
199+
200+
// Suggest moving `Foo::X(0)` down.
201+
let _ = match Foo::X(0) {
202+
Foo::X(0) => 1,
203+
Foo::Y(_) | Foo::Z(0) => 2,
204+
Foo::Z(_) => 1,
205+
_ => 0,
206+
};
190207
}

0 commit comments

Comments
 (0)