Merge pull request #501 from oli-obk/fix/seme

fix cyclomatic complexity lint (fixes #491, fixes #478)

src/cyclomatic_complexity.rs

@@ -3,6 +3,7 @@
 use rustc::lint::*;
 use rustc_front::hir::*;
 use rustc::middle::cfg::CFG;
+use rustc::middle::ty;
 use syntax::codemap::Span;
 use syntax::attr::*;
 use syntax::ast::Attribute;
@@ -32,7 +33,7 @@ impl LintPass for CyclomaticComplexity {
 }
 
 impl CyclomaticComplexity {
-    fn check(&mut self, cx: &LateContext, block: &Block, span: Span) {
+    fn check<'a, 'tcx>(&mut self, cx: &'a LateContext<'a, 'tcx>, block: &Block, span: Span) {
         if in_macro(cx, span) { return; }
         let cfg = CFG::new(cx.tcx, block);
         let n = cfg.graph.len_nodes() as u64;
@@ -40,15 +41,16 @@ impl CyclomaticComplexity {
         let cc = e + 2 - n;
         let mut arm_counter = MatchArmCounter(0);
         arm_counter.visit_block(block);
-        let mut narms = arm_counter.0;
-        if narms > 0 {
-            narms = narms - 1;
-        }
-        
-        if cc < narms {
-            report_cc_bug(cx, cc, narms, span);
+        let narms = arm_counter.0;
+
+        let mut diverge_counter = DivergenceCounter(0, &cx.tcx);
+        diverge_counter.visit_block(block);
+        let divergence = diverge_counter.0;
+
+        if cc + divergence < narms {
+            report_cc_bug(cx, cc, narms, divergence, span);
         } else {
-            let rust_cc = cc - narms;
+            let rust_cc = cc + divergence - narms;
             if rust_cc > self.limit.limit() {
                 cx.span_lint_help(CYCLOMATIC_COMPLEXITY, span,
                 &format!("The function has a cyclomatic complexity of {}.", rust_cc),
@@ -93,8 +95,28 @@ impl<'a> Visitor<'a> for MatchArmCounter {
             ExprMatch(_, ref arms, _) => {
                 walk_expr(self, e);
                 let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum();
-                if arms_n > 0 {
-                    self.0 += arms_n - 1;
+                if arms_n > 1 {
+                    self.0 += arms_n - 2;
+                }
+            },
+            ExprClosure(..) => {},
+            _ => walk_expr(self, e),
+        }
+    }
+}
+
+struct DivergenceCounter<'a, 'tcx: 'a>(u64, &'a ty::ctxt<'tcx>);
+
+impl<'a, 'b, 'tcx> Visitor<'a> for DivergenceCounter<'b, 'tcx> {
+    fn visit_expr(&mut self, e: &'a Expr) {
+        match e.node {
+            ExprCall(ref callee, _) => {
+                walk_expr(self, e);
+                let ty = self.1.node_id_to_type(callee.id);
+                if let ty::TyBareFn(_, ty) = ty.sty {
+                    if ty.sig.skip_binder().output.diverges() {
+                        self.0 += 1;
+                    }
                 }
             },
             ExprClosure(..) => {},
@@ -104,15 +126,15 @@ impl<'a> Visitor<'a> for MatchArmCounter {
 }
 
 #[cfg(feature="debugging")]
-fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, span: Span) {
+fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) {
     cx.sess().span_bug(span, &format!("Clippy encountered a bug calculating cyclomatic complexity: \
-                                       cc = {}, arms = {}. Please file a bug report.", cc, narms));;
+                                       cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));;
 }
 #[cfg(not(feature="debugging"))]
-fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, span: Span) {
+fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) {
     if cx.current_level(CYCLOMATIC_COMPLEXITY) != Level::Allow {
         cx.sess().span_note(span, &format!("Clippy encountered a bug calculating cyclomatic complexity \
                                             (hide this message with `#[allow(cyclomatic_complexity)]`): \
-                                            cc = {}, arms = {}. Please file a bug report.", cc, narms));
+                                            cc = {}, arms = {}, div = {}. Please file a bug report.", cc, narms, div));
     }
 }

src/loops.rs

@@ -147,14 +147,8 @@ impl LateLintPass for LoopsPass {
 
             // extract the expression from the first statement (if any) in a block
             let inner_stmt_expr = extract_expr_from_first_stmt(block);
-            // extract the first expression (if any) from the block
-            let inner_expr = extract_first_expr(block);
-            let (extracted, collect_expr) = match inner_stmt_expr {
-                Some(_) => (inner_stmt_expr, true),     // check if an expression exists in the first statement
-                None => (inner_expr, false),    // if not, let's go for the first expression in the block
-            };
-
-            if let Some(inner) = extracted {
+            // or extract the first expression (if any) from the block
+            if let Some(inner) = inner_stmt_expr.or_else(|| extract_first_expr(block)) {
                 if let ExprMatch(ref matchexpr, ref arms, ref source) = inner.node {
                     // collect the remaining statements below the match
                     let mut other_stuff = block.stmts
@@ -163,10 +157,11 @@ impl LateLintPass for LoopsPass {
                                   .map(|stmt| {
                                       format!("{}", snippet(cx, stmt.span, ".."))
                                   }).collect::<Vec<String>>();
-                    if collect_expr {           // if we have a statement which has a match,
-                        match block.expr {      // then collect the expression (without semicolon) below it
-                            Some(ref expr) => other_stuff.push(format!("{}", snippet(cx, expr.span, ".."))),
-                            None => (),
+                    if inner_stmt_expr.is_some() {
+                        // if we have a statement which has a match,
+                        if let Some(ref expr) = block.expr {
+                            // then collect the expression (without semicolon) below it
+                            other_stuff.push(format!("{}", snippet(cx, expr.span, "..")));
                         }
                     }
 
@@ -180,12 +175,12 @@ impl LateLintPass for LoopsPass {
                             is_break_expr(&arms[1].body)
                         {
                             if in_external_macro(cx, expr.span) { return; }
-                            let loop_body = match inner_stmt_expr {
+                            let loop_body = if inner_stmt_expr.is_some() {
                                 // FIXME: should probably be an ellipsis
                                 // tabbing and newline is probably a bad idea, especially for large blocks
-                                Some(_) => Cow::Owned(format!("{{\n    {}\n}}", other_stuff.join("\n    "))),
-                                None => expr_block(cx, &arms[0].body,
-                                                   Some(other_stuff.join("\n    ")), ".."),
+                                Cow::Owned(format!("{{\n    {}\n}}", other_stuff.join("\n    ")))
+                            } else {
+                                expr_block(cx, &arms[0].body, Some(other_stuff.join("\n    ")), "..")
                             };
                             span_help_and_lint(cx, WHILE_LET_LOOP, expr.span,
                                                "this loop could be written as a `while let` loop",

src/matches.rs

@@ -102,13 +102,11 @@ impl LateLintPass for MatchPass {
                 if arms.len() == 2 && arms[0].pats.len() == 1 { // no guards
                     let exprs = if let PatLit(ref arm_bool) = arms[0].pats[0].node {
                         if let ExprLit(ref lit) = arm_bool.node {
-                            if let LitBool(val) = lit.node {
-                                if val {
-                                    Some((&*arms[0].body, &*arms[1].body))
-                                } else {
-                                    Some((&*arms[1].body, &*arms[0].body))
-                                }
-                            } else { None }
+                            match lit.node {
+                                LitBool(true) => Some((&*arms[0].body, &*arms[1].body)),
+                                LitBool(false) => Some((&*arms[1].body, &*arms[0].body)),
+                                _ => None,
+                            }
                         } else { None }
                     } else { None };
                     if let Some((ref true_expr, ref false_expr)) = exprs {

tests/cc_seme.rs

@@ -0,0 +1,25 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#[allow(dead_code)]
+enum Baz {
+    Baz1,
+    Baz2,
+}
+
+struct Test {
+    t: Option<usize>,
+    b: Baz,
+}
+
+fn main() {
+    use Baz::*;
+    let x = Test { t: Some(0), b: Baz1 };
+
+    match x {
+        Test { t: Some(_), b: Baz1 } => unreachable!(),
+        Test { t: Some(42), b: Baz2 } => unreachable!(),
+        Test { t: None, .. } => unreachable!(),
+        Test { .. } => unreachable!(),
+    }
+}

tests/compile-fail/cyclomatic_complexity.rs

@@ -89,7 +89,7 @@ fn main() { //~ ERROR: The function has a cyclomatic complexity of 28.
 }
 
 #[cyclomatic_complexity = "0"]
-fn kaboom() {  //~ ERROR: The function has a cyclomatic complexity of 6
+fn kaboom() {  //~ ERROR: The function has a cyclomatic complexity of 8
     let n = 0;
     'a: for i in 0..20 {
         'b: for j in i..20 {
@@ -170,6 +170,114 @@ fn barr() { //~ ERROR: The function has a cyclomatic complexity of 2
     }
 }
 
+#[cyclomatic_complexity = "0"]
+fn barr2() { //~ ERROR: The function has a cyclomatic complexity of 3
+    match 99 {
+        0 => println!("hi"),
+        1 => println!("bla"),
+        2 | 3 => println!("blub"),
+        _ => println!("bye"),
+    }
+    match 99 {
+        0 => println!("hi"),
+        1 => println!("bla"),
+        2 | 3 => println!("blub"),
+        _ => println!("bye"),
+    }
+}
+
+#[cyclomatic_complexity = "0"]
+fn barrr() { //~ ERROR: The function has a cyclomatic complexity of 2
+    match 99 {
+        0 => println!("hi"),
+        1 => panic!("bla"),
+        2 | 3 => println!("blub"),
+        _ => println!("bye"),
+    }
+}
+
+#[cyclomatic_complexity = "0"]
+fn barrr2() { //~ ERROR: The function has a cyclomatic complexity of 3
+    match 99 {
+        0 => println!("hi"),
+        1 => panic!("bla"),
+        2 | 3 => println!("blub"),
+        _ => println!("bye"),
+    }
+    match 99 {
+        0 => println!("hi"),
+        1 => panic!("bla"),
+        2 | 3 => println!("blub"),
+        _ => println!("bye"),
+    }
+}
+
+#[cyclomatic_complexity = "0"]
+fn barrrr() { //~ ERROR: The function has a cyclomatic complexity of 2
+    match 99 {
+        0 => println!("hi"),
+        1 => println!("bla"),
+        2 | 3 => panic!("blub"),
+        _ => println!("bye"),
+    }
+}
+
+#[cyclomatic_complexity = "0"]
+fn barrrr2() { //~ ERROR: The function has a cyclomatic complexity of 3
+    match 99 {
+        0 => println!("hi"),
+        1 => println!("bla"),
+        2 | 3 => panic!("blub"),
+        _ => println!("bye"),
+    }
+    match 99 {
+        0 => println!("hi"),
+        1 => println!("bla"),
+        2 | 3 => panic!("blub"),
+        _ => println!("bye"),
+    }
+}
+
+#[cyclomatic_complexity = "0"]
+fn cake() { //~ ERROR: The function has a cyclomatic complexity of 2
+    if 4 == 5 {
+        println!("yea");
+    } else {
+        panic!("meh");
+    }
+    println!("whee");
+}
+
+
+#[cyclomatic_complexity = "0"]
+pub fn read_file(input_path: &str) -> String { //~ ERROR: The function has a cyclomatic complexity of 4
+    use std::fs::File;
+    use std::io::{Read, Write};
+    use std::path::Path;
+    let mut file = match File::open(&Path::new(input_path)) {
+        Ok(f) => f,
+        Err(err) => {
+            panic!("Can't open {}: {}", input_path, err);
+        }
+    };
+
+    let mut bytes = Vec::new();
+
+    match file.read_to_end(&mut bytes) {
+        Ok(..) => {},
+        Err(_) => {
+            panic!("Can't read {}", input_path);
+        }
+    };
+
+    match String::from_utf8(bytes) {
+        Ok(contents) => contents,
+        Err(_) => {
+            panic!("{} is not UTF-8 encoded", input_path);
+        }
+    }
+}
+
 enum Void {}
 
 #[cyclomatic_complexity = "0"]