From 96c41988323cd4c3d211e494fd3e143caf11f8cd Mon Sep 17 00:00:00 2001 From: ThibsG Date: Tue, 24 Dec 2019 16:42:09 +0100 Subject: [PATCH] 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 +