Use `utils::sugg` in `match` related lints

Also don't build suggestion when unnecessary.

diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs
index 94b36d28ed35d977a26ee5c0e095e11d281adb50..de21cabc6fb422d3b83277073581d62881151fd1 100644 (file)
--- a/clippy_lints/src/matches.rs
+++ b/clippy_lints/src/matches.rs
@@ -235,57 +235,54 @@ fn check_single_match_opt_like(cx: &LateContext, ex: &Expr, arms: &[Arm], expr:
 fn check_match_bool(cx: &LateContext, ex: &Expr, arms: &[Arm], expr: &Expr) {
     // type of expression == bool
     if cx.tcx.expr_ty(ex).sty == ty::TyBool {
-        let sugg = if arms.len() == 2 && arms[0].pats.len() == 1 {
-            // no guards
-            let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
-                if let ExprLit(ref lit) = arm_bool.node {
-                    match lit.node {
-                        LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
-                        LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
-                        _ => None,
+        span_lint_and_then(cx,
+                           MATCH_BOOL,
+                           expr.span,
+                           "you seem to be trying to match on a boolean expression",
+                           move |db| {
+            if arms.len() == 2 && arms[0].pats.len() == 1 {
+                // no guards
+                let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node {
+                    if let ExprLit(ref lit) = arm_bool.node {
+                        match lit.node {
+                            LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)),
+                            LitKind::Bool(false) => Some((&*arms[1].body, &*arms[0].body)),
+                            _ => None,
+                        }
+                    } else {
+                        None
                     }
                 } else {
                     None
-                }
-            } else {
-                None
-            };
-
-            if let Some((ref true_expr, ref false_expr)) = exprs {
-                match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
-                    (false, false) => {
-                        Some(format!("if {} {} else {}",
-                                     snippet(cx, ex.span, "b"),
-                                     expr_block(cx, true_expr, None, ".."),
-                                     expr_block(cx, false_expr, None, "..")))
+                };
+
+                if let Some((ref true_expr, ref false_expr)) = exprs {
+                    let sugg = match (is_unit_expr(true_expr), is_unit_expr(false_expr)) {
+                        (false, false) => {
+                            Some(format!("if {} {} else {}",
+                                         snippet(cx, ex.span, "b"),
+                                         expr_block(cx, true_expr, None, ".."),
+                                         expr_block(cx, false_expr, None, "..")))
+                        }
+                        (false, true) => {
+                            Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
+                        }
+                        (true, false) => {
+                            let test = Sugg::hir(cx, ex, "..");
+                            Some(format!("if {} {}",
+                                         !test,
+                                         expr_block(cx, false_expr, None, "..")))
+                        }
+                        (true, true) => None,
+                    };
+
+                    if let Some(sugg) = sugg {
+                        db.span_suggestion(expr.span, "consider using an if/else expression", sugg);
                     }
-                    (false, true) => {
-                        Some(format!("if {} {}", snippet(cx, ex.span, "b"), expr_block(cx, true_expr, None, "..")))
-                    }
-                    (true, false) => {
-                        let test = Sugg::hir(cx, ex, "..");
-                        Some(format!("if {} {}",
-                                     !test,
-                                     expr_block(cx, false_expr, None, "..")))
-                    }
-                    (true, true) => None,
                 }
-            } else {
-                None
             }
-        } else {
-            None
-        };
 
-        span_lint_and_then(cx,
-                           MATCH_BOOL,
-                           expr.span,
-                           "you seem to be trying to match on a boolean expression. Consider using an if..else block:",
-                           move |db| {
-                               if let Some(sugg) = sugg {
-                                   db.span_suggestion(expr.span, "try this", sugg);
-                               }
-                           });
+       });
     }
 }
 
@@ -309,26 +306,28 @@ fn check_overlapping_arms(cx: &LateContext, ex: &Expr, arms: &[Arm]) {
 fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: MatchSource, expr: &Expr) {
     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_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);
-                               });
+                let inner = Sugg::hir(cx, inner, "..");
+                let template = match_template(expr.span, source, inner);
+                db.span_suggestion(expr.span, "try", template);
+            });
         } else {
-            let template = match_template(cx, expr.span, source, "*", ex);
             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);
-                               });
+                let ex = Sugg::hir(cx, ex, "..");
+                let template = match_template(expr.span, source, ex.deref());
+                db.span_suggestion(expr.span,
+                                   "instead of prefixing all patterns with `&`, you can \
+                                   dereference the expression",
+                                   template);
+            });
         }
     }
 }
@@ -411,12 +410,11 @@ fn has_only_ref_pats(arms: &[Arm]) -> bool {
     mapped.map_or(false, |v| v.iter().any(|el| *el))
 }
 
-fn match_template(cx: &LateContext, span: Span, source: MatchSource, op: &str, expr: &Expr) -> String {
-    let expr_snippet = snippet(cx, expr.span, "..");
+fn match_template(span: Span, source: MatchSource, expr: Sugg) -> String {
     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 {} {{ .. }}", expr),
+        MatchSource::IfLetDesugar { .. } => format!("if let .. = {} {{ .. }}", expr),
+        MatchSource::WhileLetDesugar => format!("while let .. = {} {{ .. }}", expr),
         MatchSource::ForLoopDesugar => span_bug!(span, "for loop desugared to match with &-patterns!"),
         MatchSource::TryDesugar => span_bug!(span, "`?` operator desugared to match with &-patterns!"),
     }

diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs
index bbfa8648c531dbc62ef2827c56a26860dcd208c5..f857c821e7ba2ee2b789016b8385b102a25dfb2d 100644 (file)
--- a/clippy_lints/src/utils/sugg.rs
+++ b/clippy_lints/src/utils/sugg.rs
@@ -134,6 +134,11 @@ pub fn mut_addr(self) -> Sugg<'static> {
         make_unop("&mut ", self)
     }
 
+    /// Convenience method to create the `*<expr>` suggestion.
+    pub fn deref(self) -> Sugg<'static> {
+        make_unop("*", self)
+    }
+
     /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>` suggestion.
     pub fn range(self, end: Self, limit: ast::RangeLimits) -> Sugg<'static> {
         match limit {

diff --git a/tests/compile-fail/matches.rs b/tests/compile-fail/matches.rs
index e49aeaa6ec8622369055511284067dcad152c33d..f64cceb5c34711f6c7f6ae0246ad76cd8c93d5a8 100644 (file)
--- a/tests/compile-fail/matches.rs
+++ b/tests/compile-fail/matches.rs
@@ -112,7 +112,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if test { 0 } else { 42 };
         true => 0,
         false => 42,
@@ -121,7 +121,7 @@ fn match_bool() {
     let option = 1;
     match option == 1 {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if option == 1 { 1 } else { 0 };
         true => 1,
         false => 0,
@@ -129,7 +129,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if !test { println!("Noooo!"); };
         true => (),
         false => { println!("Noooo!"); }
@@ -137,7 +137,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if !test { println!("Noooo!"); };
         false => { println!("Noooo!"); }
         _ => (),
@@ -145,7 +145,7 @@ fn match_bool() {
 
     match test && test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if !(test && test) { println!("Noooo!"); };
     //~| ERROR equal expressions as operands
         false => { println!("Noooo!"); }
@@ -154,7 +154,7 @@ fn match_bool() {
 
     match test {
     //~^ ERROR you seem to be trying to match on a boolean expression
-    //~| HELP try
+    //~| HELP consider
     //~| SUGGESTION if test { println!("Yes!"); } else { println!("Noooo!"); };
         false => { println!("Noooo!"); }
         true => { println!("Yes!"); }