diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 9668c5d83..b0aae5e30 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -14,8 +14,8 @@ use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::CtorKind; use rustc_hir::{ - print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Pat, PatKind, - QPath, RangeEnd, + print, Arm, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, Local, MatchSource, Mutability, Node, Pat, + PatKind, QPath, RangeEnd, }; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -882,7 +882,7 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) { } } -fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) { +fn check_match_single_binding<'a>(cx: &LateContext<'_, 'a>, ex: &Expr<'a>, arms: &[Arm<'_>], expr: &Expr<'_>) { if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) { return; } @@ -914,19 +914,38 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A let mut applicability = Applicability::MaybeIncorrect; match arms[0].pat.kind { PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => { + // If this match is in a local (`let`) stmt + let (target_span, sugg) = if let Some(parent_let_node) = opt_parent_let(cx, ex) { + ( + parent_let_node.span, + format!( + "let {} = {};\n{}let {} = {};", + snippet_with_applicability(cx, bind_names, "..", &mut applicability), + snippet_with_applicability(cx, matched_vars, "..", &mut applicability), + " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), + snippet_with_applicability(cx, parent_let_node.pat.span, "..", &mut applicability), + snippet_body + ), + ) + } else { + ( + expr.span, + format!( + "let {} = {};\n{}{}", + snippet_with_applicability(cx, bind_names, "..", &mut applicability), + snippet_with_applicability(cx, matched_vars, "..", &mut applicability), + " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), + snippet_body + ), + ) + }; span_lint_and_sugg( cx, MATCH_SINGLE_BINDING, - expr.span, + target_span, "this match could be written as a `let` statement", "consider using `let` statement", - format!( - "let {} = {};\n{}{}", - snippet_with_applicability(cx, bind_names, "..", &mut applicability), - snippet_with_applicability(cx, matched_vars, "..", &mut applicability), - " ".repeat(indent_of(cx, expr.span).unwrap_or(0)), - snippet_body, - ), + sugg, applicability, ); }, @@ -945,6 +964,19 @@ fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[A } } +/// Returns true if the `ex` match expression is in a local (`let`) statement +fn opt_parent_let<'a>(cx: &LateContext<'_, 'a>, ex: &Expr<'a>) -> Option<&'a Local<'a>> { + if_chain! { + let map = &cx.tcx.hir(); + if let Some(Node::Expr(parent_arm_expr)) = map.find(map.get_parent_node(ex.hir_id)); + if let Some(Node::Local(parent_let_expr)) = map.find(map.get_parent_node(parent_arm_expr.hir_id)); + then { + return Some(parent_let_expr); + } + } + None +} + /// 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.fixed b/tests/ui/match_single_binding.fixed index 932bd6783..bc2346a8d 100644 --- a/tests/ui/match_single_binding.fixed +++ b/tests/ui/match_single_binding.fixed @@ -1,13 +1,17 @@ // run-rustfix #![warn(clippy::match_single_binding)] -#![allow(clippy::many_single_char_names, clippy::toplevel_ref_arg)] +#![allow(unused_variables, clippy::many_single_char_names, clippy::toplevel_ref_arg)] struct Point { x: i32, y: i32, } +fn coords() -> Point { + Point { x: 1, y: 2 } +} + fn main() { let a = 1; let b = 2; @@ -60,4 +64,7 @@ fn main() { let mut x = 5; let ref mut mr = x; println!("Got a mutable reference to {}", mr); + // Lint + let Point { x, y } = coords(); + let product = x * y; } diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs index 55b0b09a0..0517b3bbf 100644 --- a/tests/ui/match_single_binding.rs +++ b/tests/ui/match_single_binding.rs @@ -1,13 +1,17 @@ // run-rustfix #![warn(clippy::match_single_binding)] -#![allow(clippy::many_single_char_names, clippy::toplevel_ref_arg)] +#![allow(unused_variables, clippy::many_single_char_names, clippy::toplevel_ref_arg)] struct Point { x: i32, y: i32, } +fn coords() -> Point { + Point { x: 1, y: 2 } +} + fn main() { let a = 1; let b = 2; @@ -72,4 +76,8 @@ fn main() { match x { ref mut mr => println!("Got a mutable reference to {}", mr), } + // Lint + let product = match coords() { + Point { x, y } => x * y, + }; } diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr index 471aec780..05ba9e5f7 100644 --- a/tests/ui/match_single_binding.stderr +++ b/tests/ui/match_single_binding.stderr @@ -1,5 +1,5 @@ error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:16:5 + --> $DIR/match_single_binding.rs:20:5 | LL | / match (a, b, c) { LL | | (x, y, z) => { @@ -18,7 +18,7 @@ LL | } | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:22:5 + --> $DIR/match_single_binding.rs:26:5 | LL | / match (a, b, c) { LL | | (x, y, z) => println!("{} {} {}", x, y, z), @@ -32,7 +32,7 @@ LL | println!("{} {} {}", x, y, z); | error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:37:5 + --> $DIR/match_single_binding.rs:41:5 | LL | / match a { LL | | _ => println!("whatever"), @@ -40,7 +40,7 @@ LL | | } | |_____^ help: consider using the match body instead: `println!("whatever");` error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:41:5 + --> $DIR/match_single_binding.rs:45:5 | LL | / match a { LL | | _ => { @@ -59,7 +59,7 @@ LL | } | error: this match could be replaced by its body itself - --> $DIR/match_single_binding.rs:48:5 + --> $DIR/match_single_binding.rs:52:5 | LL | / match a { LL | | _ => { @@ -81,7 +81,7 @@ LL | } | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:58:5 + --> $DIR/match_single_binding.rs:62:5 | LL | / match p { LL | | Point { x, y } => println!("Coords: ({}, {})", x, y), @@ -95,7 +95,7 @@ LL | println!("Coords: ({}, {})", x, y); | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:62:5 + --> $DIR/match_single_binding.rs:66:5 | LL | / match p { LL | | Point { x: x1, y: y1 } => println!("Coords: ({}, {})", x1, y1), @@ -109,7 +109,7 @@ LL | println!("Coords: ({}, {})", x1, y1); | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:67:5 + --> $DIR/match_single_binding.rs:71:5 | LL | / match x { LL | | ref r => println!("Got a reference to {}", r), @@ -123,7 +123,7 @@ LL | println!("Got a reference to {}", r); | error: this match could be written as a `let` statement - --> $DIR/match_single_binding.rs:72:5 + --> $DIR/match_single_binding.rs:76:5 | LL | / match x { LL | | ref mut mr => println!("Got a mutable reference to {}", mr), @@ -136,5 +136,19 @@ LL | let ref mut mr = x; LL | println!("Got a mutable reference to {}", mr); | -error: aborting due to 9 previous errors +error: this match could be written as a `let` statement + --> $DIR/match_single_binding.rs:80:5 + | +LL | / let product = match coords() { +LL | | Point { x, y } => x * y, +LL | | }; + | |______^ + | +help: consider using `let` statement + | +LL | let Point { x, y } = coords(); +LL | let product = x * y; + | + +error: aborting due to 10 previous errors