misc: fix check for unit body in "match -> if let" lint (fixes #172)

src/misc.rs

@@ -6,6 +6,7 @@ use syntax::visit::{FnKind};
 use rustc::lint::{Context, LintPass, LintArray, Lint, Level};
 use rustc::middle::ty;
 use syntax::codemap::{Span, Spanned};
+use std::borrow::Cow;
 
 use utils::{match_path, snippet, snippet_block, span_lint, span_help_and_lint, walk_ptrs_ty};
 
@@ -26,39 +27,47 @@ impl LintPass for MiscPass {
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
         if let ExprMatch(ref ex, ref arms, ast::MatchSource::Normal) = expr.node {
-            if arms.len() == 2 {
+            // check preconditions: only two arms
-                if arms[0].guard.is_none() && arms[1].pats.len() == 1 {
+            if arms.len() == 2 &&
-                    match arms[1].body.node {
+                // both of the arms have a single pattern and no guard
-                        ExprTup(ref v) if v.is_empty() && arms[1].guard.is_none() => (),
+                arms[0].pats.len() == 1 && arms[0].guard.is_none() &&
-                        ExprBlock(ref b) if b.stmts.is_empty() && arms[1].guard.is_none() => (),
+                arms[1].pats.len() == 1 && arms[1].guard.is_none() &&
-                         _ => return
+                // and the second pattern is a `_` wildcard: this is not strictly necessary,
-                    }
+                // since the exhaustiveness check will ensure the last one is a catch-all,
-                    // In some cases, an exhaustive match is preferred to catch situations when
+                // but in some cases, an explicit match is preferred to catch situations
-                    // an enum is extended. So we only consider cases where a `_` wildcard is used
+                // when an enum is extended, so we don't consider these cases
-                    if arms[1].pats[0].node == PatWild(PatWildSingle) &&
+                arms[1].pats[0].node == PatWild(PatWildSingle) &&
-                            arms[0].pats.len() == 1 {
+                // finally, we don't want any content in the second arm (unit or empty block)
-                        let body_code = snippet_block(cx, arms[0].body.span, "..");
+                is_unit_expr(&*arms[1].body)
-                        let suggestion = if let ExprBlock(_) = arms[0].body.node {
+            {
-                            body_code.into_owned()
+                let body_code = snippet_block(cx, arms[0].body.span, "..");
-                        } else {
+                let body_code = if let ExprBlock(_) = arms[0].body.node {
-                            format!("{{ {} }}", body_code)
+                    body_code
-                        };
+                } else {
-                        span_help_and_lint(cx, SINGLE_MATCH, expr.span,
+                    Cow::Owned(format!("{{ {} }}", body_code))
-                              "you seem to be trying to use match for \
+                };
-                              destructuring a single pattern. Did you mean to \
+                span_help_and_lint(cx, SINGLE_MATCH, expr.span,
-                              use `if let`?",
+                      "you seem to be trying to use match for \
-                              &*format!("try\nif let {} = {} {}",
+                      destructuring a single pattern. Did you mean to \
-                                        snippet(cx, arms[0].pats[0].span, ".."),
+                      use `if let`?",
-                                        snippet(cx, ex.span, ".."),
+                      &*format!("try\nif let {} = {} {}",
-                                        suggestion)
+                                snippet(cx, arms[0].pats[0].span, ".."),
-                        );
+                                snippet(cx, ex.span, ".."),
-                    }
+                                body_code)
-                }
+                );
             }
         }
     }
 }
 
+fn is_unit_expr(expr: &Expr) -> bool {
+    match expr.node {
+        ExprTup(ref v) if v.is_empty() => true,
+        ExprBlock(ref b) if b.stmts.is_empty() && b.expr.is_none() => true,
+        _ => false,
+    }
+}
+
 
 declare_lint!(pub TOPLEVEL_REF_ARG, Warn,
               "a function argument is declared `ref` (i.e. `fn foo(ref x: u8)`, but not \

tests/compile-fail/match_if_let.rs

@@ -23,4 +23,16 @@ fn main(){
         (2...3, 7...9) => println!("{:?}", z),
         _ => {}
     }
+
+    // Not linted (pattern guards used)
+    match x {
+        Some(y) if y == 0 => println!("{:?}", y),
+        _ => ()
+    }
+
+    // Not linted (content in the else)
+    match z {
+        (2...3, 7...9) => println!("{:?}", z),
+        _ => println!("nope"),
+    }
 }