Skip to content

Commit 904f27a

Browse files
committed
Do raise a same-arms warning when the two arms are separated by an arm with a guard, fix rust-lang#1996.
1 parent 876d6d8 commit 904f27a

File tree

4 files changed

+20
-34
lines changed

4 files changed

+20
-34
lines changed

clippy_lints/src/copies.rs

+18-14
Original file line numberDiff line numberDiff line change
@@ -178,22 +178,26 @@ fn lint_same_cond(cx: &LateContext, conds: &[&Expr]) {
178178

179179
/// Implementation if `MATCH_SAME_ARMS`.
180180
fn lint_match_arms(cx: &LateContext, expr: &Expr) {
181-
let hash = |arm: &Arm| -> u64 {
182-
let mut h = SpanlessHash::new(cx);
183-
h.hash_expr(&arm.body);
184-
h.finish()
185-
};
181+
if let ExprMatch(_, ref arms, MatchSource::Normal) = expr.node {
182+
let hash = |&(_, arm): &(usize, &Arm)| -> u64 {
183+
let mut h = SpanlessHash::new(cx);
184+
h.hash_expr(&arm.body);
185+
h.finish()
186+
};
186187

187-
let eq = |lhs: &Arm, rhs: &Arm| -> bool {
188-
// Arms with a guard are ignored, those can’t always be merged together
189-
lhs.guard.is_none() && rhs.guard.is_none() &&
190-
SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) &&
191-
// all patterns should have the same bindings
192-
bindings(cx, &lhs.pats[0]) == bindings(cx, &rhs.pats[0])
193-
};
188+
let eq = |&(lindex, lhs): &(usize, &Arm), &(rindex, rhs): &(usize, &Arm)| -> bool {
189+
let min_index = usize::min(lindex, rindex);
190+
let max_index = usize::max(rindex, rindex);
191+
// Arms with a guard are ignored, those can’t always be merged together
192+
// This is also the case for arms in-between each there is an arm with a guard
193+
(min_index..=max_index).all(|index| arms[index].guard.is_none()) &&
194+
SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) &&
195+
// all patterns should have the same bindings
196+
bindings(cx, &lhs.pats[0]) == bindings(cx, &rhs.pats[0])
197+
};
194198

195-
if let ExprMatch(_, ref arms, MatchSource::Normal) = expr.node {
196-
if let Some((i, j)) = search_same(arms, hash, eq) {
199+
let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect();
200+
if let Some((&(_, i), &(_, j))) = search_same(&indexed_arms, hash, eq) {
197201
span_lint_and_then(
198202
cx,
199203
MATCH_SAME_ARMS,

tests/ui/matches.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ fn match_wild_err_arm() {
285285
Err(_) => println!("err")
286286
}
287287

288-
// this is a current false positive, see #1996
288+
// this used to be a false positive, see #1996
289289
match x {
290290
Ok(3) => println!("ok"),
291291
Ok(x) if x*x == 64 => println!("ok 64"),

tests/ui/matches.stderr

-18
Original file line numberDiff line numberDiff line change
@@ -390,24 +390,6 @@ note: consider refactoring into `Ok(3) | Ok(_)`
390390
| ^^^^^^^^^^^^^^
391391
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
392392

393-
error: this `match` has identical arm bodies
394-
--> $DIR/matches.rs:292:18
395-
|
396-
292 | Ok(_) => println!("ok"),
397-
| ^^^^^^^^^^^^^^
398-
|
399-
note: same as this
400-
--> $DIR/matches.rs:290:18
401-
|
402-
290 | Ok(3) => println!("ok"),
403-
| ^^^^^^^^^^^^^^
404-
note: consider refactoring into `Ok(3) | Ok(_)`
405-
--> $DIR/matches.rs:290:18
406-
|
407-
290 | Ok(3) => println!("ok"),
408-
| ^^^^^^^^^^^^^^
409-
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
410-
411393
error: this `match` has identical arm bodies
412394
--> $DIR/matches.rs:298:29
413395
|

tests/ui/regex.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ error: trivial regex
112112
error: trivial regex
113113
--> $DIR/regex.rs:62:40
114114
|
115-
62 | let trivial_backslash = Regex::new("a/.b");
115+
62 | let trivial_backslash = Regex::new("a//.b");
116116
| ^^^^^^^
117117
|
118118
= help: consider using consider using `str::contains`

0 commit comments

Comments
 (0)