Auto merge of rust-lang#7095 - Y-Nak:match_single_binding, r=giraffate

match_single_binding: Fix invalid suggestion when match scrutinee has side effects

fixes rust-lang#7094

changelog: `match_single_binding`: Fix invalid suggestion when match scrutinee has side effects

---
`Expr::can_have_side_effects` is used to determine the scrutinee has side effects, while this method is a little bit conservative for our use case. But I'd like to use it to avoid reimplementation of the method and too much heuristics. If you think this is problematic, then I'll implement a custom visitor to address it.

Author: bors

clippy_lints/src/matches.rs

@@ -1478,15 +1478,34 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
             );
         },
         PatKind::Wild => {
-            span_lint_and_sugg(
-                cx,
-                MATCH_SINGLE_BINDING,
-                expr.span,
-                "this match could be replaced by its body itself",
-                "consider using the match body instead",
-                snippet_body,
-                Applicability::MachineApplicable,
-            );
+            if ex.can_have_side_effects() {
+                let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
+                let sugg = format!(
+                    "{};\n{}{}",
+                    snippet_with_applicability(cx, ex.span, "..", &mut applicability),
+                    indent,
+                    snippet_body
+                );
+                span_lint_and_sugg(
+                    cx,
+                    MATCH_SINGLE_BINDING,
+                    expr.span,
+                    "this match could be replaced by its scrutinee and body",
+                    "consider using the scrutinee and body instead",
+                    sugg,
+                    applicability,
+                )
+            } else {
+                span_lint_and_sugg(
+                    cx,
+                    MATCH_SINGLE_BINDING,
+                    expr.span,
+                    "this match could be replaced by its body itself",
+                    "consider using the match body instead",
+                    snippet_body,
+                    Applicability::MachineApplicable,
+                );
+            }
         },
         _ => (),
     }

tests/ui/match_single_binding.fixed

@@ -94,10 +94,7 @@ fn main() {
         0 => println!("Disabled branch"),
         _ => println!("Enabled branch"),
     }
-    // Lint
-    let x = 1;
-    let y = 1;
-    println!("Single branch");
+
     // Ok
     let x = 1;
     let y = 1;

tests/ui/match_single_binding.rs

@@ -106,15 +106,7 @@ fn main() {
         0 => println!("Disabled branch"),
         _ => println!("Enabled branch"),
     }
-    // Lint
-    let x = 1;
-    let y = 1;
-    match match y {
-        0 => 1,
-        _ => 2,
-    } {
-        _ => println!("Single branch"),
-    }
+
     // Ok
     let x = 1;
     let y = 1;

tests/ui/match_single_binding.stderr

@@ -167,16 +167,5 @@ LL |             unwrapped
 LL |         })
    |
 
-error: this match could be replaced by its body itself
-  --> $DIR/match_single_binding.rs:112:5
-   |
-LL | /     match match y {
-LL | |         0 => 1,
-LL | |         _ => 2,
-LL | |     } {
-LL | |         _ => println!("Single branch"),
-LL | |     }
-   | |_____^ help: consider using the match body instead: `println!("Single branch");`
-
-error: aborting due to 12 previous errors
+error: aborting due to 11 previous errors
 

tests/ui/match_single_binding2.fixed

@@ -34,4 +34,20 @@ fn main() {
         },
         None => println!("nothing"),
     }
+
+    fn side_effects() {}
+
+    // Lint (scrutinee has side effects)
+    // issue #7094
+    side_effects();
+    println!("Side effects");
+
+    // Lint (scrutinee has side effects)
+    // issue #7094
+    let x = 1;
+    match x {
+        0 => 1,
+        _ => 2,
+    };
+    println!("Single branch");
 }

tests/ui/match_single_binding2.rs

@@ -34,4 +34,22 @@ fn main() {
         },
         None => println!("nothing"),
     }
+
+    fn side_effects() {}
+
+    // Lint (scrutinee has side effects)
+    // issue #7094
+    match side_effects() {
+        _ => println!("Side effects"),
+    }
+
+    // Lint (scrutinee has side effects)
+    // issue #7094
+    let x = 1;
+    match match x {
+        0 => 1,
+        _ => 2,
+    } {
+        _ => println!("Single branch"),
+    }
 }

tests/ui/match_single_binding2.stderr

@@ -30,5 +30,39 @@ LL |             let (a, b) = get_tup();
 LL |             println!("a {:?} and b {:?}", a, b);
    |
 
-error: aborting due to 2 previous errors
+error: this match could be replaced by its scrutinee and body
+  --> $DIR/match_single_binding2.rs:42:5
+   |
+LL | /     match side_effects() {
+LL | |         _ => println!("Side effects"),
+LL | |     }
+   | |_____^
+   |
+help: consider using the scrutinee and body instead
+   |
+LL |     side_effects();
+LL |     println!("Side effects");
+   |
+
+error: this match could be replaced by its scrutinee and body
+  --> $DIR/match_single_binding2.rs:49:5
+   |
+LL | /     match match x {
+LL | |         0 => 1,
+LL | |         _ => 2,
+LL | |     } {
+LL | |         _ => println!("Single branch"),
+LL | |     }
+   | |_____^
+   |
+help: consider using the scrutinee and body instead
+   |
+LL |     match x {
+LL |         0 => 1,
+LL |         _ => 2,
+LL |     };
+LL |     println!("Single branch");
+   |
+
+error: aborting due to 4 previous errors