Auto merge of #10470 - Alexendoo:match-single-binding-semicolon, r=giraffate

Fix semicolon insertion in `match_single_binding`

changelog: [`match_single_binding`]: Fix missing semicolon after the suggestion

Fixes #10447

Also fixes an edge case for unit returning macros in expression contexts:

```rust
f(match 1 {
    _ => println!("foo"),
});
```

would suggest

```rust
f(println!("foo"););
```

clippy_lints/src/matches/match_single_binding.rs

@@ -3,7 +3,7 @@ use clippy_utils::macros::HirNode;
 use clippy_utils::source::{indent_of, snippet, snippet_block_with_context, snippet_with_applicability};
 use clippy_utils::{get_parent_expr, is_refutable, peel_blocks};
 use rustc_errors::Applicability;
-use rustc_hir::{Arm, Expr, ExprKind, Node, PatKind};
+use rustc_hir::{Arm, Expr, ExprKind, Node, PatKind, StmtKind};
 use rustc_lint::LateContext;
 use rustc_span::Span;
 
@@ -24,22 +24,27 @@ pub(crate) fn check<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[Arm<'_>], e
     let bind_names = arms[0].pat.span;
     let match_body = peel_blocks(arms[0].body);
     let mut app = Applicability::MaybeIncorrect;
-    let (snippet_body, from_macro) = snippet_block_with_context(
+    let mut snippet_body = snippet_block_with_context(
         cx,
         match_body.span,
         arms[0].span.ctxt(),
         "..",
         Some(expr.span),
         &mut app,
-    );
-    let mut snippet_body = snippet_body.to_string();
+    )
+    .0
+    .to_string();
 
     // Do we need to add ';' to suggestion ?
-    if matches!(match_body.kind, ExprKind::Block(..)) {
-        // macro + expr_ty(body) == ()
-        if from_macro && cx.typeck_results().expr_ty(match_body).is_unit() {
-            snippet_body.push(';');
+    if let Node::Stmt(stmt) = cx.tcx.hir().get_parent(expr.hir_id)
+        && let StmtKind::Expr(_) = stmt.kind
+        && match match_body.kind {
+            // We don't need to add a ; to blocks, unless that block is from a macro expansion
+            ExprKind::Block(block, _) => block.span.from_expansion(),
+            _ => true,
         }
+    {
+        snippet_body.push(';');
     }
 
     match arms[0].pat.kind {

tests/ui/match_single_binding.fixed

@@ -1,7 +1,12 @@
 // run-rustfix
 #![warn(clippy::match_single_binding)]
-#![allow(unused_variables)]
-#![allow(clippy::toplevel_ref_arg, clippy::uninlined_format_args)]
+#![allow(
+    unused,
+    clippy::let_unit_value,
+    clippy::no_effect,
+    clippy::toplevel_ref_arg,
+    clippy::uninlined_format_args
+)]
 
 struct Point {
     x: i32,
@@ -109,10 +114,9 @@ fn main() {
 
     // Lint
     let x = 1;
-    println!("Not an array index start");
+    println!("Not an array index start")
 }
 
-#[allow(dead_code)]
 fn issue_8723() {
     let (mut val, idx) = ("a b", 1);
 
@@ -125,16 +129,15 @@ fn issue_8723() {
     let _ = val;
 }
 
-#[allow(dead_code)]
+fn side_effects() {}
+
 fn issue_9575() {
-    fn side_effects() {}
     let _ = || {
         side_effects();
-        println!("Needs curlies");
+        println!("Needs curlies")
     };
 }
 
-#[allow(dead_code)]
 fn issue_9725(r: Option<u32>) {
     let x = r;
     match x {
@@ -146,3 +149,25 @@ fn issue_9725(r: Option<u32>) {
         },
     };
 }
+
+fn issue_10447() -> usize {
+    ();
+
+    let a = ();
+
+    side_effects();
+
+    let b = side_effects();
+
+    println!("1");
+
+    let c = println!("1");
+
+    let in_expr = [
+        (),
+        side_effects(),
+        println!("1"),
+    ];
+
+    2
+}

tests/ui/match_single_binding.rs

@@ -1,7 +1,12 @@
 // run-rustfix
 #![warn(clippy::match_single_binding)]
-#![allow(unused_variables)]
-#![allow(clippy::toplevel_ref_arg, clippy::uninlined_format_args)]
+#![allow(
+    unused,
+    clippy::let_unit_value,
+    clippy::no_effect,
+    clippy::toplevel_ref_arg,
+    clippy::uninlined_format_args
+)]
 
 struct Point {
     x: i32,
@@ -127,7 +132,6 @@ fn main() {
     }
 }
 
-#[allow(dead_code)]
 fn issue_8723() {
     let (mut val, idx) = ("a b", 1);
 
@@ -141,15 +145,14 @@ fn issue_8723() {
     let _ = val;
 }
 
-#[allow(dead_code)]
+fn side_effects() {}
+
 fn issue_9575() {
-    fn side_effects() {}
     let _ = || match side_effects() {
         _ => println!("Needs curlies"),
     };
 }
 
-#[allow(dead_code)]
 fn issue_9725(r: Option<u32>) {
     match r {
         x => match x {
@@ -162,3 +165,43 @@ fn issue_9725(r: Option<u32>) {
         },
     };
 }
+
+fn issue_10447() -> usize {
+    match 1 {
+        _ => (),
+    }
+
+    let a = match 1 {
+        _ => (),
+    };
+
+    match 1 {
+        _ => side_effects(),
+    }
+
+    let b = match 1 {
+        _ => side_effects(),
+    };
+
+    match 1 {
+        _ => println!("1"),
+    }
+
+    let c = match 1 {
+        _ => println!("1"),
+    };
+
+    let in_expr = [
+        match 1 {
+            _ => (),
+        },
+        match 1 {
+            _ => side_effects(),
+        },
+        match 1 {
+            _ => println!("1"),
+        },
+    ];
+
+    2
+}

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:28:5
+  --> $DIR/match_single_binding.rs:33: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:34:5
+  --> $DIR/match_single_binding.rs:39: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:51:5
+  --> $DIR/match_single_binding.rs:56: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:55:5
+  --> $DIR/match_single_binding.rs:60: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:62:5
+  --> $DIR/match_single_binding.rs:67: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:72:5
+  --> $DIR/match_single_binding.rs:77: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:76:5
+  --> $DIR/match_single_binding.rs:81: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:81:5
+  --> $DIR/match_single_binding.rs:86: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:86:5
+  --> $DIR/match_single_binding.rs:91:5
    |
 LL | /     match x {
 LL | |         ref mut mr => println!("Got a mutable reference to {}", mr),
@@ -137,7 +137,7 @@ LL +     println!("Got a mutable reference to {}", mr);
    |
 
 error: this match could be written as a `let` statement
-  --> $DIR/match_single_binding.rs:90:5
+  --> $DIR/match_single_binding.rs:95:5
    |
 LL | /     let product = match coords() {
 LL | |         Point { x, y } => x * y,
@@ -151,7 +151,7 @@ LL +     let product = x * y;
    |
 
 error: this match could be written as a `let` statement
-  --> $DIR/match_single_binding.rs:98:18
+  --> $DIR/match_single_binding.rs:103:18
    |
 LL |           .map(|i| match i.unwrap() {
    |  __________________^
@@ -168,16 +168,16 @@ LL ~         })
    |
 
 error: this match could be replaced by its body itself
-  --> $DIR/match_single_binding.rs:124:5
+  --> $DIR/match_single_binding.rs:129:5
    |
 LL | /     match x {
 LL | |         // =>
 LL | |         _ => println!("Not an array index start"),
 LL | |     }
-   | |_____^ help: consider using the match body instead: `println!("Not an array index start");`
+   | |_____^ help: consider using the match body instead: `println!("Not an array index start")`
 
 error: this assignment could be simplified
-  --> $DIR/match_single_binding.rs:134:5
+  --> $DIR/match_single_binding.rs:138:5
    |
 LL | /     val = match val.split_at(idx) {
 LL | |         (pre, suf) => {
@@ -197,7 +197,7 @@ LL ~     };
    |
 
 error: this match could be replaced by its scrutinee and body
-  --> $DIR/match_single_binding.rs:147:16
+  --> $DIR/match_single_binding.rs:151:16
    |
 LL |       let _ = || match side_effects() {
    |  ________________^
@@ -209,12 +209,12 @@ help: consider using the scrutinee and body instead
    |
 LL ~     let _ = || {
 LL +         side_effects();
-LL +         println!("Needs curlies");
+LL +         println!("Needs curlies")
 LL ~     };
    |
 
 error: this match could be written as a `let` statement
-  --> $DIR/match_single_binding.rs:154:5
+  --> $DIR/match_single_binding.rs:157:5
    |
 LL | /     match r {
 LL | |         x => match x {
@@ -238,5 +238,80 @@ LL +         },
 LL ~     };
    |
 
-error: aborting due to 15 previous errors
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:170:5
+   |
+LL | /     match 1 {
+LL | |         _ => (),
+LL | |     }
+   | |_____^ help: consider using the match body instead: `();`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:174:13
+   |
+LL |       let a = match 1 {
+   |  _____________^
+LL | |         _ => (),
+LL | |     };
+   | |_____^ help: consider using the match body instead: `()`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:178:5
+   |
+LL | /     match 1 {
+LL | |         _ => side_effects(),
+LL | |     }
+   | |_____^ help: consider using the match body instead: `side_effects();`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:182:13
+   |
+LL |       let b = match 1 {
+   |  _____________^
+LL | |         _ => side_effects(),
+LL | |     };
+   | |_____^ help: consider using the match body instead: `side_effects()`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:186:5
+   |
+LL | /     match 1 {
+LL | |         _ => println!("1"),
+LL | |     }
+   | |_____^ help: consider using the match body instead: `println!("1");`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:190:13
+   |
+LL |       let c = match 1 {
+   |  _____________^
+LL | |         _ => println!("1"),
+LL | |     };
+   | |_____^ help: consider using the match body instead: `println!("1")`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:195:9
+   |
+LL | /         match 1 {
+LL | |             _ => (),
+LL | |         },
+   | |_________^ help: consider using the match body instead: `()`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:198:9
+   |
+LL | /         match 1 {
+LL | |             _ => side_effects(),
+LL | |         },
+   | |_________^ help: consider using the match body instead: `side_effects()`
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:201:9
+   |
+LL | /         match 1 {
+LL | |             _ => println!("1"),
+LL | |         },
+   | |_________^ help: consider using the match body instead: `println!("1")`
+
+error: aborting due to 24 previous errors
 

tests/ui/match_single_binding2.fixed

@@ -30,7 +30,7 @@ fn main() {
         #[rustfmt::skip]
         Some((first, _second)) => {
             let (a, b) = get_tup();
-            println!("a {:?} and b {:?}", a, b);
+            println!("a {:?} and b {:?}", a, b)
         },
         None => println!("nothing"),
     }
@@ -49,5 +49,5 @@ fn main() {
         0 => 1,
         _ => 2,
     };
-    println!("Single branch");
+    println!("Single branch")
 }

tests/ui/match_single_binding2.stderr

@@ -27,7 +27,7 @@ LL | |             }
 help: consider using a `let` statement
    |
 LL ~             let (a, b) = get_tup();
-LL +             println!("a {:?} and b {:?}", a, b);
+LL +             println!("a {:?} and b {:?}", a, b)
    |
 
 error: this match could be replaced by its scrutinee and body
@@ -61,7 +61,7 @@ LL ~     match x {
 LL +         0 => 1,
 LL +         _ => 2,
 LL +     };
-LL +     println!("Single branch");
+LL +     println!("Single branch")
    |
 
 error: aborting due to 4 previous errors