From 9cfc6124a3e09276c75d8998b5a93a29e82213a4 Mon Sep 17 00:00:00 2001
From: mcarton <cartonmartin+git@gmail.com>
Date: Wed, 9 Mar 2016 16:22:31 +0100
Subject: [PATCH] Improve the MATCH_REF_PATS suggestions

---
 src/matches.rs                | 36 +++++++++++++++++++++--------------
 tests/compile-fail/matches.rs | 25 +++++++++++++++++++-----
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/src/matches.rs b/src/matches.rs
index 85a8a4b005d..4c60ea89b34 100644
--- a/src/matches.rs
+++ b/src/matches.rs
@@ -8,7 +8,7 @@ use std::cmp::Ordering;
 use syntax::ast::LitKind;
 use syntax::codemap::Span;
 use utils::{COW_PATH, OPTION_PATH, RESULT_PATH};
-use utils::{match_type, snippet, span_lint, span_note_and_lint, span_lint_and_then, in_external_macro, expr_block};
+use utils::{match_type, snippet, span_note_and_lint, span_lint_and_then, in_external_macro, expr_block};
 
 /// **What it does:** This lint checks for matches with a single arm where an `if let` will usually suffice.
 ///
@@ -309,18 +309,26 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match
     if has_only_ref_pats(arms) {
         if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
             let template = match_template(cx, expr.span, source, "", inner);
-            span_lint(cx,
-                      MATCH_REF_PATS,
-                      expr.span,
-                      &format!("you don't need to add `&` to both the expression and the patterns: use `{}`",
-                               template));
+            span_lint_and_then(cx,
+                               MATCH_REF_PATS,
+                               expr.span,
+                               "you don't need to add `&` to both the expression and the patterns",
+                               |db| {
+                                   db.span_suggestion(expr.span,
+                                                      "try",
+                                                      template);
+                               });
         } else {
             let template = match_template(cx, expr.span, source, "*", ex);
-            span_lint(cx,
-                      MATCH_REF_PATS,
-                      expr.span,
-                      &format!("instead of prefixing all patterns with `&`, you can dereference the expression: `{}`",
-                               template));
+            span_lint_and_then(cx,
+                               MATCH_REF_PATS,
+                               expr.span,
+                               "you don't need to add `&` to all patterns",
+                               |db| {
+                                   db.span_suggestion(expr.span,
+                                                      "instead of prefixing all patterns with `&`, you can dereference the expression",
+                                                      template);
+                               });
         }
     }
 }
@@ -435,9 +443,9 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
 fn match_template(cx: &LateContext, span: Span, source: MatchSource, op: &str, expr: &Expr) -> String {
     let expr_snippet = snippet(cx, expr.span, "..");
     match source {
-        MatchSource::Normal => format!("match {}{} {{ ...", op, expr_snippet),
-        MatchSource::IfLetDesugar { .. } => format!("if let ... = {}{} {{", op, expr_snippet),
-        MatchSource::WhileLetDesugar => format!("while let ... = {}{} {{", op, expr_snippet),
+        MatchSource::Normal => format!("match {}{} {{ .. }}", op, expr_snippet),
+        MatchSource::IfLetDesugar { .. } => format!("if let .. = {}{} {{ .. }}", op, expr_snippet),
+        MatchSource::WhileLetDesugar => format!("while let .. = {}{} {{ .. }}", op, expr_snippet),
         MatchSource::ForLoopDesugar => cx.sess().span_bug(span, "for loop desugared to match with &-patterns!"),
         MatchSource::TryDesugar => cx.sess().span_bug(span, "`?` operator desugared to match with &-patterns!")
     }
diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs
index 46d3ff8d5fb..f5f830fed51 100644
--- a/tests/compile-fail/matches.rs
+++ b/tests/compile-fail/matches.rs
@@ -137,7 +137,10 @@ fn match_bool() {
 fn ref_pats() {
     {
         let v = &Some(0);
-        match v {  //~ERROR dereference the expression: `match *v { ...`
+        match v {
+            //~^ERROR add `&` to all patterns
+            //~|HELP instead of
+            //~|SUGGESTION `match *v { .. }`
             &Some(v) => println!("{:?}", v),
             &None => println!("none"),
         }
@@ -147,13 +150,19 @@ fn ref_pats() {
         }
     }
     let tup =& (1, 2);
-    match tup {  //~ERROR dereference the expression: `match *tup { ...`
+    match tup {
+        //~^ERROR add `&` to all patterns
+        //~|HELP instead of
+        //~|SUGGESTION `match *tup { .. }`
         &(v, 1) => println!("{}", v),
         _ => println!("none"),
     }
     // special case: using & both in expr and pats
     let w = Some(0);
-    match &w {  //~ERROR use `match w { ...`
+    match &w {
+        //~^ERROR add `&` to both
+        //~|HELP try
+        //~|SUGGESTION `match w { .. }`
         &Some(v) => println!("{:?}", v),
         &None => println!("none"),
     }
@@ -164,12 +173,18 @@ fn ref_pats() {
     }
 
     let a = &Some(0);
-    if let &None = a { //~ERROR dereference the expression: `if let ... = *a {`
+    if let &None = a {
+        //~^ERROR add `&` to all patterns
+        //~|HELP instead of
+        //~|SUGGESTION `if let ... = *a { .. }`
         println!("none");
     }
 
     let b = Some(0);
-    if let &None = &b { //~ERROR use `if let ... = b {`
+    if let &None = &b {
+        //~^ERROR add `&` to both
+        //~|HELP try
+        //~|SUGGESTION `if let ... = b { .. }`
         println!("none");
     }
 }