fix: match_single_binding wrongly handles scope

Author: profetia

clippy_lints/src/matches/match_single_binding.rs

@@ -1,6 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::macros::HirNode;
-use clippy_utils::source::{indent_of, snippet, snippet_block_with_context, snippet_with_context};
+use clippy_utils::source::{
+    RelativeIndent, indent_of, reindent_multiline_relative, snippet, snippet_block_with_context, snippet_with_context,
+};
 use clippy_utils::{is_refutable, peel_blocks};
 use rustc_errors::Applicability;
 use rustc_hir::{Arm, Expr, ExprKind, Node, PatKind, StmtKind};
@@ -50,7 +52,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
                         cx,
                         (ex, expr),
                         (bind_names, matched_vars),
-                        &snippet_body,
+                        snippet_body,
                         &mut app,
                         Some(span),
                         true,
@@ -83,7 +85,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
                         cx,
                         (ex, expr),
                         (bind_names, matched_vars),
-                        &snippet_body,
+                        snippet_body,
                         &mut app,
                         None,
                         true,
@@ -108,7 +110,7 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
                     cx,
                     (ex, expr),
                     (bind_names, matched_vars),
-                    &snippet_body,
+                    snippet_body,
                     &mut app,
                     None,
                     false,
@@ -161,47 +163,56 @@ fn opt_parent_assign_span<'a>(cx: &LateContext<'a>, ex: &Expr<'a>) -> Option<Ass
     None
 }
 
