diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fcf01ae48a1..2f4e6a0fd48 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -605,6 +605,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &matches::MATCH_OVERLAPPING_ARM, &matches::MATCH_REF_PATS, &matches::MATCH_WILD_ERR_ARM, + &matches::MATCH_SINGLE_BINDING, &matches::SINGLE_MATCH, &matches::SINGLE_MATCH_ELSE, &matches::WILDCARD_ENUM_MATCH_ARM, @@ -1206,6 +1207,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&matches::MATCH_OVERLAPPING_ARM), LintId::of(&matches::MATCH_REF_PATS), LintId::of(&matches::MATCH_WILD_ERR_ARM), + LintId::of(&matches::MATCH_SINGLE_BINDING), LintId::of(&matches::SINGLE_MATCH), LintId::of(&matches::WILDCARD_IN_OR_PATTERNS), LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM), @@ -1483,6 +1485,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, 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::MATCH_SINGLE_BINDING), LintId::of(&matches::WILDCARD_IN_OR_PATTERNS), LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::CLONE_ON_COPY), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 0fc7799c97a..b5cf12b0947 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -3,9 +3,9 @@ use crate::utils::paths; use crate::utils::sugg::Sugg; use crate::utils::usage::is_unused; use crate::utils::{ - expr_block, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks, snippet, - snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, - walk_ptrs_ty, + span_lint_and_help, span_lint_and_note, + expr_block, in_macro, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks, + snippet, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then, }; use if_chain::if_chain; use rustc::lint::in_external_macro; @@ -245,6 +245,33 @@ declare_clippy_lint! { "a wildcard pattern used with others patterns in same match arm" } +declare_clippy_lint! { + /// **What it does:** Checks for useless match that binds to only one value. + /// + /// **Why is this bad?** Readability and needless complexity. + /// + /// **Known problems:** This situation frequently happen in macros, so can't lint there. + /// + /// **Example:** + /// ```rust + /// # let a = 1; + /// # let b = 2; + /// + /// // Bad + /// match (a, b) { + /// (c, d) => { + /// // useless match + /// } + /// } + /// + /// // Good + /// let (c, d) = (a, b); + /// ``` + pub MATCH_SINGLE_BINDING, + complexity, + "a match with a single binding instead of using `let` statement" +} + declare_lint_pass!(Matches => [ SINGLE_MATCH, MATCH_REF_PATS, @@ -254,7 +281,8 @@ declare_lint_pass!(Matches => [ MATCH_WILD_ERR_ARM, MATCH_AS_REF, WILDCARD_ENUM_MATCH_ARM, - WILDCARD_IN_OR_PATTERNS + WILDCARD_IN_OR_PATTERNS, + MATCH_SINGLE_BINDING, ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { @@ -270,6 +298,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { check_wild_enum_match(cx, ex, arms); check_match_as_ref(cx, ex, arms, expr); check_wild_in_or_pats(cx, arms); + check_match_single_binding(cx, ex, arms, expr); } if let ExprKind::Match(ref ex, ref arms, _) = expr.kind { check_match_ref_pats(cx, ex, arms, expr); @@ -712,6 +741,29 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) { } } +fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) { + if in_macro(expr.span) { + return; + } + if arms.len() == 1 { + let bind_names = arms[0].pat.span; + let matched_vars = ex.span; + span_lint_and_sugg( + cx, + MATCH_SINGLE_BINDING, + expr.span, + "this match could be written as a `let` statement", + "try this", + format!( + "let {} = {};", + snippet(cx, bind_names, ".."), + snippet(cx, matched_vars, "..") + ), + Applicability::HasPlaceholders, + ); + } +} + /// Gets all arms that are unbounded `PatRange`s. fn all_ranges<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs new file mode 100644 index 00000000000..816a7e7197c --- /dev/null +++ b/tests/ui/match_single_binding.rs @@ -0,0 +1,25 @@ +#![warn(clippy::match_single_binding)] +#[allow(clippy::many_single_char_names)] + +fn main() { + let a = 1; + let b = 2; + let c = 3; + // Lint + match (a, b, c) { + (x, y, z) => { + println!("{} {} {}", x, y, z); + }, + } + // Ok + match a { + 2 => println!("2"), + _ => println!("Not 2"), + } + // Ok + let d = Some(5); + match d { + Some(d) => println!("5"), + _ => println!("None"), + } +} diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr new file mode 100644 index 00000000000..2fad6aab32c --- /dev/null +++ b/tests/ui/match_single_binding.stderr @@ -0,0 +1,14 @@ +error: this match could be written as a `let` statement + --> $DIR/match_single_binding.rs:9:5 + | +LL | / match (a, b, c) { +LL | | (x, y, z) => { +LL | | println!("{} {} {}", x, y, z); +LL | | }, +LL | | } + | |_____^ help: try this: `let (x, y, z) = (a, b, c);` + | + = note: `-D clippy::match-single-binding` implied by `-D warnings` + +error: aborting due to previous error +