match_single_binding: Fix invalid suggestion when match scrutinee has side effects

7 files changed, 100 insertions, 35 deletions

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 44b4eb29035..345b360e633 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -1479,15 +1479,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,
+                );
+            }
         },
         _ => (),
     }
diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed
index 526e94b10bd..30bf6402253 100644
--- a/tests/ui/match_single_binding.fixed
+++ b/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;
diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs
index 6a2ca7c5e93..d8bb80d8b96 100644
--- a/tests/ui/match_single_binding.rs
+++ b/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;
diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr
index cbbf5d29c02..795c8c3e24d 100644
--- a/tests/ui/match_single_binding.stderr
+++ b/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
 
diff --git a/tests/ui/match_single_binding2.fixed b/tests/ui/match_single_binding2.fixed
index e73a85b73d7..a91fcc2125d 100644
--- a/tests/ui/match_single_binding2.fixed
+++ b/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");
 }
diff --git a/tests/ui/match_single_binding2.rs b/tests/ui/match_single_binding2.rs
index 7362cb390e5..476386ebabe 100644
--- a/tests/ui/match_single_binding2.rs
+++ b/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"),
+    }
 }
diff --git a/tests/ui/match_single_binding2.stderr b/tests/ui/match_single_binding2.stderr
index bc18d191aa3..4372f55af87 100644
--- a/tests/ui/match_single_binding2.stderr
+++ b/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