-fn expr_parent_requires_curlies<'a>(cx: &LateContext<'a>, match_expr: &Expr<'a>) -> bool {
+fn expr_in_nested_block(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool {
+    if let Node::Block(block) = cx.tcx.parent_hir_node(match_expr.hir_id) {
+        return block
+            .expr
+            .map_or_else(|| matches!(block.stmts, [_]), |_| block.stmts.is_empty());
+    }
+    false
+}
+
+fn expr_must_have_curlies(cx: &LateContext<'_>, match_expr: &Expr<'_>) -> bool {
     let parent = cx.tcx.parent_hir_node(match_expr.hir_id);
-    matches!(
-        parent,
-        Node::Expr(Expr {
-            kind: ExprKind::Closure { .. },
-            ..
-        }) | Node::AnonConst(..)
+    if let Node::Expr(Expr {
+        kind: ExprKind::Closure { .. },
+        ..
+    })
+    | Node::AnonConst(..) = parent
+    {
+        return true;
+    }
+
+    if let Node::Arm(arm) = &cx.tcx.parent_hir_node(match_expr.hir_id)
+        && let ExprKind::Match(..) = arm.body.kind
+    {
+        return true;
+    }
+
+    false
+}
+
+fn reindent_snippet_if_in_block(snippet_body: &str, has_assignment: bool) -> String {
+    if has_assignment || !snippet_body.starts_with('{') {
+        return reindent_multiline_relative(snippet_body, true, RelativeIndent::Add(0));
+    }
+
+    reindent_multiline_relative(
+        snippet_body.trim_start_matches('{').trim_end_matches('}').trim(),
+        false,
+        RelativeIndent::Sub(4),
     )
 }
 
 fn sugg_with_curlies<'a>(
     cx: &LateContext<'a>,
     (ex, match_expr): (&Expr<'a>, &Expr<'a>),
     (bind_names, matched_vars): (Span, Span),
-    snippet_body: &str,
+    mut snippet_body: String,
     applicability: &mut Applicability,
     assignment: Option<Span>,
     needs_var_binding: bool,
 ) -> String {
-    let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
-
-    let (mut cbrace_start, mut cbrace_end) = (String::new(), String::new());
-    if expr_parent_requires_curlies(cx, match_expr) {
-        cbrace_end = format!("\n{indent}}}");
-        // Fix body indent due to the closure
-        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
-        cbrace_start = format!("{{\n{indent}");
-    }
-
-    // If the parent is already an arm, and the body is another match statement,
-    // we need curly braces around suggestion
-    if let Node::Arm(arm) = &cx.tcx.parent_hir_node(match_expr.hir_id)
-        && let ExprKind::Match(..) = arm.body.kind
-    {
-        cbrace_end = format!("\n{indent}}}");
-        // Fix body indent due to the match
-        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
-        cbrace_start = format!("{{\n{indent}");
-    }
-
     let assignment_str = assignment.map_or_else(String::new, |span| {
         let mut s = snippet(cx, span, "..").to_string();
         s.push_str(" = ");
@@ -221,5 +232,15 @@ fn sugg_with_curlies<'a>(
             .to_string()
     };
 
+    let mut indent = " ".repeat(indent_of(cx, ex.span).unwrap_or(0));
+    let (mut cbrace_start, mut cbrace_end) = (String::new(), String::new());
+    if !expr_in_nested_block(cx, match_expr) && (needs_var_binding || expr_must_have_curlies(cx, match_expr)) {
+        cbrace_end = format!("\n{indent}}}");
+        // Fix body indent due to the closure
+        indent = " ".repeat(indent_of(cx, bind_names).unwrap_or(0));
+        cbrace_start = format!("{{\n{indent}");
+        snippet_body = reindent_snippet_if_in_block(&snippet_body, !assignment_str.is_empty());
+    }
+
     format!("{cbrace_start}{scrutinee};\n{indent}{assignment_str}{snippet_body}{cbrace_end}")
 }

clippy_utils/src/source.rs

@@ -18,7 +18,7 @@ use rustc_span::{
 };
 use std::borrow::Cow;
 use std::fmt;
-use std::ops::{Deref, Index, Range};
+use std::ops::{Add, Deref, Index, Range};
 
 pub trait HasSession {
     fn sess(&self) -> &Session;
@@ -476,6 +476,38 @@ pub fn position_before_rarrow(s: &str) -> Option<usize> {
     })
 }
 
+pub enum RelativeIndent {
+    Add(usize),
+    Sub(usize),
+}
+
+impl Add<RelativeIndent> for usize {
+    type Output = usize;
+
+    fn add(self, rhs: RelativeIndent) -> Self::Output {
+        match rhs {
+            RelativeIndent::Add(n) => self + n,
+            RelativeIndent::Sub(n) => self.saturating_sub(n),
+        }
+    }
+}
+
+/// Reindents a multiline string with possibility of ignoring the first line and relative
+/// indentation.
+pub fn reindent_multiline_relative(s: &str, ignore_first: bool, relative_indent: RelativeIndent) -> String {
+    fn indent_of_string(s: &str) -> usize {
+        s.find(|c: char| !c.is_whitespace()).unwrap_or(0)
+    }
+
+    let mut indent = 0;
+    if let Some(line) = s.lines().nth(usize::from(ignore_first)) {
+        let line_indent = indent_of_string(line);
+        indent = line_indent + relative_indent;
+    }
+
+    reindent_multiline(s, ignore_first, Some(indent))
+}
+
 /// Reindent a multiline string with possibility of ignoring the first line.
 pub fn reindent_multiline(s: &str, ignore_first: bool, indent: Option<usize>) -> String {
     let s_space = reindent_multiline_inner(s, ignore_first, indent, ' ');

tests/ui/match_single_binding.fixed

@@ -30,13 +30,15 @@ fn main() {
     let b = 2;
     let c = 3;
     // Lint
-    let (x, y, z) = (a, b, c);
     {
+        let (x, y, z) = (a, b, c);
         println!("{} {} {}", x, y, z);
     }
     // Lint
-    let (x, y, z) = (a, b, c);
-    println!("{} {} {}", x, y, z);
+    {
+        let (x, y, z) = (a, b, c);
+        println!("{} {} {}", x, y, z);
+    }
     // Ok
     foo!(a);
     // Ok
@@ -66,19 +68,27 @@ fn main() {
     }
     // Lint
     let p = Point { x: 0, y: 7 };
-    let Point { x, y } = p;
-    println!("Coords: ({}, {})", x, y);
+    {
+        let Point { x, y } = p;
+        println!("Coords: ({}, {})", x, y);
+    }
     // Lint
-    let Point { x: x1, y: y1 } = p;
-    println!("Coords: ({}, {})", x1, y1);
+    {
+        let Point { x: x1, y: y1 } = p;
+        println!("Coords: ({}, {})", x1, y1);
+    }
     // Lint
     let x = 5;
-    let ref r = x;
-    println!("Got a reference to {}", r);
+    {
+        let ref r = x;
+        println!("Got a reference to {}", r);
+    }
     // Lint
     let mut x = 5;
-    let ref mut mr = x;
-    println!("Got a mutable reference to {}", mr);
+    {
+        let ref mut mr = x;
+        println!("Got a mutable reference to {}", mr);
+    }
     // Lint
     let Point { x, y } = coords();
     let product = x * y;
@@ -120,10 +130,12 @@ fn main() {
 fn issue_8723() {
     let (mut val, idx) = ("a b", 1);
 
-    let (pre, suf) = val.split_at(idx);
-    val = {
-        println!("{}", pre);
-        suf
+    {
+        let (pre, suf) = val.split_at(idx);
+        val = {
+            println!("{}", pre);
+            suf
+        }
     };
 
     let _ = val;
@@ -139,14 +151,16 @@ fn issue_9575() {
 }
 
 fn issue_9725(r: Option<u32>) {
-    let x = r;
-    match x {
-        Some(_) => {
-            println!("Some");
-        },
-        None => {
-            println!("None");
-        },
+    {
+        let x = r;
+        match x {
+            Some(_) => {
+                println!("Some");
+            },
+            None => {
+                println!("None");
+            },
+        }
     };
 }
 
@@ -181,8 +195,10 @@ fn issue14634() {
     dbg!(3);
     println!("here");
     //~^^^ match_single_binding
-    let id!(a) = dbg!(3);
-    println!("found {a}");
+    {
+        let id!(a) = dbg!(3);
+        println!("found {a}");
+    }
     //~^^^ match_single_binding
     let id!(b) = dbg!(3);
     let id!(_a) = dbg!(b + 1);
@@ -204,3 +220,12 @@ mod issue14991 {
         }],
     }
 }
+
+fn issue15018() {
+    let a = 10;
+    {
+        let a = 11;
+        println!("a = {a}")
+    };
+    println!("a = {a}");
+}

tests/ui/match_single_binding.rs

@@ -267,3 +267,12 @@ mod issue14991 {
         }],
     }
 }
+
+fn issue15018() {
+    let a = 10;
+    match 11 {
+        //~^ match_single_binding
+        a => println!("a = {a}"),
+    };
+    println!("a = {a}");
+}