diff --git a/src/cyclomatic_complexity.rs b/src/cyclomatic_complexity.rs index 1379a8db15d..cb391769279 100644 --- a/src/cyclomatic_complexity.rs +++ b/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)); } } diff --git a/src/loops.rs b/src/loops.rs index 3c4e023b98e..8295e186172 100644 --- a/src/loops.rs +++ b/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::>(); - 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", diff --git a/src/matches.rs b/src/matches.rs index 39459bafba7..460893d93ab 100644 --- a/src/matches.rs +++ b/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 { diff --git a/tests/cc_seme.rs b/tests/cc_seme.rs new file mode 100644 index 00000000000..a26731c396a --- /dev/null +++ b/tests/cc_seme.rs @@ -0,0 +1,25 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#[allow(dead_code)] +enum Baz { + Baz1, + Baz2, +} + +struct Test { + t: Option, + 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!(), + } +} diff --git a/tests/compile-fail/cyclomatic_complexity.rs b/tests/compile-fail/cyclomatic_complexity.rs index 8e3bf123c26..1a6dfd28728 100644 --- a/tests/compile-fail/cyclomatic_complexity.rs +++ b/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"]