Skip to content

Commit 4c6792a

Browse files
committed
also flag on arm with bodies that aren't blocks
NB: this doesn't suggest removing the inner body's curlies since there are no outer ones
1 parent 747fcf6 commit 4c6792a

File tree

4 files changed

+92
-15
lines changed

4 files changed

+92
-15
lines changed

clippy_lints/src/duplicate_match_guard.rs

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,24 @@ declare_lint_pass!(DuplicateMatchGuard => [DUPLICATE_MATCH_GUARD]);
5252

5353
impl<'tcx> LateLintPass<'tcx> for DuplicateMatchGuard {
5454
fn check_arm(&mut self, cx: &LateContext<'tcx>, arm: &'tcx Arm<'tcx>) {
55-
if let Some(guard) = arm.guard
56-
&& let ExprKind::Block(block, _) = arm.body.kind
57-
&& block.stmts.is_empty()
58-
&& let Some(trailing_expr) = block.expr
59-
&& let ExprKind::If(cond, then, None) = trailing_expr.kind
55+
let Some(guard) = arm.guard else {
56+
return;
57+
};
58+
59+
let (arm_body_expr, body_has_block) = if let ExprKind::Block(block, _) = arm.body.kind {
60+
if block.stmts.is_empty()
61+
&& let Some(trailing_expr) = block.expr
62+
{
63+
(trailing_expr, true)
64+
} else {
65+
// the body contains something other than the `if` clause -- bail out
66+
return;
67+
}
68+
} else {
69+
(arm.body, false)
70+
};
71+
72+
if let ExprKind::If(cond, then, None) = arm_body_expr.kind
6073
&& eq_expr_value(cx, guard, cond.peel_drop_temps())
6174
{
6275
let ExprKind::Block(then, _) = then.kind else {
@@ -70,15 +83,49 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateMatchGuard {
7083

7184
let sugg = snippet_with_applicability(cx, then.span, "..", &mut applicability);
7285

73-
span_lint_and_sugg(
74-
cx,
75-
DUPLICATE_MATCH_GUARD,
76-
trailing_expr.span,
77-
"condition duplicates match guard",
78-
"remove the condition",
79-
sugg.to_string(),
80-
applicability,
81-
);
86+
if body_has_block {
87+
// the common case:
88+
// ```
89+
// match 0u32 {
90+
// 0 if true => {
91+
// if true {
92+
// return;
93+
// }
94+
// }
95+
// }
96+
// ```
97+
//
98+
// suggest removing the `if` _and_ the curlies of the inner brace,
99+
// since the arm body already has braces
100+
span_lint_and_sugg(
101+
cx,
102+
DUPLICATE_MATCH_GUARD,
103+
arm_body_expr.span,
104+
"condition duplicates match guard",
105+
"remove the condition",
106+
sugg.to_string(),
107+
applicability,
108+
);
109+
} else {
110+
// the uncommon case (rusfmt would add the braces here automatically)
111+
// ```
112+
// match 0u32 {
113+
// 0 if true => if true { return; }
114+
// }
115+
// ```
116+
//
117+
// suggest removing the `if` but _not_ the curlies of the inner brace,
118+
// since there are no outer braces coming from the arm body
119+
span_lint_and_sugg(
120+
cx,
121+
DUPLICATE_MATCH_GUARD,
122+
arm_body_expr.span,
123+
"condition duplicates match guard",
124+
"remove the condition",
125+
sugg.to_string(),
126+
applicability,
127+
);
128+
}
82129
}
83130
}
84131
}

tests/ui/duplicate_match_guard.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ fn main() {
3333
return;
3434
}
3535
},
36+
// no curlies around arm body
37+
#[rustfmt::skip] // would add the outer curlies
38+
0 if true => {
39+
//~^ duplicate_match_guard
40+
return;
41+
},
3642

3743
// no warnings
3844
0 if true => {

tests/ui/duplicate_match_guard.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ fn main() {
3333
return;
3434
}
3535
},
36+
// no curlies around arm body
37+
#[rustfmt::skip] // would add the outer curlies
38+
0 if true => if true {
39+
//~^ duplicate_match_guard
40+
return;
41+
},
3642

3743
// no warnings
3844
0 if true => {

tests/ui/duplicate_match_guard.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,23 @@ LL + return;
6868
LL + }
6969
|
7070

71-
error: aborting due to 4 previous errors
71+
error: condition duplicates match guard
72+
--> tests/ui/duplicate_match_guard.rs:38:22
73+
|
74+
LL | 0 if true => if true {
75+
| ______________________^
76+
LL | |
77+
LL | | return;
78+
LL | | },
79+
| |_________^
80+
|
81+
help: remove the condition
82+
|
83+
LL ~ 0 if true => {
84+
LL +
85+
LL + return;
86+
LL ~ },
87+
|
88+
89+
error: aborting due to 5 previous errors
7290

0 commit comments

Comments
 (0)