From 902726c38d79d4e63567f2bbd5ddff475c2f1b2b Mon Sep 17 00:00:00 2001 From: Vincent Dal Maso Date: Thu, 16 May 2019 11:27:45 +0200 Subject: [PATCH 1/7] Fix match_same_arms to fail late Changes: - Add a function search_same_list which return a list of matched expressions - Change the match_same_arms implementation behaviour. It will lint each same arms found. --- clippy_lints/src/copies.rs | 111 ++++++++++++++++++++++++------------- 1 file changed, 74 insertions(+), 37 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index d85cf97a6bdd..a28af2371edf 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -185,44 +185,48 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) { }; 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, - j.body.span, - "this `match` has identical arm bodies", - |db| { - db.span_note(i.body.span, "same as this"); - - // Note: this does not use `span_suggestion` on purpose: - // there is no clean way - // to remove the other arm. Building a span and suggest to replace it to "" - // makes an even more confusing error message. Also in order not to make up a - // span for the whole pattern, the suggestion is only shown when there is only - // one pattern. The user should know about `|` if they are already using it… - - if i.pats.len() == 1 && j.pats.len() == 1 { - let lhs = snippet(cx, i.pats[0].span, ""); - let rhs = snippet(cx, j.pats[0].span, ""); - - if let PatKind::Wild = j.pats[0].node { - // if the last arm is _, then i could be integrated into _ - // note that i.pats[0] cannot be _, because that would mean that we're - // hiding all the subsequent arms, and rust won't compile - db.span_note( - i.body.span, - &format!( - "`{}` has the same arm body as the `_` wildcard, consider removing it`", - lhs - ), - ); - } else { - db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs)); + search_same_list(&indexed_arms, hash, eq).map(|item| { + for match_expr in item { + let (&(_, i), &(_, j)) = match_expr; + + span_lint_and_then( + cx, + MATCH_SAME_ARMS, + j.body.span, + "this `match` has identical arm bodies", + |db| { + db.span_note(i.body.span, "same as this"); + + // Note: this does not use `span_suggestion` on purpose: + // there is no clean way + // to remove the other arm. Building a span and suggest to replace it to "" + // makes an even more confusing error message. Also in order not to make up a + // span for the whole pattern, the suggestion is only shown when there is only + // one pattern. The user should know about `|` if they are already using it… + + if i.pats.len() == 1 && j.pats.len() == 1 { + let lhs = snippet(cx, i.pats[0].span, ""); + let rhs = snippet(cx, j.pats[0].span, ""); + + if let PatKind::Wild = j.pats[0].node { + // if the last arm is _, then i could be integrated into _ + // note that i.pats[0] cannot be _, because that would mean that we're + // hiding all the subsequent arms, and rust won't compile + db.span_note( + i.body.span, + &format!( + "`{}` has the same arm body as the `_` wildcard, consider removing it`", + lhs + ), + ); + } else { + db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs)); + } } - } - }, - ); - } + }, + ); + } + }); } } @@ -360,3 +364,36 @@ where None } + +fn search_same_list(exprs: &[T], hash: Hash, eq: Eq) -> Option> +where + Hash: Fn(&T) -> u64, + Eq: Fn(&T, &T) -> bool, +{ + let mut match_expr_list: Vec<(&T, &T)> = Vec::new(); + + let mut map: FxHashMap<_, Vec<&_>> = + FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default()); + + for expr in exprs { + match map.entry(hash(expr)) { + Entry::Occupied(mut o) => { + for o in o.get() { + if eq(o, expr) { + match_expr_list.push((o, expr)); + } + } + o.get_mut().push(expr); + }, + Entry::Vacant(v) => { + v.insert(vec![expr]); + }, + } + } + + if match_expr_list.is_empty() { + None + } else { + Some(match_expr_list) + } +} From bfb230369e53a3959698a5e3ef1ff75d17ac4a41 Mon Sep 17 00:00:00 2001 From: Vincent Dal Maso Date: Thu, 16 May 2019 14:13:57 +0200 Subject: [PATCH 2/7] Add test for multiple same arms lints Changes: - Add a test to match multiple arms in the same match statement --- tests/ui/match_same_arms.rs | 8 ++++ tests/ui/match_same_arms.stderr | 76 ++++++++++++++++----------------- 2 files changed, 44 insertions(+), 40 deletions(-) diff --git a/tests/ui/match_same_arms.rs b/tests/ui/match_same_arms.rs index 8a7588f2e7f7..5c65ea0a10cd 100644 --- a/tests/ui/match_same_arms.rs +++ b/tests/ui/match_same_arms.rs @@ -108,6 +108,14 @@ fn match_same_arms() { (None, Some(a)) => bar(a), // bindings have different types _ => (), } + + let _ = match 42 { + 42 => 1, + 51 => 1, //~ ERROR match arms have same body + 41 => 2, + 52 => 2, //~ ERROR match arms have same body + _ => 0, + }; } fn main() {} diff --git a/tests/ui/match_same_arms.stderr b/tests/ui/match_same_arms.stderr index 9389e48a3e4b..47440af2485e 100644 --- a/tests/ui/match_same_arms.stderr +++ b/tests/ui/match_same_arms.stderr @@ -1,48 +1,10 @@ -error: this `match` has identical arm bodies - --> $DIR/match_same_arms.rs:37:14 - | -LL | _ => { - | ______________^ -LL | | //~ ERROR match arms have same body -LL | | foo(); -LL | | let mut a = 42 + [23].len() as i32; -... | -LL | | a -LL | | }, - | |_________^ - | - = note: `-D clippy::match-same-arms` implied by `-D warnings` -note: same as this - --> $DIR/match_same_arms.rs:28:15 - | -LL | 42 => { - | _______________^ -LL | | foo(); -LL | | let mut a = 42 + [23].len() as i32; -LL | | if true { -... | -LL | | a -LL | | }, - | |_________^ -note: `42` has the same arm body as the `_` wildcard, consider removing it` - --> $DIR/match_same_arms.rs:28:15 - | -LL | 42 => { - | _______________^ -LL | | foo(); -LL | | let mut a = 42 + [23].len() as i32; -LL | | if true { -... | -LL | | a -LL | | }, - | |_________^ - error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:52:14 | LL | _ => 0, //~ ERROR match arms have same body | ^ | + = note: `-D clippy::match-same-arms` implied by `-D warnings` note: same as this --> $DIR/match_same_arms.rs:50:19 | @@ -139,5 +101,39 @@ note: consider refactoring into `(1, .., 3) | (.., 3)` LL | (1, .., 3) => 42, | ^^ -error: aborting due to 7 previous errors +error: this `match` has identical arm bodies + --> $DIR/match_same_arms.rs:114:15 + | +LL | 51 => 1, //~ ERROR match arms have same body + | ^ + | +note: same as this + --> $DIR/match_same_arms.rs:113:15 + | +LL | 42 => 1, + | ^ +note: consider refactoring into `42 | 51` + --> $DIR/match_same_arms.rs:113:15 + | +LL | 42 => 1, + | ^ + +error: this `match` has identical arm bodies + --> $DIR/match_same_arms.rs:116:15 + | +LL | 52 => 2, //~ ERROR match arms have same body + | ^ + | +note: same as this + --> $DIR/match_same_arms.rs:115:15 + | +LL | 41 => 2, + | ^ +note: consider refactoring into `41 | 52` + --> $DIR/match_same_arms.rs:115:15 + | +LL | 41 => 2, + | ^ + +error: aborting due to 8 previous errors From 874de889dcc48f71bb2d5194f6d8110bcefe41d6 Mon Sep 17 00:00:00 2001 From: BO41 Date: Mon, 20 May 2019 15:08:53 +0200 Subject: [PATCH 3/7] Improve verbosity of non_ascii_literal lint example --- clippy_lints/src/unicode.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/unicode.rs b/clippy_lints/src/unicode.rs index d96596bbb525..fe7b1fbc6bfe 100644 --- a/clippy_lints/src/unicode.rs +++ b/clippy_lints/src/unicode.rs @@ -34,7 +34,11 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// let x = "Hä?" + /// let x = String::from("€"); + /// ``` + /// Could be written as: + /// ```rust + /// let x = String::from("\u{20ac}"); /// ``` pub NON_ASCII_LITERAL, pedantic, From 859b3296038d1f226a54a85f7464fe4baeb4bf72 Mon Sep 17 00:00:00 2001 From: BO41 Date: Mon, 20 May 2019 15:23:38 +0200 Subject: [PATCH 4/7] Make non_ascii_literal auto-fixable --- clippy_lints/src/unicode.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/unicode.rs b/clippy_lints/src/unicode.rs index fe7b1fbc6bfe..ceae97a1d2ea 100644 --- a/clippy_lints/src/unicode.rs +++ b/clippy_lints/src/unicode.rs @@ -1,7 +1,8 @@ -use crate::utils::{is_allowed, snippet, span_help_and_lint}; +use crate::utils::{is_allowed, snippet, span_help_and_lint, span_lint_and_sugg}; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; +use rustc_errors::Applicability; use syntax::ast::LitKind; use syntax::source_map::Span; use unicode_normalization::UnicodeNormalization; @@ -103,7 +104,7 @@ fn check_str(cx: &LateContext<'_, '_>, span: Span, id: HirId) { ); } if string.chars().any(|c| c as u32 > 0x7F) { - span_help_and_lint( + span_lint_and_sugg( cx, NON_ASCII_LITERAL, span, @@ -116,6 +117,15 @@ fn check_str(cx: &LateContext<'_, '_>, span: Span, id: HirId) { escape(string.nfc()) } ), + format!( + "{}", + if is_allowed(cx, UNICODE_NOT_NFC, id) { + escape(string.chars()) + } else { + escape(string.nfc()) + } + ), + Applicability::MachineApplicable, ); } if is_allowed(cx, NON_ASCII_LITERAL, id) && string.chars().zip(string.nfc()).any(|(a, b)| a != b) { From 36c8aaba8f9b525aaf37adec52592f4302cb8e12 Mon Sep 17 00:00:00 2001 From: BO41 Date: Mon, 20 May 2019 16:02:50 +0200 Subject: [PATCH 5/7] Fix tests and make other ascii lints auto-fixable --- clippy_lints/src/unicode.rs | 42 +++++++++++++------------------------ tests/ui/unicode.stderr | 6 ------ 2 files changed, 15 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/unicode.rs b/clippy_lints/src/unicode.rs index ceae97a1d2ea..15c74eff73b1 100644 --- a/clippy_lints/src/unicode.rs +++ b/clippy_lints/src/unicode.rs @@ -1,4 +1,4 @@ -use crate::utils::{is_allowed, snippet, span_help_and_lint, span_lint_and_sugg}; +use crate::utils::{is_allowed, snippet, span_lint_and_sugg}; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; @@ -92,15 +92,14 @@ fn escape>(s: T) -> String { fn check_str(cx: &LateContext<'_, '_>, span: Span, id: HirId) { let string = snippet(cx, span, ""); if string.contains('\u{200B}') { - span_help_and_lint( + span_lint_and_sugg( cx, ZERO_WIDTH_SPACE, span, "zero-width space detected", - &format!( - "Consider replacing the string with:\n\"{}\"", - string.replace("\u{200B}", "\\u{200B}") - ), + "consider replacing the string with", + string.replace("\u{200B}", "\\u{200B}"), + Applicability::MachineApplicable, ); } if string.chars().any(|c| c as u32 > 0x7F) { @@ -109,35 +108,24 @@ fn check_str(cx: &LateContext<'_, '_>, span: Span, id: HirId) { NON_ASCII_LITERAL, span, "literal non-ASCII character detected", - &format!( - "Consider replacing the string with:\n\"{}\"", - if is_allowed(cx, UNICODE_NOT_NFC, id) { - escape(string.chars()) - } else { - escape(string.nfc()) - } - ), - format!( - "{}", - if is_allowed(cx, UNICODE_NOT_NFC, id) { - escape(string.chars()) - } else { - escape(string.nfc()) - } - ), + "consider replacing the string with", + if is_allowed(cx, UNICODE_NOT_NFC, id) { + escape(string.chars()) + } else { + escape(string.nfc()) + }, Applicability::MachineApplicable, ); } if is_allowed(cx, NON_ASCII_LITERAL, id) && string.chars().zip(string.nfc()).any(|(a, b)| a != b) { - span_help_and_lint( + span_lint_and_sugg( cx, UNICODE_NOT_NFC, span, "non-nfc unicode sequence detected", - &format!( - "Consider replacing the string with:\n\"{}\"", - string.nfc().collect::() - ), + "consider replacing the string with", + string.nfc().collect::(), + Applicability::MachineApplicable, ); } } diff --git a/tests/ui/unicode.stderr b/tests/ui/unicode.stderr index c60dcdaec1d3..641680431a2c 100644 --- a/tests/ui/unicode.stderr +++ b/tests/ui/unicode.stderr @@ -5,8 +5,6 @@ LL | print!("Here >​< is a ZWS, and ​another"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::zero-width-space` implied by `-D warnings` - = help: Consider replacing the string with: - ""Here >/u{200B}< is a ZWS, and /u{200B}another"" error: non-nfc unicode sequence detected --> $DIR/unicode.rs:9:12 @@ -15,8 +13,6 @@ LL | print!("̀àh?"); | ^^^^^ | = note: `-D clippy::unicode-not-nfc` implied by `-D warnings` - = help: Consider replacing the string with: - ""̀àh?"" error: literal non-ASCII character detected --> $DIR/unicode.rs:15:12 @@ -25,8 +21,6 @@ LL | print!("Üben!"); | ^^^^^^^ | = note: `-D clippy::non-ascii-literal` implied by `-D warnings` - = help: Consider replacing the string with: - ""/u{dc}ben!"" error: aborting due to 3 previous errors From fa9f744c2cdb725fb36dbf1c8810e6600b845acd Mon Sep 17 00:00:00 2001 From: Vincent Dal Maso Date: Mon, 20 May 2019 10:22:13 +0200 Subject: [PATCH 6/7] Add the common case search Changes: - Refactor the common case search into a function. - Fix the useless Option around the vec in the search_same_list. --- clippy_lints/src/copies.rs | 126 ++++++++++++----------------- tests/ui/match_same_arms.rs | 8 ++ tests/ui/match_same_arms.stderr | 135 ++++++++++++++++++++++++++------ 3 files changed, 172 insertions(+), 97 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index a28af2371edf..c1e06be7d6bc 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -152,7 +152,7 @@ fn lint_same_cond(cx: &LateContext<'_, '_>, conds: &[&Expr]) { let eq: &dyn Fn(&&Expr, &&Expr) -> bool = &|&lhs, &rhs| -> bool { SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, rhs) }; - if let Some((i, j)) = search_same(conds, hash, eq) { + for (i, j) in search_same(conds, hash, eq) { span_note_and_lint( cx, IFS_SAME_COND, @@ -185,48 +185,47 @@ fn lint_match_arms(cx: &LateContext<'_, '_>, expr: &Expr) { }; let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect(); - search_same_list(&indexed_arms, hash, eq).map(|item| { - for match_expr in item { - let (&(_, i), &(_, j)) = match_expr; - - span_lint_and_then( - cx, - MATCH_SAME_ARMS, - j.body.span, - "this `match` has identical arm bodies", - |db| { - db.span_note(i.body.span, "same as this"); - - // Note: this does not use `span_suggestion` on purpose: - // there is no clean way - // to remove the other arm. Building a span and suggest to replace it to "" - // makes an even more confusing error message. Also in order not to make up a - // span for the whole pattern, the suggestion is only shown when there is only - // one pattern. The user should know about `|` if they are already using it… - - if i.pats.len() == 1 && j.pats.len() == 1 { - let lhs = snippet(cx, i.pats[0].span, ""); - let rhs = snippet(cx, j.pats[0].span, ""); - - if let PatKind::Wild = j.pats[0].node { - // if the last arm is _, then i could be integrated into _ - // note that i.pats[0] cannot be _, because that would mean that we're - // hiding all the subsequent arms, and rust won't compile - db.span_note( - i.body.span, - &format!( - "`{}` has the same arm body as the `_` wildcard, consider removing it`", - lhs - ), - ); - } else { - db.span_note(i.body.span, &format!("consider refactoring into `{} | {}`", lhs, rhs)); - } + for (&(_, i), &(_, j)) in search_same(&indexed_arms, hash, eq) { + span_lint_and_then( + cx, + MATCH_SAME_ARMS, + j.body.span, + "this `match` has identical arm bodies", + |db| { + db.span_note(i.body.span, "same as this"); + + // Note: this does not use `span_suggestion` on purpose: + // there is no clean way + // to remove the other arm. Building a span and suggest to replace it to "" + // makes an even more confusing error message. Also in order not to make up a + // span for the whole pattern, the suggestion is only shown when there is only + // one pattern. The user should know about `|` if they are already using it… + + if i.pats.len() == 1 && j.pats.len() == 1 { + let lhs = snippet(cx, i.pats[0].span, ""); + let rhs = snippet(cx, j.pats[0].span, ""); + + if let PatKind::Wild = j.pats[0].node { + // if the last arm is _, then i could be integrated into _ + // note that i.pats[0] cannot be _, because that would mean that we're + // hiding all the subsequent arms, and rust won't compile + db.span_note( + i.body.span, + &format!( + "`{}` has the same arm body as the `_` wildcard, consider removing it`", + lhs + ), + ); + } else { + db.span_help( + i.pats[0].span, + &format!("consider refactoring into `{} | {}`", lhs, rhs), + ); } - }, - ); - } - }); + } + }, + ); + } } } @@ -327,49 +326,32 @@ where None } -fn search_same(exprs: &[T], hash: Hash, eq: Eq) -> Option<(&T, &T)> +fn search_common_cases<'a, T, Eq>(exprs: &'a [T], eq: &Eq) -> Option<(&'a T, &'a T)> where - Hash: Fn(&T) -> u64, Eq: Fn(&T, &T) -> bool, { - // common cases if exprs.len() < 2 { - return None; + None } else if exprs.len() == 2 { - return if eq(&exprs[0], &exprs[1]) { + if eq(&exprs[0], &exprs[1]) { Some((&exprs[0], &exprs[1])) } else { None - }; - } - - let mut map: FxHashMap<_, Vec<&_>> = - FxHashMap::with_capacity_and_hasher(exprs.len(), BuildHasherDefault::default()); - - for expr in exprs { - match map.entry(hash(expr)) { - Entry::Occupied(mut o) => { - for o in o.get() { - if eq(o, expr) { - return Some((o, expr)); - } - } - o.get_mut().push(expr); - }, - Entry::Vacant(v) => { - v.insert(vec![expr]); - }, } + } else { + None } - - None } -fn search_same_list(exprs: &[T], hash: Hash, eq: Eq) -> Option> +fn search_same(exprs: &[T], hash: Hash, eq: Eq) -> Vec<(&T, &T)> where Hash: Fn(&T) -> u64, Eq: Fn(&T, &T) -> bool, { + if let Some(expr) = search_common_cases(&exprs, &eq) { + return vec![expr]; + } + let mut match_expr_list: Vec<(&T, &T)> = Vec::new(); let mut map: FxHashMap<_, Vec<&_>> = @@ -391,9 +373,5 @@ where } } - if match_expr_list.is_empty() { - None - } else { - Some(match_expr_list) - } + match_expr_list } diff --git a/tests/ui/match_same_arms.rs b/tests/ui/match_same_arms.rs index 5c65ea0a10cd..baba4367fb40 100644 --- a/tests/ui/match_same_arms.rs +++ b/tests/ui/match_same_arms.rs @@ -116,6 +116,14 @@ fn match_same_arms() { 52 => 2, //~ ERROR match arms have same body _ => 0, }; + + let _ = match 42 { + 1 => 2, + 2 => 2, //~ ERROR 2rd matched arms have same body + 3 => 2, //~ ERROR 3rd matched arms have same body + 4 => 3, + _ => 0, + }; } fn main() {} diff --git a/tests/ui/match_same_arms.stderr b/tests/ui/match_same_arms.stderr index 47440af2485e..d7545fed586c 100644 --- a/tests/ui/match_same_arms.stderr +++ b/tests/ui/match_same_arms.stderr @@ -1,10 +1,48 @@ +error: this `match` has identical arm bodies + --> $DIR/match_same_arms.rs:37:14 + | +LL | _ => { + | ______________^ +LL | | //~ ERROR match arms have same body +LL | | foo(); +LL | | let mut a = 42 + [23].len() as i32; +... | +LL | | a +LL | | }, + | |_________^ + | + = note: `-D clippy::match-same-arms` implied by `-D warnings` +note: same as this + --> $DIR/match_same_arms.rs:28:15 + | +LL | 42 => { + | _______________^ +LL | | foo(); +LL | | let mut a = 42 + [23].len() as i32; +LL | | if true { +... | +LL | | a +LL | | }, + | |_________^ +note: `42` has the same arm body as the `_` wildcard, consider removing it` + --> $DIR/match_same_arms.rs:28:15 + | +LL | 42 => { + | _______________^ +LL | | foo(); +LL | | let mut a = 42 + [23].len() as i32; +LL | | if true { +... | +LL | | a +LL | | }, + | |_________^ + error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:52:14 | LL | _ => 0, //~ ERROR match arms have same body | ^ | - = note: `-D clippy::match-same-arms` implied by `-D warnings` note: same as this --> $DIR/match_same_arms.rs:50:19 | @@ -27,11 +65,11 @@ note: same as this | LL | 42 => foo(), | ^^^^^ -note: consider refactoring into `42 | 51` - --> $DIR/match_same_arms.rs:56:15 +help: consider refactoring into `42 | 51` + --> $DIR/match_same_arms.rs:56:9 | LL | 42 => foo(), - | ^^^^^ + | ^^ error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:63:17 @@ -44,11 +82,11 @@ note: same as this | LL | Some(_) => 24, | ^^ -note: consider refactoring into `Some(_) | None` - --> $DIR/match_same_arms.rs:62:20 +help: consider refactoring into `Some(_) | None` + --> $DIR/match_same_arms.rs:62:9 | LL | Some(_) => 24, - | ^^ + | ^^^^^^^ error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:85:28 @@ -61,11 +99,11 @@ note: same as this | LL | (Some(a), None) => bar(a), | ^^^^^^ -note: consider refactoring into `(Some(a), None) | (None, Some(a))` - --> $DIR/match_same_arms.rs:84:28 +help: consider refactoring into `(Some(a), None) | (None, Some(a))` + --> $DIR/match_same_arms.rs:84:9 | LL | (Some(a), None) => bar(a), - | ^^^^^^ + | ^^^^^^^^^^^^^^^ error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:91:26 @@ -78,11 +116,11 @@ note: same as this | LL | (Some(a), ..) => bar(a), | ^^^^^^ -note: consider refactoring into `(Some(a), ..) | (.., Some(a))` - --> $DIR/match_same_arms.rs:90:26 +help: consider refactoring into `(Some(a), ..) | (.., Some(a))` + --> $DIR/match_same_arms.rs:90:9 | LL | (Some(a), ..) => bar(a), - | ^^^^^^ + | ^^^^^^^^^^^^^ error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:97:20 @@ -95,11 +133,11 @@ note: same as this | LL | (1, .., 3) => 42, | ^^ -note: consider refactoring into `(1, .., 3) | (.., 3)` - --> $DIR/match_same_arms.rs:96:23 +help: consider refactoring into `(1, .., 3) | (.., 3)` + --> $DIR/match_same_arms.rs:96:9 | LL | (1, .., 3) => 42, - | ^^ + | ^^^^^^^^^^ error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:114:15 @@ -112,11 +150,11 @@ note: same as this | LL | 42 => 1, | ^ -note: consider refactoring into `42 | 51` - --> $DIR/match_same_arms.rs:113:15 +help: consider refactoring into `42 | 51` + --> $DIR/match_same_arms.rs:113:9 | LL | 42 => 1, - | ^ + | ^^ error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:116:15 @@ -129,11 +167,62 @@ note: same as this | LL | 41 => 2, | ^ -note: consider refactoring into `41 | 52` - --> $DIR/match_same_arms.rs:115:15 +help: consider refactoring into `41 | 52` + --> $DIR/match_same_arms.rs:115:9 | LL | 41 => 2, - | ^ + | ^^ + +error: this `match` has identical arm bodies + --> $DIR/match_same_arms.rs:122:14 + | +LL | 2 => 2, //~ ERROR 2rd matched arms have same body + | ^ + | +note: same as this + --> $DIR/match_same_arms.rs:121:14 + | +LL | 1 => 2, + | ^ +help: consider refactoring into `1 | 2` + --> $DIR/match_same_arms.rs:121:9 + | +LL | 1 => 2, + | ^ + +error: this `match` has identical arm bodies + --> $DIR/match_same_arms.rs:123:14 + | +LL | 3 => 2, //~ ERROR 3rd matched arms have same body + | ^ + | +note: same as this + --> $DIR/match_same_arms.rs:121:14 + | +LL | 1 => 2, + | ^ +help: consider refactoring into `1 | 3` + --> $DIR/match_same_arms.rs:121:9 + | +LL | 1 => 2, + | ^ + +error: this `match` has identical arm bodies + --> $DIR/match_same_arms.rs:123:14 + | +LL | 3 => 2, //~ ERROR 3rd matched arms have same body + | ^ + | +note: same as this + --> $DIR/match_same_arms.rs:122:14 + | +LL | 2 => 2, //~ ERROR 2rd matched arms have same body + | ^ +help: consider refactoring into `2 | 3` + --> $DIR/match_same_arms.rs:122:9 + | +LL | 2 => 2, //~ ERROR 2rd matched arms have same body + | ^ -error: aborting due to 8 previous errors +error: aborting due to 12 previous errors From 40f36658f544bd83be4430ec0126f381db57bb14 Mon Sep 17 00:00:00 2001 From: Vincent Dal Maso Date: Mon, 20 May 2019 18:25:13 +0200 Subject: [PATCH 7/7] Fix breaking tests Changes: - Fix stderr breaking the tests - Adding tests over the if arms --- tests/ui/if_same_then_else.rs | 14 ++++++++ tests/ui/if_same_then_else.stderr | 35 +++++++++++++++++++- tests/ui/match_same_arms.rs | 2 +- tests/ui/match_same_arms.stderr | 6 ++-- tests/ui/matches.stderr | 54 +++++++++++++++---------------- 5 files changed, 79 insertions(+), 32 deletions(-) diff --git a/tests/ui/if_same_then_else.rs b/tests/ui/if_same_then_else.rs index 0ec933e87847..d1213e5e5fda 100644 --- a/tests/ui/if_same_then_else.rs +++ b/tests/ui/if_same_then_else.rs @@ -232,6 +232,20 @@ fn if_same_then_else() -> Result<&'static str, ()> { return Ok(&foo[0..]); } + if true { + let foo = ""; + return Ok(&foo[0..]); + } else if false { + let foo = "bar"; + return Ok(&foo[0..]); + } else if true { + let foo = ""; + return Ok(&foo[0..]); + } else { + let foo = ""; + return Ok(&foo[0..]); + } + // False positive `if_same_then_else`: `let (x, y)` vs. `let (y, x)`; see issue #3559. if true { let foo = ""; diff --git a/tests/ui/if_same_then_else.stderr b/tests/ui/if_same_then_else.stderr index b170db31b853..fa42afff0be0 100644 --- a/tests/ui/if_same_then_else.stderr +++ b/tests/ui/if_same_then_else.stderr @@ -210,5 +210,38 @@ LL | | try!(Ok("foo")); LL | | } else { | |_____^ -error: aborting due to 10 previous errors +error: this `if` has identical blocks + --> $DIR/if_same_then_else.rs:244:12 + | +LL | } else { + | ____________^ +LL | | let foo = ""; +LL | | return Ok(&foo[0..]); +LL | | } + | |_____^ + | +note: same as this + --> $DIR/if_same_then_else.rs:241:20 + | +LL | } else if true { + | ____________________^ +LL | | let foo = ""; +LL | | return Ok(&foo[0..]); +LL | | } else { + | |_____^ + +error: this `if` has the same condition as a previous if + --> $DIR/if_same_then_else.rs:241:15 + | +LL | } else if true { + | ^^^^ + | + = note: #[deny(clippy::ifs_same_cond)] on by default +note: same as this + --> $DIR/if_same_then_else.rs:235:8 + | +LL | if true { + | ^^^^ + +error: aborting due to 12 previous errors diff --git a/tests/ui/match_same_arms.rs b/tests/ui/match_same_arms.rs index baba4367fb40..880518b010a7 100644 --- a/tests/ui/match_same_arms.rs +++ b/tests/ui/match_same_arms.rs @@ -119,7 +119,7 @@ fn match_same_arms() { let _ = match 42 { 1 => 2, - 2 => 2, //~ ERROR 2rd matched arms have same body + 2 => 2, //~ ERROR 2nd matched arms have same body 3 => 2, //~ ERROR 3rd matched arms have same body 4 => 3, _ => 0, diff --git a/tests/ui/match_same_arms.stderr b/tests/ui/match_same_arms.stderr index d7545fed586c..42c6f910dc30 100644 --- a/tests/ui/match_same_arms.stderr +++ b/tests/ui/match_same_arms.stderr @@ -176,7 +176,7 @@ LL | 41 => 2, error: this `match` has identical arm bodies --> $DIR/match_same_arms.rs:122:14 | -LL | 2 => 2, //~ ERROR 2rd matched arms have same body +LL | 2 => 2, //~ ERROR 2nd matched arms have same body | ^ | note: same as this @@ -216,12 +216,12 @@ LL | 3 => 2, //~ ERROR 3rd matched arms have same body note: same as this --> $DIR/match_same_arms.rs:122:14 | -LL | 2 => 2, //~ ERROR 2rd matched arms have same body +LL | 2 => 2, //~ ERROR 2nd matched arms have same body | ^ help: consider refactoring into `2 | 3` --> $DIR/match_same_arms.rs:122:9 | -LL | 2 => 2, //~ ERROR 2rd matched arms have same body +LL | 2 => 2, //~ ERROR 2nd matched arms have same body | ^ error: aborting due to 12 previous errors diff --git a/tests/ui/matches.stderr b/tests/ui/matches.stderr index 27438258d232..232ae28009b6 100644 --- a/tests/ui/matches.stderr +++ b/tests/ui/matches.stderr @@ -89,11 +89,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:53:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:53:9 | LL | 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: Err(_) will match all errors, maybe not a good idea @@ -115,11 +115,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:59:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:59:9 | LL | 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: Err(_) will match all errors, maybe not a good idea @@ -141,11 +141,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:65:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:65:9 | LL | 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 @@ -159,11 +159,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:74:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:74:9 | LL | 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 @@ -177,11 +177,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:81:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:81:9 | LL | 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 @@ -195,11 +195,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:87:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:87:9 | LL | 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 @@ -213,11 +213,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:93:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:93:9 | LL | 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 @@ -231,11 +231,11 @@ note: same as this | LL | (Ok(x), Some(_)) => println!("ok {}", x), | ^^^^^^^^^^^^^^^^^^^^ -note: consider refactoring into `(Ok(x), Some(_)) | (Ok(_), Some(x))` - --> $DIR/matches.rs:116:29 +help: consider refactoring into `(Ok(x), Some(_)) | (Ok(_), Some(x))` + --> $DIR/matches.rs:116:9 | LL | (Ok(x), Some(_)) => println!("ok {}", x), - | ^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ = 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 @@ -249,11 +249,11 @@ note: same as this | LL | Ok(3) => println!("ok"), | ^^^^^^^^^^^^^^ -note: consider refactoring into `Ok(3) | Ok(_)` - --> $DIR/matches.rs:131:18 +help: consider refactoring into `Ok(3) | Ok(_)` + --> $DIR/matches.rs:131:9 | LL | 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: you don't need to add `&` to all patterns