Auto merge of #5291 - ThibsG:FixSingleBinding, r=flip1995

Fix match single binding when in a let stmt

Fix bad suggestion when `match_single_binding` lints when inside a local (let) statement.

Fixes #5267

changelog: none

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(..) => {
-            span_lint_and_sugg(
-                cx,
-                MATCH_SINGLE_BINDING,
+            // 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,
-                "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,
+                        snippet_body
                     ),
+                )
+            };
+            span_lint_and_sugg(
+                cx,
+                MATCH_SINGLE_BINDING,
+                target_span,
+                "this match could be written as a `let` statement",
+                "consider using `let` statement",
+                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>,

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;
 }

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,
+    };
 }

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