From 96c41988323cd4c3d211e494fd3e143caf11f8cd Mon Sep 17 00:00:00 2001 From: ThibsG Date: Tue, 24 Dec 2019 16:42:09 +0100 Subject: [PATCH 1/7] New lint: pats_with_wild_match_arm - Wildcard use with other pattern in same match arm --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 2 ++ clippy_lints/src/matches.rs | 43 +++++++++++++++++++++++- src/lintlist/mod.rs | 7 ++++ tests/ui/pats_with_wild_match_arm.fixed | 38 +++++++++++++++++++++ tests/ui/pats_with_wild_match_arm.rs | 38 +++++++++++++++++++++ tests/ui/pats_with_wild_match_arm.stderr | 28 +++++++++++++++ 7 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 tests/ui/pats_with_wild_match_arm.fixed create mode 100644 tests/ui/pats_with_wild_match_arm.rs create mode 100644 tests/ui/pats_with_wild_match_arm.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index f60ac7d5267b..d4ecdabeb14f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1241,6 +1241,7 @@ Released 2018-09-13 [`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite +[`pats_with_wild_match_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#pats_with_wild_match_arm [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence [`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6846730a4156..2d83a5b41d0f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -597,6 +597,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf &matches::MATCH_OVERLAPPING_ARM, &matches::MATCH_REF_PATS, &matches::MATCH_WILD_ERR_ARM, + &matches::PATS_WITH_WILD_MATCH_ARM, &matches::SINGLE_MATCH, &matches::SINGLE_MATCH_ELSE, &matches::WILDCARD_ENUM_MATCH_ARM, @@ -1001,6 +1002,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&integer_division::INTEGER_DIVISION), LintId::of(&let_underscore::LET_UNDERSCORE_MUST_USE), LintId::of(&literal_representation::DECIMAL_LITERAL_REPRESENTATION), + LintId::of(&matches::PATS_WITH_WILD_MATCH_ARM), LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM), LintId::of(&mem_forget::MEM_FORGET), LintId::of(&methods::CLONE_ON_REF_PTR), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 3200de1cfc17..29d34c379880 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -223,6 +223,26 @@ declare_clippy_lint! { "a wildcard enum match arm using `_`" } +declare_clippy_lint! { + /// **What it does:** Checks for wildcard pattern used with others patterns in same match arm. + /// + /// **Why is this bad?** Wildcard pattern already covers any other pattern as it will match anyway. + /// It makes the code less readable, especially to spot wildcard pattern use in match arm. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// match "foo" { + /// "a" => {}, + /// "bar" | _ => {}, + /// } + /// ``` + pub PATS_WITH_WILD_MATCH_ARM, + restriction, + "a wildcard pattern used with others patterns in same match arm" +} + declare_lint_pass!(Matches => [ SINGLE_MATCH, MATCH_REF_PATS, @@ -231,7 +251,8 @@ declare_lint_pass!(Matches => [ MATCH_OVERLAPPING_ARM, MATCH_WILD_ERR_ARM, MATCH_AS_REF, - WILDCARD_ENUM_MATCH_ARM + WILDCARD_ENUM_MATCH_ARM, + PATS_WITH_WILD_MATCH_ARM ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { @@ -246,6 +267,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { check_wild_err_arm(cx, ex, arms); check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); + check_pats_wild_match(cx, ex, arms, expr); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -664,6 +686,25 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], } } +fn check_pats_wild_match(cx: &LateContext<'_, '_>, _ex: &Expr, arms: &[Arm], _expr: &Expr) { + for arm in arms { + if let PatKind::Or(ref fields) = arm.pat.kind { + // look for multiple fields where one at least matches Wild pattern + if fields.len() > 1 && fields.into_iter().any(|pat| is_wild(pat)) { + span_lint_and_sugg( + cx, + PATS_WITH_WILD_MATCH_ARM, + arm.pat.span, + "wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only", + "try this", + "_".to_string(), + Applicability::MachineApplicable, + ) + } + } + } +} + /// Gets all arms that are unbounded `PatRange`s. fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm<'_>]) -> Vec> { arms.iter() diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 1086f5e48f9a..a6b31876a7aa 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1568,6 +1568,13 @@ pub const ALL_LINTS: [Lint; 345] = [ deprecation: None, module: "path_buf_push_overwrite", }, + Lint { + name: "pats_with_wild_match_arm", + group: "restriction", + desc: "a wildcard pattern used with others patterns in same match arm", + deprecation: None, + module: "matches", + }, Lint { name: "possible_missing_comma", group: "correctness", diff --git a/tests/ui/pats_with_wild_match_arm.fixed b/tests/ui/pats_with_wild_match_arm.fixed new file mode 100644 index 000000000000..daa34af94363 --- /dev/null +++ b/tests/ui/pats_with_wild_match_arm.fixed @@ -0,0 +1,38 @@ +// run-rustfix + +#![warn(clippy::pats_with_wild_match_arm)] + +fn main() { + match "foo" { + "a" => { + dbg!("matched a"); + }, + _ => { + dbg!("matched (bar or) wild"); + }, + }; + match "foo" { + "a" => { + dbg!("matched a"); + }, + _ => { + dbg!("matched (bar or bar2 or) wild"); + }, + }; + match "foo" { + "a" => { + dbg!("matched a"); + }, + _ => { + dbg!("matched (bar or) wild"); + }, + }; + match "foo" { + "a" => { + dbg!("matched a"); + }, + _ => { + dbg!("matched (bar or) wild"); + }, + }; +} diff --git a/tests/ui/pats_with_wild_match_arm.rs b/tests/ui/pats_with_wild_match_arm.rs new file mode 100644 index 000000000000..0410cecb5d67 --- /dev/null +++ b/tests/ui/pats_with_wild_match_arm.rs @@ -0,0 +1,38 @@ +// run-rustfix + +#![warn(clippy::pats_with_wild_match_arm)] + +fn main() { + match "foo" { + "a" => { + dbg!("matched a"); + }, + "bar" | _ => { + dbg!("matched (bar or) wild"); + }, + }; + match "foo" { + "a" => { + dbg!("matched a"); + }, + "bar" | "bar2" | _ => { + dbg!("matched (bar or bar2 or) wild"); + }, + }; + match "foo" { + "a" => { + dbg!("matched a"); + }, + _ | "bar" | _ => { + dbg!("matched (bar or) wild"); + }, + }; + match "foo" { + "a" => { + dbg!("matched a"); + }, + _ | "bar" => { + dbg!("matched (bar or) wild"); + }, + }; +} diff --git a/tests/ui/pats_with_wild_match_arm.stderr b/tests/ui/pats_with_wild_match_arm.stderr new file mode 100644 index 000000000000..5b5d8d28738a --- /dev/null +++ b/tests/ui/pats_with_wild_match_arm.stderr @@ -0,0 +1,28 @@ +error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only + --> $DIR/pats_with_wild_match_arm.rs:10:9 + | +LL | "bar" | _ => { + | ^^^^^^^^^ help: try this: `_` + | + = note: `-D clippy::pats-with-wild-match-arm` implied by `-D warnings` + +error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only + --> $DIR/pats_with_wild_match_arm.rs:18:9 + | +LL | "bar" | "bar2" | _ => { + | ^^^^^^^^^^^^^^^^^^ help: try this: `_` + +error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only + --> $DIR/pats_with_wild_match_arm.rs:26:9 + | +LL | _ | "bar" | _ => { + | ^^^^^^^^^^^^^ help: try this: `_` + +error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only + --> $DIR/pats_with_wild_match_arm.rs:34:9 + | +LL | _ | "bar" => { + | ^^^^^^^^^ help: try this: `_` + +error: aborting due to 4 previous errors + From 8ec32175fa8825b8cc547038776fe7b36b8ad90d Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 27 Dec 2019 01:56:09 +0100 Subject: [PATCH 2/7] Remove useless parameters in func call --- clippy_lints/src/matches.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 29d34c379880..108f453a9bcf 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -267,7 +267,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { check_wild_err_arm(cx, ex, arms); check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); - check_pats_wild_match(cx, ex, arms, expr); + check_pats_wild_match(cx, arms); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -686,7 +686,7 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], } } -fn check_pats_wild_match(cx: &LateContext<'_, '_>, _ex: &Expr, arms: &[Arm], _expr: &Expr) { +fn check_pats_wild_match(cx: &LateContext<'_, '_>, arms: &[Arm]) { for arm in arms { if let PatKind::Or(ref fields) = arm.pat.kind { // look for multiple fields where one at least matches Wild pattern From 649af71f9e516dd1f316111b3eaccb824f5a4981 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 27 Dec 2019 02:03:16 +0100 Subject: [PATCH 3/7] Change group and use only func call --- clippy_lints/src/matches.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 108f453a9bcf..e7d9b9413f1e 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -239,7 +239,7 @@ declare_clippy_lint! { /// } /// ``` pub PATS_WITH_WILD_MATCH_ARM, - restriction, + complexity, "a wildcard pattern used with others patterns in same match arm" } @@ -690,7 +690,7 @@ fn check_pats_wild_match(cx: &LateContext<'_, '_>, arms: &[Arm]) { for arm in arms { if let PatKind::Or(ref fields) = arm.pat.kind { // look for multiple fields where one at least matches Wild pattern - if fields.len() > 1 && fields.into_iter().any(|pat| is_wild(pat)) { + if fields.len() > 1 && fields.into_iter().any(is_wild) { span_lint_and_sugg( cx, PATS_WITH_WILD_MATCH_ARM, From d60c6f939862ed0be315499f53a0c2fe63b580d6 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Fri, 27 Dec 2019 05:42:28 +0100 Subject: [PATCH 4/7] Move to complexity and adapt test - test wildcard_enum_match_arm has been impacted by this new lint --- clippy_lints/src/lib.rs | 3 ++- src/lintlist/mod.rs | 2 +- tests/ui/wildcard_enum_match_arm.fixed | 8 +++++++- tests/ui/wildcard_enum_match_arm.rs | 8 +++++++- tests/ui/wildcard_enum_match_arm.stderr | 10 +++++----- 5 files changed, 22 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2d83a5b41d0f..a009bab57c67 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1002,7 +1002,6 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&integer_division::INTEGER_DIVISION), LintId::of(&let_underscore::LET_UNDERSCORE_MUST_USE), LintId::of(&literal_representation::DECIMAL_LITERAL_REPRESENTATION), - LintId::of(&matches::PATS_WITH_WILD_MATCH_ARM), LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM), LintId::of(&mem_forget::MEM_FORGET), LintId::of(&methods::CLONE_ON_REF_PTR), @@ -1189,6 +1188,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_REF_PATS), LintId::of(&matches::MATCH_WILD_ERR_ARM), + LintId::of(&matches::PATS_WITH_WILD_MATCH_ARM), LintId::of(&matches::SINGLE_MATCH), LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM), LintId::of(&mem_replace::MEM_REPLACE_OPTION_WITH_NONE), @@ -1463,6 +1463,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(&matches::MATCH_AS_REF), + LintId::of(&matches::PATS_WITH_WILD_MATCH_ARM), LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::CLONE_ON_COPY), LintId::of(&methods::FILTER_NEXT), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index a6b31876a7aa..34bf4749d83f 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1570,7 +1570,7 @@ pub const ALL_LINTS: [Lint; 345] = [ }, Lint { name: "pats_with_wild_match_arm", - group: "restriction", + group: "complexity", desc: "a wildcard pattern used with others patterns in same match arm", deprecation: None, module: "matches", diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 1da2833e2605..f46320e03cc5 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -1,7 +1,13 @@ // run-rustfix #![deny(clippy::wildcard_enum_match_arm)] -#![allow(unreachable_code, unused_variables, dead_code, clippy::single_match)] +#![allow( + unreachable_code, + unused_variables, + dead_code, + clippy::single_match, + clippy::pats_with_wild_match_arm +)] use std::io::ErrorKind; diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index c2eb4b308024..508d0e0bb796 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -1,7 +1,13 @@ // run-rustfix #![deny(clippy::wildcard_enum_match_arm)] -#![allow(unreachable_code, unused_variables, dead_code, clippy::single_match)] +#![allow( + unreachable_code, + unused_variables, + dead_code, + clippy::single_match, + clippy::pats_with_wild_match_arm +)] use std::io::ErrorKind; diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 735f610b7e5d..e6f0411095ca 100644 --- a/tests/ui/wildcard_enum_match_arm.stderr +++ b/tests/ui/wildcard_enum_match_arm.stderr @@ -1,5 +1,5 @@ error: wildcard match will miss any future added variants - --> $DIR/wildcard_enum_match_arm.rs:31:9 + --> $DIR/wildcard_enum_match_arm.rs:37:9 | LL | _ => eprintln!("Not red"), | ^ help: try this: `Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` @@ -11,25 +11,25 @@ LL | #![deny(clippy::wildcard_enum_match_arm)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: wildcard match will miss any future added variants - --> $DIR/wildcard_enum_match_arm.rs:35:9 + --> $DIR/wildcard_enum_match_arm.rs:41:9 | LL | _not_red => eprintln!("Not red"), | ^^^^^^^^ help: try this: `_not_red @ Color::Green | _not_red @ Color::Blue | _not_red @ Color::Rgb(..) | _not_red @ Color::Cyan` error: wildcard match will miss any future added variants - --> $DIR/wildcard_enum_match_arm.rs:39:9 + --> $DIR/wildcard_enum_match_arm.rs:45:9 | LL | not_red => format!("{:?}", not_red), | ^^^^^^^ help: try this: `not_red @ Color::Green | not_red @ Color::Blue | not_red @ Color::Rgb(..) | not_red @ Color::Cyan` error: wildcard match will miss any future added variants - --> $DIR/wildcard_enum_match_arm.rs:55:9 + --> $DIR/wildcard_enum_match_arm.rs:61:9 | LL | _ => "No red", | ^ help: try this: `Color::Red | Color::Green | Color::Blue | Color::Rgb(..) | Color::Cyan` error: match on non-exhaustive enum doesn't explicitly match all known variants - --> $DIR/wildcard_enum_match_arm.rs:72:9 + --> $DIR/wildcard_enum_match_arm.rs:78:9 | LL | _ => {}, | ^ help: try this: `std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::ConnectionRefused | std::io::ErrorKind::ConnectionReset | std::io::ErrorKind::ConnectionAborted | std::io::ErrorKind::NotConnected | std::io::ErrorKind::AddrInUse | std::io::ErrorKind::AddrNotAvailable | std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::AlreadyExists | std::io::ErrorKind::WouldBlock | std::io::ErrorKind::InvalidInput | std::io::ErrorKind::InvalidData | std::io::ErrorKind::TimedOut | std::io::ErrorKind::WriteZero | std::io::ErrorKind::Interrupted | std::io::ErrorKind::Other | std::io::ErrorKind::UnexpectedEof | _` From 58deaad42d2986154303647dbc005133aaf3ab92 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Thu, 2 Jan 2020 20:00:27 +0100 Subject: [PATCH 5/7] Handle case for non-exhaustive enums --- clippy_lints/src/matches.rs | 45 ++++++++++++++++++------ tests/ui/pats_with_wild_match_arm.stderr | 16 ++++----- 2 files changed, 43 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index e7d9b9413f1e..2347a0619d65 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -267,7 +267,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { check_wild_err_arm(cx, ex, arms); check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); - check_pats_wild_match(cx, arms); + check_pats_wild_match(cx, ex, arms); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -686,20 +686,45 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], } } -fn check_pats_wild_match(cx: &LateContext<'_, '_>, arms: &[Arm]) { +fn check_pats_wild_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { + let mut is_non_exhaustive_enum = false; + let ty = cx.tables.expr_ty(ex); + if ty.is_enum() { + if let ty::Adt(def, _) = ty.kind { + if def.is_variant_list_non_exhaustive() { + is_non_exhaustive_enum = true; + } + } + } + for arm in arms { if let PatKind::Or(ref fields) = arm.pat.kind { - // look for multiple fields where one at least matches Wild pattern - if fields.len() > 1 && fields.into_iter().any(is_wild) { - span_lint_and_sugg( + // look for multiple fields in this arm that contains at least one Wild pattern + if fields.len() > 1 && fields.iter().any(is_wild) { + span_lint_and_then( cx, PATS_WITH_WILD_MATCH_ARM, arm.pat.span, - "wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only", - "try this", - "_".to_string(), - Applicability::MachineApplicable, - ) + "wildcard pattern covers any other pattern as it will match anyway.", + |db| { + // handle case where a non exhaustive enum is being used + if is_non_exhaustive_enum { + db.span_suggestion( + arm.pat.span, + "consider handling `_` separately.", + "_ => ...".to_string(), + Applicability::MaybeIncorrect, + ); + } else { + db.span_suggestion( + arm.pat.span, + "consider replacing with wildcard pattern only", + "_".to_string(), + Applicability::MachineApplicable, + ); + } + }, + ); } } } diff --git a/tests/ui/pats_with_wild_match_arm.stderr b/tests/ui/pats_with_wild_match_arm.stderr index 5b5d8d28738a..215d0b2e4134 100644 --- a/tests/ui/pats_with_wild_match_arm.stderr +++ b/tests/ui/pats_with_wild_match_arm.stderr @@ -1,28 +1,28 @@ -error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only +error: wildcard pattern covers any other pattern as it will match anyway. --> $DIR/pats_with_wild_match_arm.rs:10:9 | LL | "bar" | _ => { - | ^^^^^^^^^ help: try this: `_` + | ^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` | = note: `-D clippy::pats-with-wild-match-arm` implied by `-D warnings` -error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only +error: wildcard pattern covers any other pattern as it will match anyway. --> $DIR/pats_with_wild_match_arm.rs:18:9 | LL | "bar" | "bar2" | _ => { - | ^^^^^^^^^^^^^^^^^^ help: try this: `_` + | ^^^^^^^^^^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` -error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only +error: wildcard pattern covers any other pattern as it will match anyway. --> $DIR/pats_with_wild_match_arm.rs:26:9 | LL | _ | "bar" | _ => { - | ^^^^^^^^^^^^^ help: try this: `_` + | ^^^^^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` -error: wildcard pattern covers any other pattern as it will match anyway. Consider replacing with wildcard pattern only +error: wildcard pattern covers any other pattern as it will match anyway. --> $DIR/pats_with_wild_match_arm.rs:34:9 | LL | _ | "bar" => { - | ^^^^^^^^^ help: try this: `_` + | ^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` error: aborting due to 4 previous errors From 8ae8b08e323b7dcfad331af18bebbeb68757a904 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Sun, 5 Jan 2020 15:05:16 +0100 Subject: [PATCH 6/7] Change lint name to WILDCARD_IN_OR_PATTERNS --- CHANGELOG.md | 2 +- clippy_lints/src/lib.rs | 6 +++--- clippy_lints/src/matches.rs | 10 +++++----- src/lintlist/mod.rs | 14 +++++++------- ..._wild_match_arm.fixed => wild_in_or_pats.fixed} | 2 +- ...s_with_wild_match_arm.rs => wild_in_or_pats.rs} | 2 +- ...ild_match_arm.stderr => wild_in_or_pats.stderr} | 10 +++++----- tests/ui/wildcard_enum_match_arm.fixed | 2 +- tests/ui/wildcard_enum_match_arm.rs | 2 +- 9 files changed, 25 insertions(+), 25 deletions(-) rename tests/ui/{pats_with_wild_match_arm.fixed => wild_in_or_pats.fixed} (93%) rename tests/ui/{pats_with_wild_match_arm.rs => wild_in_or_pats.rs} (94%) rename tests/ui/{pats_with_wild_match_arm.stderr => wild_in_or_pats.stderr} (76%) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4ecdabeb14f..69694da15203 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1241,7 +1241,6 @@ Released 2018-09-13 [`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite -[`pats_with_wild_match_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#pats_with_wild_match_arm [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence [`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal @@ -1362,6 +1361,7 @@ Released 2018-09-13 [`while_let_on_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#while_let_on_iterator [`wildcard_dependencies`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_dependencies [`wildcard_enum_match_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_enum_match_arm +[`wildcard_in_or_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#wildcard_in_or_patterns [`write_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#write_literal [`write_with_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#write_with_newline [`writeln_empty_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#writeln_empty_string diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index a009bab57c67..518b6ee6141e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -597,10 +597,10 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf &matches::MATCH_OVERLAPPING_ARM, &matches::MATCH_REF_PATS, &matches::MATCH_WILD_ERR_ARM, - &matches::PATS_WITH_WILD_MATCH_ARM, &matches::SINGLE_MATCH, &matches::SINGLE_MATCH_ELSE, &matches::WILDCARD_ENUM_MATCH_ARM, + &matches::WILDCARD_IN_OR_PATTERNS, &mem_discriminant::MEM_DISCRIMINANT_NON_ENUM, &mem_forget::MEM_FORGET, &mem_replace::MEM_REPLACE_OPTION_WITH_NONE, @@ -1188,8 +1188,8 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_REF_PATS), LintId::of(&matches::MATCH_WILD_ERR_ARM), - LintId::of(&matches::PATS_WITH_WILD_MATCH_ARM), LintId::of(&matches::SINGLE_MATCH), + LintId::of(&matches::WILDCARD_IN_OR_PATTERNS), LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM), LintId::of(&mem_replace::MEM_REPLACE_OPTION_WITH_NONE), LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT), @@ -1463,7 +1463,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(&matches::MATCH_AS_REF), - LintId::of(&matches::PATS_WITH_WILD_MATCH_ARM), + LintId::of(&matches::WILDCARD_IN_OR_PATTERNS), LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::CLONE_ON_COPY), LintId::of(&methods::FILTER_NEXT), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 2347a0619d65..2e25b7236d75 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -238,7 +238,7 @@ declare_clippy_lint! { /// "bar" | _ => {}, /// } /// ``` - pub PATS_WITH_WILD_MATCH_ARM, + pub WILDCARD_IN_OR_PATTERNS, complexity, "a wildcard pattern used with others patterns in same match arm" } @@ -252,7 +252,7 @@ declare_lint_pass!(Matches => [ MATCH_WILD_ERR_ARM, MATCH_AS_REF, WILDCARD_ENUM_MATCH_ARM, - PATS_WITH_WILD_MATCH_ARM + WILDCARD_IN_OR_PATTERNS ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { @@ -267,7 +267,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { check_wild_err_arm(cx, ex, arms); check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); - check_pats_wild_match(cx, ex, arms); + check_wild_in_or_pats(cx, ex, arms); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -686,7 +686,7 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], } } -fn check_pats_wild_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { +fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { let mut is_non_exhaustive_enum = false; let ty = cx.tables.expr_ty(ex); if ty.is_enum() { @@ -703,7 +703,7 @@ fn check_pats_wild_match(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_ if fields.len() > 1 && fields.iter().any(is_wild) { span_lint_and_then( cx, - PATS_WITH_WILD_MATCH_ARM, + WILDCARD_IN_OR_PATTERNS, arm.pat.span, "wildcard pattern covers any other pattern as it will match anyway.", |db| { diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 34bf4749d83f..399f3483a59b 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1568,13 +1568,6 @@ pub const ALL_LINTS: [Lint; 345] = [ deprecation: None, module: "path_buf_push_overwrite", }, - Lint { - name: "pats_with_wild_match_arm", - group: "complexity", - desc: "a wildcard pattern used with others patterns in same match arm", - deprecation: None, - module: "matches", - }, Lint { name: "possible_missing_comma", group: "correctness", @@ -2352,6 +2345,13 @@ pub const ALL_LINTS: [Lint; 345] = [ deprecation: None, module: "matches", }, + Lint { + name: "wildcard_in_or_patterns", + group: "complexity", + desc: "a wildcard pattern used with others patterns in same match arm", + deprecation: None, + module: "matches", + }, Lint { name: "write_literal", group: "style", diff --git a/tests/ui/pats_with_wild_match_arm.fixed b/tests/ui/wild_in_or_pats.fixed similarity index 93% rename from tests/ui/pats_with_wild_match_arm.fixed rename to tests/ui/wild_in_or_pats.fixed index daa34af94363..40d4a6e8fac5 100644 --- a/tests/ui/pats_with_wild_match_arm.fixed +++ b/tests/ui/wild_in_or_pats.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![warn(clippy::pats_with_wild_match_arm)] +#![warn(clippy::wildcard_in_or_patterns)] fn main() { match "foo" { diff --git a/tests/ui/pats_with_wild_match_arm.rs b/tests/ui/wild_in_or_pats.rs similarity index 94% rename from tests/ui/pats_with_wild_match_arm.rs rename to tests/ui/wild_in_or_pats.rs index 0410cecb5d67..569871db936f 100644 --- a/tests/ui/pats_with_wild_match_arm.rs +++ b/tests/ui/wild_in_or_pats.rs @@ -1,6 +1,6 @@ // run-rustfix -#![warn(clippy::pats_with_wild_match_arm)] +#![warn(clippy::wildcard_in_or_patterns)] fn main() { match "foo" { diff --git a/tests/ui/pats_with_wild_match_arm.stderr b/tests/ui/wild_in_or_pats.stderr similarity index 76% rename from tests/ui/pats_with_wild_match_arm.stderr rename to tests/ui/wild_in_or_pats.stderr index 215d0b2e4134..45ae31db21a9 100644 --- a/tests/ui/pats_with_wild_match_arm.stderr +++ b/tests/ui/wild_in_or_pats.stderr @@ -1,25 +1,25 @@ error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/pats_with_wild_match_arm.rs:10:9 + --> $DIR/wild_in_or_pats.rs:10:9 | LL | "bar" | _ => { | ^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` | - = note: `-D clippy::pats-with-wild-match-arm` implied by `-D warnings` + = note: `-D clippy::wildcard-in-or-patterns` implied by `-D warnings` error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/pats_with_wild_match_arm.rs:18:9 + --> $DIR/wild_in_or_pats.rs:18:9 | LL | "bar" | "bar2" | _ => { | ^^^^^^^^^^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/pats_with_wild_match_arm.rs:26:9 + --> $DIR/wild_in_or_pats.rs:26:9 | LL | _ | "bar" | _ => { | ^^^^^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/pats_with_wild_match_arm.rs:34:9 + --> $DIR/wild_in_or_pats.rs:34:9 | LL | _ | "bar" => { | ^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index f46320e03cc5..2aa24ea1156a 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -6,7 +6,7 @@ unused_variables, dead_code, clippy::single_match, - clippy::pats_with_wild_match_arm + clippy::wildcard_in_or_patterns )] use std::io::ErrorKind; diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index 508d0e0bb796..07c93feaf284 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -6,7 +6,7 @@ unused_variables, dead_code, clippy::single_match, - clippy::pats_with_wild_match_arm + clippy::wildcard_in_or_patterns )] use std::io::ErrorKind; From 0fa0df9efb9f34c8e0612bd0d0844417259873e9 Mon Sep 17 00:00:00 2001 From: ThibsG Date: Tue, 7 Jan 2020 18:13:31 +0100 Subject: [PATCH 7/7] Span help without suggestion --- README.md | 2 +- clippy_lints/src/matches.rs | 38 ++++++--------------------------- src/lintlist/mod.rs | 2 +- tests/ui/wild_in_or_pats.fixed | 38 --------------------------------- tests/ui/wild_in_or_pats.rs | 2 -- tests/ui/wild_in_or_pats.stderr | 23 +++++++++++++------- 6 files changed, 23 insertions(+), 82 deletions(-) delete mode 100644 tests/ui/wild_in_or_pats.fixed diff --git a/README.md b/README.md index c46d3e16bb17..4de256e9e72f 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 345 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 346 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 2e25b7236d75..6b5b4e4c4f0b 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -3,7 +3,8 @@ use crate::utils::paths; use crate::utils::sugg::Sugg; use crate::utils::{ expr_block, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg, remove_blocks, snippet, - snippet_with_applicability, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, + snippet_with_applicability, span_help_and_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint, + walk_ptrs_ty, }; use if_chain::if_chain; use rustc::declare_lint_pass; @@ -267,7 +268,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { check_wild_err_arm(cx, ex, arms); check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); - check_wild_in_or_pats(cx, ex, arms); + check_wild_in_or_pats(cx, arms); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -686,44 +687,17 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], } } -fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { - let mut is_non_exhaustive_enum = false; - let ty = cx.tables.expr_ty(ex); - if ty.is_enum() { - if let ty::Adt(def, _) = ty.kind { - if def.is_variant_list_non_exhaustive() { - is_non_exhaustive_enum = true; - } - } - } - +fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) { for arm in arms { if let PatKind::Or(ref fields) = arm.pat.kind { // look for multiple fields in this arm that contains at least one Wild pattern if fields.len() > 1 && fields.iter().any(is_wild) { - span_lint_and_then( + span_help_and_lint( cx, WILDCARD_IN_OR_PATTERNS, arm.pat.span, "wildcard pattern covers any other pattern as it will match anyway.", - |db| { - // handle case where a non exhaustive enum is being used - if is_non_exhaustive_enum { - db.span_suggestion( - arm.pat.span, - "consider handling `_` separately.", - "_ => ...".to_string(), - Applicability::MaybeIncorrect, - ); - } else { - db.span_suggestion( - arm.pat.span, - "consider replacing with wildcard pattern only", - "_".to_string(), - Applicability::MachineApplicable, - ); - } - }, + "Consider handling `_` separately.", ); } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 399f3483a59b..3cc4efb9b057 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 345] = [ +pub const ALL_LINTS: [Lint; 346] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", diff --git a/tests/ui/wild_in_or_pats.fixed b/tests/ui/wild_in_or_pats.fixed deleted file mode 100644 index 40d4a6e8fac5..000000000000 --- a/tests/ui/wild_in_or_pats.fixed +++ /dev/null @@ -1,38 +0,0 @@ -// run-rustfix - -#![warn(clippy::wildcard_in_or_patterns)] - -fn main() { - match "foo" { - "a" => { - dbg!("matched a"); - }, - _ => { - dbg!("matched (bar or) wild"); - }, - }; - match "foo" { - "a" => { - dbg!("matched a"); - }, - _ => { - dbg!("matched (bar or bar2 or) wild"); - }, - }; - match "foo" { - "a" => { - dbg!("matched a"); - }, - _ => { - dbg!("matched (bar or) wild"); - }, - }; - match "foo" { - "a" => { - dbg!("matched a"); - }, - _ => { - dbg!("matched (bar or) wild"); - }, - }; -} diff --git a/tests/ui/wild_in_or_pats.rs b/tests/ui/wild_in_or_pats.rs index 569871db936f..ad600f125772 100644 --- a/tests/ui/wild_in_or_pats.rs +++ b/tests/ui/wild_in_or_pats.rs @@ -1,5 +1,3 @@ -// run-rustfix - #![warn(clippy::wildcard_in_or_patterns)] fn main() { diff --git a/tests/ui/wild_in_or_pats.stderr b/tests/ui/wild_in_or_pats.stderr index 45ae31db21a9..33c34cbbd408 100644 --- a/tests/ui/wild_in_or_pats.stderr +++ b/tests/ui/wild_in_or_pats.stderr @@ -1,28 +1,35 @@ error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/wild_in_or_pats.rs:10:9 + --> $DIR/wild_in_or_pats.rs:8:9 | LL | "bar" | _ => { - | ^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` + | ^^^^^^^^^ | = note: `-D clippy::wildcard-in-or-patterns` implied by `-D warnings` + = help: Consider handling `_` separately. error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/wild_in_or_pats.rs:18:9 + --> $DIR/wild_in_or_pats.rs:16:9 | LL | "bar" | "bar2" | _ => { - | ^^^^^^^^^^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` + | ^^^^^^^^^^^^^^^^^^ + | + = help: Consider handling `_` separately. error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/wild_in_or_pats.rs:26:9 + --> $DIR/wild_in_or_pats.rs:24:9 | LL | _ | "bar" | _ => { - | ^^^^^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` + | ^^^^^^^^^^^^^ + | + = help: Consider handling `_` separately. error: wildcard pattern covers any other pattern as it will match anyway. - --> $DIR/wild_in_or_pats.rs:34:9 + --> $DIR/wild_in_or_pats.rs:32:9 | LL | _ | "bar" => { - | ^^^^^^^^^ help: consider replacing with wildcard pattern only: `_` + | ^^^^^^^^^ + | + = help: Consider handling `_` separately. error: aborting due to 4 previous errors