Fixups from review comments

1. Moved common check `in_external_macro` to the top of function from inside each conditionals. 2. Inlined `is_bool_expr` call
rust-lang · Oct 20, 2015 · 5e78fbb · 5e78fbb
1 parent 675c532
commit 5e78fbb
Showing 1 changed file with 4 additions and 9 deletions.
diff --git a/src/matches.rs b/src/matches.rs
@@ -25,6 +25,8 @@ impl LintPass for MatchPass {
 impl LateLintPass for MatchPass {
     fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
         if let ExprMatch(ref ex, ref arms, MatchSource::Normal) = expr.node {
+            if in_external_macro(cx, expr.span) { return; }
+
             // check preconditions for SINGLE_MATCH
                 // only two arms
             if arms.len() == 2 &&
@@ -39,7 +41,6 @@ impl LateLintPass for MatchPass {
                 // finally, we don't want any content in the second arm (unit or empty block)
                 is_unit_expr(&arms[1].body)
             {
-                if in_external_macro(cx, expr.span) {return;}
                 span_help_and_lint(cx, SINGLE_MATCH, expr.span,
                                    "you seem to be trying to use match for destructuring a \
                                     single pattern. Consider using `if let`",
@@ -51,7 +52,6 @@ impl LateLintPass for MatchPass {
 
             // check preconditions for MATCH_REF_PATS
             if has_only_ref_pats(arms) {
-                if in_external_macro(cx, expr.span) { return; }
                 if let ExprAddrOf(Mutability::MutImmutable, ref inner) = ex.node {
                     span_lint(cx, MATCH_REF_PATS, expr.span, &format!(
                         "you don't need to add `&` to both the expression to match \
@@ -65,12 +65,11 @@ impl LateLintPass for MatchPass {
 
             // check preconditions for MATCH_BOOL
             // type of expression == bool
-            if is_bool_expr(cx, ex) {
-                if in_external_macro(cx, expr.span) { return; }
+            if cx.tcx.expr_ty(ex).sty == ty::TyBool {
 
                 span_lint(cx, MATCH_BOOL, expr.span,
                                    "you seem to be trying to match on a boolean expression. \
-                                   Consider using if..else block");
+                                   Consider using an if..else block");
             }
         }
     }
@@ -84,10 +83,6 @@ fn is_unit_expr(expr: &Expr) -> bool {
     }
 }
 
-fn is_bool_expr(cx: &LateContext, ex: &Expr ) -> bool {
-    cx.tcx.expr_ty(ex).sty == ty::TyBool
-}
-
 fn has_only_ref_pats(arms: &[Arm]) -> bool {
     let mapped = arms.iter().flat_map(|a| &a.pats).map(|p| match p.node {
         PatRegion(..) => Some(true),  // &-patterns