From bf20b40664d699954599d4e7e333b756cec0ad59 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 8 Mar 2016 15:10:02 +0100 Subject: [PATCH] fix cyclomatic complexity lint triggering because of short circuit operations --- src/cyclomatic_complexity.rs | 30 +++++++++++++------ src/overflow_check_conditional.rs | 9 +++--- src/utils/hir.rs | 2 -- tests/compile-fail/cyclomatic_complexity.rs | 14 +++++++-- .../overflow_check_conditional.rs | 17 +++++------ 5 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/cyclomatic_complexity.rs b/src/cyclomatic_complexity.rs index 05f633ee0..b1b578af7 100644 --- a/src/cyclomatic_complexity.rs +++ b/src/cyclomatic_complexity.rs @@ -51,19 +51,21 @@ impl CyclomaticComplexity { let mut helper = CCHelper { match_arms: 0, divergence: 0, + short_circuits: 0, tcx: &cx.tcx, }; helper.visit_block(block); let CCHelper { match_arms, divergence, + short_circuits, .. } = helper; - if cc + divergence < match_arms { - report_cc_bug(cx, cc, match_arms, divergence, span); + if cc + divergence < match_arms + short_circuits { + report_cc_bug(cx, cc, match_arms, divergence, short_circuits, span); } else { - let rust_cc = cc + divergence - match_arms; + let rust_cc = cc + divergence - match_arms - short_circuits; if rust_cc > self.limit.limit() { span_help_and_lint(cx, CYCLOMATIC_COMPLEXITY, @@ -105,6 +107,7 @@ impl LateLintPass for CyclomaticComplexity { struct CCHelper<'a, 'tcx: 'a> { match_arms: u64, divergence: u64, + short_circuits: u64, // && and || tcx: &'a ty::TyCtxt<'tcx>, } @@ -128,29 +131,38 @@ impl<'a, 'b, 'tcx> Visitor<'a> for CCHelper<'b, 'tcx> { } } ExprClosure(..) => {} + ExprBinary(op, _, _) => { + walk_expr(self, e); + match op.node { + BiAnd | BiOr => self.short_circuits += 1, + _ => {}, + } + } _ => walk_expr(self, e), } } } #[cfg(feature="debugging")] -fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) { +fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, shorts: u64, span: Span) { cx.sess().span_bug(span, &format!("Clippy encountered a bug calculating cyclomatic complexity: cc = {}, arms = {}, \ - div = {}. Please file a bug report.", + div = {}, shorts = {}. Please file a bug report.", cc, narms, - div));; + div, + shorts));; } #[cfg(not(feature="debugging"))] -fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, span: Span) { +fn report_cc_bug(cx: &LateContext, cc: u64, narms: u64, div: u64, shorts: 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 = {}. Please file a bug report.", + = {}, arms = {}, div = {}, shorts = {}. Please file a bug report.", cc, narms, - div)); + div, + shorts)); } } diff --git a/src/overflow_check_conditional.rs b/src/overflow_check_conditional.rs index 2dccbfdb2..823cb6969 100644 --- a/src/overflow_check_conditional.rs +++ b/src/overflow_check_conditional.rs @@ -1,4 +1,3 @@ -#![allow(cyclomatic_complexity)] use rustc::lint::*; use rustc_front::hir::*; use utils::{span_lint}; @@ -38,12 +37,12 @@ impl LateLintPass for OverflowCheckConditional { ], { if let BinOp_::BiLt = op.node { if let BinOp_::BiAdd = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C overflow conditons that will fail in Rust."); + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C overflow conditions that will fail in Rust."); } } if let BinOp_::BiGt = op.node { if let BinOp_::BiSub = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C underflow conditons that will fail in Rust."); + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C underflow conditions that will fail in Rust."); } } }} @@ -60,12 +59,12 @@ impl LateLintPass for OverflowCheckConditional { ], { if let BinOp_::BiGt = op.node { if let BinOp_::BiAdd = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C overflow conditons that will fail in Rust."); + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C overflow conditions that will fail in Rust."); } } if let BinOp_::BiLt = op.node { if let BinOp_::BiSub = op2.node { - span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C underflow conditons that will fail in Rust."); + span_lint(cx, OVERFLOW_CHECK_CONDITIONAL, expr.span, "You are trying to use classic C underflow conditions that will fail in Rust."); } } }} diff --git a/src/utils/hir.rs b/src/utils/hir.rs index 0bd054a83..b4b786f97 100644 --- a/src/utils/hir.rs +++ b/src/utils/hir.rs @@ -55,8 +55,6 @@ impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { both(&left.expr, &right.expr, |l, r| self.eq_expr(l, r)) } - // ok, it’s a big function, but mostly one big match with simples cases - #[allow(cyclomatic_complexity)] pub fn eq_expr(&self, left: &Expr, right: &Expr) -> bool { if self.ignore_fn && differing_macro_contexts(left.span, right.span) { return false; diff --git a/tests/compile-fail/cyclomatic_complexity.rs b/tests/compile-fail/cyclomatic_complexity.rs index 3a4a83af5..30a05c3f8 100644 --- a/tests/compile-fail/cyclomatic_complexity.rs +++ b/tests/compile-fail/cyclomatic_complexity.rs @@ -1,6 +1,6 @@ #![feature(plugin, custom_attribute)] #![plugin(clippy)] -#![deny(clippy)] +#![allow(clippy)] #![deny(cyclomatic_complexity)] #![allow(unused)] @@ -90,7 +90,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 8 +fn kaboom() { //~ ERROR: the function has a cyclomatic complexity of 7 let n = 0; 'a: for i in 0..20 { 'b: for j in i..20 { @@ -135,6 +135,16 @@ fn bloo() { } } +#[cyclomatic_complexity = "0"] +fn lots_of_short_circuits() -> bool { //~ ERROR: the function has a cyclomatic complexity of 1 + true && false && true && false && true && false && true +} + +#[cyclomatic_complexity = "0"] +fn lots_of_short_circuits2() -> bool { //~ ERROR: the function has a cyclomatic complexity of 1 + true || false || true || false || true || false || true +} + #[cyclomatic_complexity = "0"] fn baa() { //~ ERROR: the function has a cyclomatic complexity of 2 let x = || match 99 { diff --git a/tests/compile-fail/overflow_check_conditional.rs b/tests/compile-fail/overflow_check_conditional.rs index df629146d..db7b27924 100644 --- a/tests/compile-fail/overflow_check_conditional.rs +++ b/tests/compile-fail/overflow_check_conditional.rs @@ -7,28 +7,28 @@ fn main() { let a: u32 = 1; let b: u32 = 2; let c: u32 = 3; - if a + b < a { //~ERROR You are trying to use classic C overflow conditons that will fail in Rust. + if a + b < a { //~ERROR You are trying to use classic C overflow conditions that will fail in Rust. } - if a > a + b { //~ERROR You are trying to use classic C overflow conditons that will fail in Rust. + if a > a + b { //~ERROR You are trying to use classic C overflow conditions that will fail in Rust. } - if a + b < b { //~ERROR You are trying to use classic C overflow conditons that will fail in Rust. + if a + b < b { //~ERROR You are trying to use classic C overflow conditions that will fail in Rust. } - if b > a + b { //~ERROR You are trying to use classic C overflow conditons that will fail in Rust. + if b > a + b { //~ERROR You are trying to use classic C overflow conditions that will fail in Rust. } - if a - b > b { //~ERROR You are trying to use classic C underflow conditons that will fail in Rust. + if a - b > b { //~ERROR You are trying to use classic C underflow conditions that will fail in Rust. } - if b < a - b { //~ERROR You are trying to use classic C underflow conditons that will fail in Rust. + if b < a - b { //~ERROR You are trying to use classic C underflow conditions that will fail in Rust. } - if a - b > a { //~ERROR You are trying to use classic C underflow conditons that will fail in Rust. + if a - b > a { //~ERROR You are trying to use classic C underflow conditions that will fail in Rust. } - if a < a - b { //~ERROR You are trying to use classic C underflow conditons that will fail in Rust. + if a < a - b { //~ERROR You are trying to use classic C underflow conditions that will fail in Rust. } if a + b < c { @@ -58,4 +58,3 @@ fn main() { } } -