diff --git a/CHANGELOG.md b/CHANGELOG.md index f60ac7d5267..69694da1520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1361,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/README.md b/README.md index c46d3e16bb1..4de256e9e72 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/lib.rs b/clippy_lints/src/lib.rs index 0ba1bf6008c..1a112c0d370 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -600,6 +600,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf &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,6 +1189,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf LintId::of(&matches::MATCH_REF_PATS), LintId::of(&matches::MATCH_WILD_ERR_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), @@ -1462,6 +1464,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::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 3200de1cfc1..6b5b4e4c4f0 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; @@ -223,6 +224,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 WILDCARD_IN_OR_PATTERNS, + complexity, + "a wildcard pattern used with others patterns in same match arm" +} + declare_lint_pass!(Matches => [ SINGLE_MATCH, MATCH_REF_PATS, @@ -231,7 +252,8 @@ declare_lint_pass!(Matches => [ MATCH_OVERLAPPING_ARM, MATCH_WILD_ERR_ARM, MATCH_AS_REF, - WILDCARD_ENUM_MATCH_ARM + WILDCARD_ENUM_MATCH_ARM, + WILDCARD_IN_OR_PATTERNS ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { @@ -246,6 +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, arms); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -664,6 +687,23 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], } } +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_help_and_lint( + cx, + WILDCARD_IN_OR_PATTERNS, + arm.pat.span, + "wildcard pattern covers any other pattern as it will match anyway.", + "Consider handling `_` separately.", + ); + } + } + } +} + /// Gets all arms that are unbounded `PatRange`s. fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm<'_>]) -> Vec<SpannedRange<Constant>> { arms.iter() diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index f237809d920..f576bb152de 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", @@ -2345,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/wild_in_or_pats.rs b/tests/ui/wild_in_or_pats.rs new file mode 100644 index 00000000000..ad600f12577 --- /dev/null +++ b/tests/ui/wild_in_or_pats.rs @@ -0,0 +1,36 @@ +#![warn(clippy::wildcard_in_or_patterns)] + +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/wild_in_or_pats.stderr b/tests/ui/wild_in_or_pats.stderr new file mode 100644 index 00000000000..33c34cbbd40 --- /dev/null +++ b/tests/ui/wild_in_or_pats.stderr @@ -0,0 +1,35 @@ +error: wildcard pattern covers any other pattern as it will match anyway. + --> $DIR/wild_in_or_pats.rs:8:9 + | +LL | "bar" | _ => { + | ^^^^^^^^^ + | + = 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:16:9 + | +LL | "bar" | "bar2" | _ => { + | ^^^^^^^^^^^^^^^^^^ + | + = help: Consider handling `_` separately. + +error: wildcard pattern covers any other pattern as it will match anyway. + --> $DIR/wild_in_or_pats.rs:24:9 + | +LL | _ | "bar" | _ => { + | ^^^^^^^^^^^^^ + | + = help: Consider handling `_` separately. + +error: wildcard pattern covers any other pattern as it will match anyway. + --> $DIR/wild_in_or_pats.rs:32:9 + | +LL | _ | "bar" => { + | ^^^^^^^^^ + | + = help: Consider handling `_` separately. + +error: aborting due to 4 previous errors + diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 1da2833e260..2aa24ea1156 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::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 c2eb4b30802..07c93feaf28 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::wildcard_in_or_patterns +)] use std::io::ErrorKind; diff --git a/tests/ui/wildcard_enum_match_arm.stderr b/tests/ui/wildcard_enum_match_arm.stderr index 735f610b7e5..e6f0411095c 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 | _`