Add wild and struct handling

7 files changed, 272 insertions, 36 deletions

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index e4335abcea2..2323df1f0fc 100644
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -3,9 +3,9 @@ use crate::utils::paths;
 use crate::utils::sugg::Sugg;
 use crate::utils::usage::is_unused;
 use crate::utils::{
-    span_lint_and_help, span_lint_and_note, 
-    expr_block, in_macro, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks,
-    snippet, snippet_block, snippet_with_applicability,  span_lint_and_sugg, span_lint_and_then,
+    expr_block, get_arg_name, in_macro, is_allowed, is_expn_of, is_refutable, is_wild, match_qpath, match_type,
+    match_var, multispan_sugg, remove_blocks, snippet, snippet_block, snippet_with_applicability, span_lint_and_help,
+    span_lint_and_note, span_lint_and_sugg, span_lint_and_then, walk_ptrs_ty,
 };
 use if_chain::if_chain;
 use rustc::lint::in_external_macro;
@@ -822,36 +822,66 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) {
 }
 
 fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
-    if in_macro(expr.span) {
+    if in_macro(expr.span) || arms.len() != 1 || is_refutable(cx, arms[0].pat) {
         return;
     }
-    if arms.len() == 1 {
-        if is_refutable(cx, arms[0].pat) {
-            return;
-        }
-        match arms[0].pat.kind {
-            PatKind::Binding(..) | PatKind::Tuple(_, _) => {
-                let bind_names = arms[0].pat.span;
-                let matched_vars = ex.span;
-                let match_body = remove_blocks(&arms[0].body);
-                span_lint_and_sugg(
-                    cx,
-                    MATCH_SINGLE_BINDING,
-                    expr.span,
-                    "this match could be written as a `let` statement",
-                    "consider using `let` statement",
-                    format!(
-                        "let {} = {};\n{}",
-                        snippet(cx, bind_names, ".."),
-                        snippet(cx, matched_vars, ".."),
-                        snippet_block(cx, match_body.span, "..")
-                    ),
-                    Applicability::MachineApplicable,
-                );
-            },
-            _ => (),
+    let matched_vars = ex.span;
+    let bind_names = arms[0].pat.span;
+    let match_body = remove_blocks(&arms[0].body);
+    let mut snippet_body = if match_body.span.from_expansion() {
+        Sugg::hir_with_macro_callsite(cx, match_body, "..").to_string()
+    } else {
+        snippet_block(cx, match_body.span, "..").to_owned().to_string()
+    };
+
+    // Do we need to add ';' to suggestion ?
+    if_chain! {
+        if let ExprKind::Block(block, _) = &arms[0].body.kind;
+        if block.stmts.len() == 1;
+        if let StmtKind::Semi(s) = block.stmts.get(0).unwrap().kind;
+        then {
+            match s.kind {
+                ExprKind::Block(_, _) => (),
+                _ => {
+                    // expr_ty(body) == ()
+                    if cx.tables.expr_ty(&arms[0].body).is_unit() {
+                        snippet_body.push(';');
+                    }
+                }
+            }
         }
     }
+
+    match arms[0].pat.kind {
+        PatKind::Binding(..) | PatKind::Tuple(_, _) | PatKind::Struct(..) => {
+            span_lint_and_sugg(
+                cx,
+                MATCH_SINGLE_BINDING,
+                expr.span,
+                "this match could be written as a `let` statement",
+                "consider using `let` statement",
+                format!(
+                    "let {} = {};\n{}",
+                    snippet(cx, bind_names, ".."),
+                    snippet(cx, matched_vars, ".."),
+                    snippet_body
+                ),
+                Applicability::MachineApplicable,
+            );
+        },
+        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,
+            );
+        },
+        _ => (),
+    }
 }
 
 /// Gets all arms that are unbounded `PatRange`s.
diff --git a/tests/ui/escape_analysis.rs b/tests/ui/escape_analysis.rs
index 7d4b71d09dc..c0a52d832c0 100644
--- a/tests/ui/escape_analysis.rs
+++ b/tests/ui/escape_analysis.rs
@@ -4,6 +4,7 @@
     clippy::needless_pass_by_value,
     clippy::unused_unit,
     clippy::redundant_clone,
+    clippy::match_single_binding
 )]
 #![warn(clippy::boxed_local)]
 
