From bf4221c51a54294482e437c803a7b1f9c37dbcd8 Mon Sep 17 00:00:00 2001 From: Oliver 'ker' Schneider Date: Sat, 23 Apr 2016 14:30:05 +0200 Subject: [PATCH] cc: early returns are special --- src/cyclomatic_complexity.rs | 39 +++++++++----- tests/compile-fail/cyclomatic_complexity.rs | 57 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 12 deletions(-) diff --git a/src/cyclomatic_complexity.rs b/src/cyclomatic_complexity.rs index e8a3a569a..9b20cc4a3 100644 --- a/src/cyclomatic_complexity.rs +++ b/src/cyclomatic_complexity.rs @@ -9,7 +9,7 @@ use syntax::ast::Attribute; use syntax::attr; use syntax::codemap::Span; -use utils::{in_macro, LimitStack, span_help_and_lint}; +use utils::{in_macro, LimitStack, span_help_and_lint, paths, match_type}; /// **What it does:** This lint checks for methods with high cyclomatic complexity /// @@ -57,15 +57,26 @@ impl CyclomaticComplexity { match_arms: 0, divergence: 0, short_circuits: 0, + returns: 0, tcx: &cx.tcx, }; helper.visit_block(block); - let CCHelper { match_arms, divergence, short_circuits, .. } = helper; - - if cc + divergence < match_arms + short_circuits { - report_cc_bug(cx, cc, match_arms, divergence, short_circuits, span); + let CCHelper { match_arms, divergence, short_circuits, returns, .. } = helper; + let ret_ty = cx.tcx.node_id_to_type(block.id); + let ret_adjust = if match_type(cx, ret_ty, &paths::RESULT) { + returns } else { - let rust_cc = cc + divergence - match_arms - short_circuits; + returns / 2 + }; + + if cc + divergence < match_arms + short_circuits { + report_cc_bug(cx, cc, match_arms, divergence, short_circuits, ret_adjust, span); + } else { + let mut rust_cc = cc + divergence - match_arms - short_circuits; + // prevent degenerate cases where unreachable code contains `return` statements + if rust_cc >= ret_adjust { + rust_cc -= ret_adjust; + } if rust_cc > self.limit.limit() { span_help_and_lint(cx, CYCLOMATIC_COMPLEXITY, @@ -109,6 +120,7 @@ impl LateLintPass for CyclomaticComplexity { struct CCHelper<'a, 'tcx: 'a> { match_arms: u64, divergence: u64, + returns: u64, short_circuits: u64, // && and || tcx: &'a ty::TyCtxt<'tcx>, } @@ -142,31 +154,34 @@ impl<'a, 'b, 'tcx> Visitor<'a> for CCHelper<'b, 'tcx> { _ => (), } } + ExprRet(_) => self.returns += 1, _ => walk_expr(self, e), } } } #[cfg(feature="debugging")] -fn report_cc_bug(_: &LateContext, cc: u64, narms: u64, div: u64, shorts: u64, span: Span) { +fn report_cc_bug(_: &LateContext, cc: u64, narms: u64, div: u64, shorts: u64, returns: u64, span: Span) { span_bug!(span, "Clippy encountered a bug calculating cyclomatic complexity: cc = {}, arms = {}, \ - div = {}, shorts = {}. Please file a bug report.", + div = {}, shorts = {}, returns = {}. Please file a bug report.", cc, narms, div, - shorts); + shorts, + returns); } #[cfg(not(feature="debugging"))] -fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, shorts: u64, span: Span) { +fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, shorts: u64, returns: u64, span: Span) { if cx.current_level(CYCLOMATIC_COMPLEXITY) != Level::Allow { cx.sess().span_note_without_error(span, &format!("Clippy encountered a bug calculating cyclomatic complexity \ (hide this message with `#[allow(cyclomatic_complexity)]`): cc \ - = {}, arms = {}, div = {}, shorts = {}. Please file a bug report.", + = {}, arms = {}, div = {}, shorts = {}, returns = {}. Please file a bug report.", cc, narms, div, - shorts)); + shorts, + returns)); } } diff --git a/tests/compile-fail/cyclomatic_complexity.rs b/tests/compile-fail/cyclomatic_complexity.rs index 4b24f16ed..2160272bf 100644 --- a/tests/compile-fail/cyclomatic_complexity.rs +++ b/tests/compile-fail/cyclomatic_complexity.rs @@ -315,3 +315,60 @@ fn mcarton_sees_all() { panic!("meh"); panic!("möh"); } + +#[cyclomatic_complexity = "0"] +fn try() -> Result { //~ ERROR: cyclomatic complexity of 1 + match 5 { + 5 => Ok(5), + _ => return Err("bla"), + } +} + +#[cyclomatic_complexity = "0"] +fn try_again() -> Result { //~ ERROR: cyclomatic complexity of 1 + let _ = try!(Ok(42)); + let _ = try!(Ok(43)); + let _ = try!(Ok(44)); + let _ = try!(Ok(45)); + let _ = try!(Ok(46)); + let _ = try!(Ok(47)); + let _ = try!(Ok(48)); + let _ = try!(Ok(49)); + match 5 { + 5 => Ok(5), + _ => return Err("bla"), + } +} + +#[cyclomatic_complexity = "0"] +fn early() -> Result { //~ ERROR: cyclomatic complexity of 1 + return Ok(5); + return Ok(5); + return Ok(5); + return Ok(5); + return Ok(5); + return Ok(5); + return Ok(5); + return Ok(5); + return Ok(5); +} + +#[cyclomatic_complexity = "0"] +fn early_ret() -> i32 { //~ ERROR: cyclomatic complexity of 8 + let a = if true { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + let a = if a < 99 { 42 } else { return 0; }; + match 5 { + 5 => 5, + _ => return 6, + } +}