From 904f27a2ea7b13161cefc0a359b51a0e49f878b3 Mon Sep 17 00:00:00 2001 From: laurent Date: Fri, 1 Dec 2017 19:25:43 +0000 Subject: [PATCH] Do raise a same-arms warning when the two arms are separated by an arm with a guard, fix #1996. --- clippy_lints/src/copies.rs | 34 +++++++++++++++++++--------------- tests/ui/matches.rs | 2 +- tests/ui/matches.stderr | 18 ------------------ tests/ui/regex.stderr | 2 +- 4 files changed, 21 insertions(+), 35 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 5a693ce5524..140b895526d 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -178,22 +178,26 @@ fn lint_same_cond(cx: &LateContext, conds: &[&Expr]) { /// Implementation if `MATCH_SAME_ARMS`. fn lint_match_arms(cx: &LateContext, expr: &Expr) { - let hash = |arm: &Arm| -> u64 { - let mut h = SpanlessHash::new(cx); - h.hash_expr(&arm.body); - h.finish() - }; - - let eq = |lhs: &Arm, rhs: &Arm| -> bool { - // Arms with a guard are ignored, those can’t always be merged together - lhs.guard.is_none() && rhs.guard.is_none() && - SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) && - // all patterns should have the same bindings - bindings(cx, &lhs.pats[0]) == bindings(cx, &rhs.pats[0]) - }; - if let ExprMatch(_, ref arms, MatchSource::Normal) = expr.node { - if let Some((i, j)) = search_same(arms, hash, eq) { + let hash = |&(_, arm): &(usize, &Arm)| -> u64 { + let mut h = SpanlessHash::new(cx); + h.hash_expr(&arm.body); + h.finish() + }; + + let eq = |&(lindex, lhs): &(usize, &Arm), &(rindex, rhs): &(usize, &Arm)| -> bool { + let min_index = usize::min(lindex, rindex); + let max_index = usize::max(rindex, rindex); + // Arms with a guard are ignored, those can’t always be merged together + // This is also the case for arms in-between each there is an arm with a guard + (min_index..=max_index).all(|index| arms[index].guard.is_none()) && + SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) && + // all patterns should have the same bindings + bindings(cx, &lhs.pats[0]) == bindings(cx, &rhs.pats[0]) + }; + + let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect(); + if let Some((&(_, i), &(_, j))) = search_same(&indexed_arms, hash, eq) { span_lint_and_then( cx, MATCH_SAME_ARMS, diff --git a/tests/ui/matches.rs b/tests/ui/matches.rs index 352749d48e1..8130436d485 100644 --- a/tests/ui/matches.rs +++ b/tests/ui/matches.rs @@ -285,7 +285,7 @@ fn match_wild_err_arm() { Err(_) => println!("err") } - // this is a current false positive, see #1996 + // this used to be a false positive, see #1996 match x { Ok(3) => println!("ok"), Ok(x) if x*x == 64 => println!("ok 64"), diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index beb3387d038..8c0ec49e626 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -390,24 +390,6 @@ note: consider refactoring into `Ok(3) | Ok(_)` | ^^^^^^^^^^^^^^ = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: this `match` has identical arm bodies - --> $DIR/matches.rs:292:18 - | -292 | Ok(_) => println!("ok"), - | ^^^^^^^^^^^^^^ - | -note: same as this - --> $DIR/matches.rs:290:18 - | -290 | Ok(3) => println!("ok"), - | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:290:18 - | -290 | Ok(3) => println!("ok"), - | ^^^^^^^^^^^^^^ - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) - error: this `match` has identical arm bodies --> $DIR/matches.rs:298:29 | diff --git a/tests/ui/regex.stderr b/tests/ui/regex.stderr index 9f1397990bb..1c244c1df12 100644 --- a/tests/ui/regex.stderr +++ b/tests/ui/regex.stderr @@ -112,7 +112,7 @@ error: trivial regex error: trivial regex --> $DIR/regex.rs:62:40 | -62 | let trivial_backslash = Regex::new("a/.b"); +62 | let trivial_backslash = Regex::new("a//.b"); | ^^^^^^^ | = help: consider using consider using `str::contains`