diff --git a/tests/ui/match_ref_pats.rs b/tests/ui/match_ref_pats.rs
index d26b59db91b..5de43733ad3 100644
--- a/tests/ui/match_ref_pats.rs
+++ b/tests/ui/match_ref_pats.rs
@@ -26,6 +26,7 @@ fn ref_pats() {
     }
     // False positive: only wildcard pattern.
     let w = Some(0);
+    #[allow(clippy::match_single_binding)]
     match w {
         _ => println!("none"),
     }
diff --git a/tests/ui/match_ref_pats.stderr b/tests/ui/match_ref_pats.stderr
index 492f4b9ba20..52cb4a14b72 100644
--- a/tests/ui/match_ref_pats.stderr
+++ b/tests/ui/match_ref_pats.stderr
@@ -47,7 +47,7 @@ LL |         None => println!("none"),
    |
 
 error: you don't need to add `&` to all patterns
-  --> $DIR/match_ref_pats.rs:34:5
+  --> $DIR/match_ref_pats.rs:35:5
    |
 LL | /     if let &None = a {
 LL | |         println!("none");
@@ -60,7 +60,7 @@ LL |     if let None = *a {
    |            ^^^^   ^^
 
 error: you don't need to add `&` to both the expression and the patterns
-  --> $DIR/match_ref_pats.rs:39:5
+  --> $DIR/match_ref_pats.rs:40:5
    |
 LL | /     if let &None = &b {
 LL | |         println!("none");
@@ -73,7 +73,7 @@ LL |     if let None = b {
    |            ^^^^   ^
 
 error: you don't need to add `&` to all patterns
-  --> $DIR/match_ref_pats.rs:66:9
+  --> $DIR/match_ref_pats.rs:67:9
    |
 LL | /         match foo_variant!(0) {
 LL | |             &Foo::A => println!("A"),
diff --git a/tests/ui/match_single_binding.fixed b/tests/ui/match_single_binding.fixed
index 41faa1e1c21..8fb8dc323e4 100644
--- a/tests/ui/match_single_binding.fixed
+++ b/tests/ui/match_single_binding.fixed
@@ -1,7 +1,12 @@
 // run-rustfix
 
 #![warn(clippy::match_single_binding)]
-#[allow(clippy::many_single_char_names)]
+#![allow(clippy::many_single_char_names, clippy::toplevel_ref_arg)]
+
+struct Point {
+    x: i32,
+    y: i32,
+}
 
 fn main() {
     let a = 1;
@@ -12,6 +17,9 @@ fn main() {
 {
     println!("{} {} {}", x, y, z);
 }
+    // Lint
+    let (x, y, z) = (a, b, c);
+println!("{} {} {}", x, y, z);
     // Ok
     match a {
         2 => println!("2"),
@@ -23,4 +31,33 @@ fn main() {
         Some(d) => println!("{}", d),
         _ => println!("None"),
     }
+    // Lint
+    println!("whatever");
+    // Lint
+    {
+    let x = 29;
+    println!("x has a value of {}", x);
+}
+    // Lint
+    {
+    let e = 5 * a;
+    if e >= 5 {
+        println!("e is superior to 5");
+    }
+}
+    // Lint
+    let p = Point { x: 0, y: 7 };
+    let Point { x, y } = p;
+println!("Coords: ({}, {})", x, y);
+    // Lint
+    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);
+    // Lint
+    let mut x = 5;
+    let ref mut mr = x;
+println!("Got a mutable reference to {}", mr);
 }
diff --git a/tests/ui/match_single_binding.rs b/tests/ui/match_single_binding.rs
index 06b924d0471..55b0b09a008 100644
--- a/tests/ui/match_single_binding.rs
+++ b/tests/ui/match_single_binding.rs
@@ -1,7 +1,12 @@
 // run-rustfix
 
 #![warn(clippy::match_single_binding)]
-#[allow(clippy::many_single_char_names)]
+#![allow(clippy::many_single_char_names, clippy::toplevel_ref_arg)]
+
+struct Point {
+    x: i32,
+    y: i32,
+}
 
 fn main() {
     let a = 1;
@@ -13,6 +18,10 @@ fn main() {
             println!("{} {} {}", x, y, z);
         },
     }
+    // Lint
+    match (a, b, c) {
+        (x, y, z) => println!("{} {} {}", x, y, z),
+    }
     // Ok
     match a {
         2 => println!("2"),
@@ -24,4 +33,43 @@ fn main() {
         Some(d) => println!("{}", d),
         _ => println!("None"),
     }
+    // Lint
+    match a {
+        _ => println!("whatever"),
+    }
+    // Lint
+    match a {
+        _ => {
+            let x = 29;
+            println!("x has a value of {}", x);
+        },
+    }
+    // Lint
+    match a {
+        _ => {
+            let e = 5 * a;
+            if e >= 5 {
+                println!("e is superior to 5");
+            }
+        },
+    }
+    // Lint
+    let p = Point { x: 0, y: 7 };
+    match p {
+        Point { x, y } => println!("Coords: ({}, {})", x, y),
+    }
+    // Lint
+    match p {
+        Point { x: x1, y: y1 } => println!("Coords: ({}, {})", x1, y1),
+    }
+    // Lint
+    let x = 5;
+    match x {
+        ref r => println!("Got a reference to {}", r),
+    }
+    // Lint
+    let mut x = 5;
+    match x {
+        ref mut mr => println!("Got a mutable reference to {}", mr),
+    }
 }
diff --git a/tests/ui/match_single_binding.stderr b/tests/ui/match_single_binding.stderr
index 64216a72ef7..d76e229adff 100644
--- a/tests/ui/match_single_binding.stderr
+++ b/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:11:5
+  --> $DIR/match_single_binding.rs:16:5
    |
 LL | /     match (a, b, c) {
 LL | |         (x, y, z) => {
@@ -17,5 +17,124 @@ LL |     println!("{} {} {}", x, y, z);
 LL | }
    |
 
-error: aborting due to previous error
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:22:5
+   |
+LL | /     match (a, b, c) {
+LL | |         (x, y, z) => println!("{} {} {}", x, y, z),
+LL | |     }
+   | |_____^
+   |
+help: consider using `let` statement
+   |
+LL |     let (x, y, z) = (a, b, c);
+LL | println!("{} {} {}", x, y, z);
+   |
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:37:5
+   |
+LL | /     match a {
+LL | |         _ => println!("whatever"),
+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
+   |
+LL | /     match a {
+LL | |         _ => {
+LL | |             let x = 29;
+LL | |             println!("x has a value of {}", x);
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: consider using the match body instead
+   |
+LL |     {
+LL |     let x = 29;
+LL |     println!("x has a value of {}", x);
+LL | }
+   |
+
+error: this match could be replaced by its body itself
+  --> $DIR/match_single_binding.rs:48:5
+   |
+LL | /     match a {
+LL | |         _ => {
+LL | |             let e = 5 * a;
+LL | |             if e >= 5 {
+...  |
+LL | |         },
+LL | |     }
+   | |_____^
+   |
+help: consider using the match body instead
+   |
+LL |     {
+LL |     let e = 5 * a;
+LL |     if e >= 5 {
+LL |         println!("e is superior to 5");
+LL |     }
+LL | }
+   |
+
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:58:5
+   |
+LL | /     match p {
+LL | |         Point { x, y } => println!("Coords: ({}, {})", x, y),
+LL | |     }
+   | |_____^
+   |
+help: consider using `let` statement
+   |
+LL |     let Point { x, y } = p;
+LL | println!("Coords: ({}, {})", x, y);
+   |
+
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:62:5
+   |
+LL | /     match p {
+LL | |         Point { x: x1, y: y1 } => println!("Coords: ({}, {})", x1, y1),
+LL | |     }
+   | |_____^
+   |
+help: consider using `let` statement
+   |
+LL |     let Point { x: x1, y: y1 } = p;
+LL | println!("Coords: ({}, {})", x1, y1);
+   |
+
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:67:5
+   |
+LL | /     match x {
+LL | |         ref r => println!("Got a reference to {}", r),
+LL | |     }
+   | |_____^
+   |
+help: consider using `let` statement
+   |
+LL |     let ref r = x;
+LL | println!("Got a reference to {}", r);
+   |
+
+error: this match could be written as a `let` statement
+  --> $DIR/match_single_binding.rs:72:5
+   |
+LL | /     match x {
+LL | |         ref mut mr => println!("Got a mutable reference to {}", mr),
+LL | |     }
+   | |_____^
+   |
+help: consider using `let` statement
+   |
+LL |     let ref mut mr = x;
+LL | println!("Got a mutable reference to {}", mr);
+   |
+
+error: aborting due to 9 previous errors