fix cc computation in the presence of diverging calls

CFG treats diverging calls as its completely own path out of the function.
While this makes sense, it should also mean that a panic should increase the cyclomatic
complexity. Instead it decreases it.

Minimal example:

```rust
if a {
    b
} else {
    panic!("cake");
}
d
```

creates the following graph

```dot
digraph G {
  "if a" -> "b"
  "if a" -> "panic!(\"cake\")"
  "b" -> c
}
```

which has a CC of 1 (3 - 4 + 2). A CC of 1 means there is one path through the program.
Obviously that is wrong. There are two paths. One returning normally, and one panicking.
This commit is contained in:
Oliver Schneider 2015-12-14 14:29:20 +01:00
parent d7292fe235
commit 902c7d832b
3 changed files with 171 additions and 16 deletions

View File

@ -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));
}
}

25
tests/cc_seme.rs Normal file
View File

@ -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!(),
}
}

View File

@ -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"]