Remove use of the HIR CFG

This commit is contained in:
Matthew Jasper 2019-09-08 11:39:42 +01:00
parent f30bf69ec7
commit a2477f7759
3 changed files with 75 additions and 240 deletions

View File

@ -1,15 +1,13 @@
//! calculate cognitive complexity and warn about overly complex functions
use rustc::cfg::CFG;
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc::ty;
use rustc::{declare_tool_lint, impl_lint_pass};
use syntax::ast::Attribute;
use syntax::source_map::Span;
use crate::utils::{is_allowed, match_type, paths, span_help_and_lint, LimitStack};
use crate::utils::{match_type, paths, span_help_and_lint, LimitStack};
declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
@ -46,30 +44,11 @@ fn check<'a, 'tcx>(&mut self, cx: &'a LateContext<'a, 'tcx>, body: &'tcx Body, s
return;
}
let cfg = CFG::new(cx.tcx, body);
let expr = &body.value;
let n = cfg.graph.len_nodes() as u64;
let e = cfg.graph.len_edges() as u64;
if e + 2 < n {
// the function has unreachable code, other lints should catch this
return;
}
let cc = e + 2 - n;
let mut helper = CCHelper {
match_arms: 0,
divergence: 0,
short_circuits: 0,
returns: 0,
cx,
};
let mut helper = CCHelper { cc: 1, returns: 0 };
helper.visit_expr(expr);
let CCHelper {
match_arms,
divergence,
short_circuits,
returns,
..
} = helper;
let CCHelper { cc, returns } = helper;
let ret_ty = cx.tables.node_type(expr.hir_id);
let ret_adjust = if match_type(cx, ret_ty, &paths::RESULT) {
returns
@ -78,36 +57,23 @@ fn check<'a, 'tcx>(&mut self, cx: &'a LateContext<'a, 'tcx>, body: &'tcx Body, s
(returns / 2)
};
if cc + divergence < match_arms + short_circuits {
report_cc_bug(
let mut rust_cc = cc;
// 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,
cc,
match_arms,
divergence,
short_circuits,
ret_adjust,
COGNITIVE_COMPLEXITY,
span,
body.id().hir_id,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
self.limit.limit()
),
"you could split it up into multiple smaller functions",
);
} 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,
COGNITIVE_COMPLEXITY,
span,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
self.limit.limit()
),
"you could split it up into multiple smaller functions",
);
}
}
}
}
@ -136,99 +102,27 @@ fn exit_lint_attrs(&mut self, cx: &LateContext<'a, 'tcx>, attrs: &'tcx [Attribut
}
}
struct CCHelper<'a, 'tcx> {
match_arms: u64,
divergence: u64,
struct CCHelper {
cc: u64,
returns: u64,
short_circuits: u64, // && and ||
cx: &'a LateContext<'a, 'tcx>,
}
impl<'a, 'tcx> Visitor<'tcx> for CCHelper<'a, 'tcx> {
impl<'tcx> Visitor<'tcx> for CCHelper {
fn visit_expr(&mut self, e: &'tcx Expr) {
walk_expr(self, e);
match e.node {
ExprKind::Match(_, ref arms, _) => {
walk_expr(self, e);
let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum();
if arms_n > 1 {
self.match_arms += arms_n - 2;
}
},
ExprKind::Call(ref callee, _) => {
walk_expr(self, e);
let ty = self.cx.tables.node_type(callee.hir_id);
match ty.sty {
ty::FnDef(..) | ty::FnPtr(_) => {
let sig = ty.fn_sig(self.cx.tcx);
if sig.skip_binder().output().sty == ty::Never {
self.divergence += 1;
}
},
_ => (),
}
},
ExprKind::Closure(.., _) => (),
ExprKind::Binary(op, _, _) => {
walk_expr(self, e);
match op.node {
BinOpKind::And | BinOpKind::Or => self.short_circuits += 1,
_ => (),
self.cc += 1;
}
self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64;
},
ExprKind::Ret(_) => self.returns += 1,
_ => walk_expr(self, e),
_ => {},
}
}
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::None
}
}
#[cfg(feature = "debugging")]
#[allow(clippy::too_many_arguments)]
fn report_cc_bug(
_: &LateContext<'_, '_>,
cc: u64,
narms: u64,
div: u64,
shorts: u64,
returns: u64,
span: Span,
_: HirId,
) {
span_bug!(
span,
"Clippy encountered a bug calculating cognitive complexity: cc = {}, arms = {}, \
div = {}, shorts = {}, returns = {}. Please file a bug report.",
cc,
narms,
div,
shorts,
returns
);
}
#[cfg(not(feature = "debugging"))]
#[allow(clippy::too_many_arguments)]
fn report_cc_bug(
cx: &LateContext<'_, '_>,
cc: u64,
narms: u64,
div: u64,
shorts: u64,
returns: u64,
span: Span,
id: HirId,
) {
if !is_allowed(cx, COGNITIVE_COMPLEXITY, id) {
cx.sess().span_note_without_error(
span,
&format!(
"Clippy encountered a bug calculating cognitive complexity \
(hide this message with `#[allow(cognitive_complexity)]`): \
cc = {}, arms = {}, div = {}, shorts = {}, returns = {}. \
Please file a bug report.",
cc, narms, div, shorts, returns
),
);
}
}

View File

@ -87,7 +87,7 @@ fn main() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn kaboom() {
let n = 0;
'a: for i in 0..20 {
@ -133,17 +133,19 @@ fn bloo() {
}
}
#[clippy::cognitive_complexity = "0"]
// Short circuiting operations don't increase the complexity of a function.
// Note that the minimum complexity of a function is 1.
#[clippy::cognitive_complexity = "1"]
fn lots_of_short_circuits() -> bool {
true && false && true && false && true && false && true
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn lots_of_short_circuits2() -> bool {
true || false || true || false || true || false || true
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn baa() {
let x = || match 99 {
0 => 0,
@ -161,7 +163,7 @@ fn baa() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn bar() {
match 99 {
0 => println!("hi"),
@ -170,7 +172,7 @@ fn bar() {
}
#[test]
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
/// Tests are usually complex but simple at the same time. `clippy::cognitive_complexity` used to
/// give lots of false-positives in tests.
fn dont_warn_on_tests() {
@ -180,7 +182,7 @@ fn dont_warn_on_tests() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn barr() {
match 99 {
0 => println!("hi"),
@ -190,7 +192,7 @@ fn barr() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn barr2() {
match 99 {
0 => println!("hi"),
@ -206,7 +208,7 @@ fn barr2() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn barrr() {
match 99 {
0 => println!("hi"),
@ -216,7 +218,7 @@ fn barrr() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn barrr2() {
match 99 {
0 => println!("hi"),
@ -232,7 +234,7 @@ fn barrr2() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn barrrr() {
match 99 {
0 => println!("hi"),
@ -242,7 +244,7 @@ fn barrrr() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn barrrr2() {
match 99 {
0 => println!("hi"),
@ -258,7 +260,7 @@ fn barrrr2() {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn cake() {
if 4 == 5 {
println!("yea");
@ -268,7 +270,7 @@ fn cake() {
println!("whee");
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
pub fn read_file(input_path: &str) -> String {
use std::fs::File;
use std::io::{Read, Write};
@ -299,20 +301,20 @@ pub fn read_file(input_path: &str) -> String {
enum Void {}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn void(void: Void) {
if true {
match void {}
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn mcarton_sees_all() {
panic!("meh");
panic!("möh");
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn try_() -> Result<i32, &'static str> {
match 5 {
5 => Ok(5),
@ -320,7 +322,7 @@ fn try_() -> Result<i32, &'static str> {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn try_again() -> Result<i32, &'static str> {
let _ = Ok(42)?;
let _ = Ok(43)?;
@ -336,7 +338,7 @@ fn try_again() -> Result<i32, &'static str> {
}
}
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn early() -> Result<i32, &'static str> {
return Ok(5);
return Ok(5);
@ -350,7 +352,7 @@ fn early() -> Result<i32, &'static str> {
}
#[rustfmt::skip]
#[clippy::cognitive_complexity = "0"]
#[clippy::cognitive_complexity = "1"]
fn early_ret() -> i32 {
let a = if true { 42 } else { return 0; };
let a = if a < 99 { 42 } else { return 0; };

View File

@ -13,7 +13,7 @@ LL | | }
= note: `-D clippy::cognitive-complexity` implied by `-D warnings`
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (7/0)
error: the function has a cognitive complexity of (7/1)
--> $DIR/cognitive_complexity.rs:91:1
|
LL | / fn kaboom() {
@ -27,28 +27,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (1/0)
--> $DIR/cognitive_complexity.rs:137:1
|
LL | / fn lots_of_short_circuits() -> bool {
LL | | true && false && true && false && true && false && true
LL | | }
| |_^
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (1/0)
--> $DIR/cognitive_complexity.rs:142:1
|
LL | / fn lots_of_short_circuits2() -> bool {
LL | | true || false || true || false || true || false || true
LL | | }
| |_^
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:147:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:149:1
|
LL | / fn baa() {
LL | | let x = || match 99 {
@ -61,8 +41,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:148:13
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:150:13
|
LL | let x = || match 99 {
| _____________^
@ -76,8 +56,8 @@ LL | | };
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:165:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:167:1
|
LL | / fn bar() {
LL | | match 99 {
@ -89,8 +69,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:184:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:186:1
|
LL | / fn barr() {
LL | | match 99 {
@ -103,8 +83,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (3/0)
--> $DIR/cognitive_complexity.rs:194:1
error: the function has a cognitive complexity of (3/1)
--> $DIR/cognitive_complexity.rs:196:1
|
LL | / fn barr2() {
LL | | match 99 {
@ -117,8 +97,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:210:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:212:1
|
LL | / fn barrr() {
LL | | match 99 {
@ -131,8 +111,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (3/0)
--> $DIR/cognitive_complexity.rs:220:1
error: the function has a cognitive complexity of (3/1)
--> $DIR/cognitive_complexity.rs:222:1
|
LL | / fn barrr2() {
LL | | match 99 {
@ -145,8 +125,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:236:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:238:1
|
LL | / fn barrrr() {
LL | | match 99 {
@ -159,8 +139,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (3/0)
--> $DIR/cognitive_complexity.rs:246:1
error: the function has a cognitive complexity of (3/1)
--> $DIR/cognitive_complexity.rs:248:1
|
LL | / fn barrrr2() {
LL | | match 99 {
@ -173,8 +153,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (2/0)
--> $DIR/cognitive_complexity.rs:262:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:264:1
|
LL | / fn cake() {
LL | | if 4 == 5 {
@ -187,8 +167,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (4/0)
--> $DIR/cognitive_complexity.rs:272:1
error: the function has a cognitive complexity of (4/1)
--> $DIR/cognitive_complexity.rs:274:1
|
LL | / pub fn read_file(input_path: &str) -> String {
LL | | use std::fs::File;
@ -201,8 +181,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (1/0)
--> $DIR/cognitive_complexity.rs:303:1
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:305:1
|
LL | / fn void(void: Void) {
LL | | if true {
@ -213,49 +193,8 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (1/0)
--> $DIR/cognitive_complexity.rs:316:1
|
LL | / fn try_() -> Result<i32, &'static str> {
LL | | match 5 {
LL | | 5 => Ok(5),
LL | | _ => return Err("bla"),
LL | | }
LL | | }
| |_^
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (1/0)
--> $DIR/cognitive_complexity.rs:324:1
|
LL | / fn try_again() -> Result<i32, &'static str> {
LL | | let _ = Ok(42)?;
LL | | let _ = Ok(43)?;
LL | | let _ = Ok(44)?;
... |
LL | | }
LL | | }
| |_^
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (1/0)
--> $DIR/cognitive_complexity.rs:340:1
|
LL | / fn early() -> Result<i32, &'static str> {
LL | | return Ok(5);
LL | | return Ok(5);
LL | | return Ok(5);
... |
LL | | return Ok(5);
LL | | }
| |_^
|
= help: you could split it up into multiple smaller functions
error: the function has a cognitive complexity of (8/0)
--> $DIR/cognitive_complexity.rs:354:1
error: the function has a cognitive complexity of (8/1)
--> $DIR/cognitive_complexity.rs:356:1
|
LL | / fn early_ret() -> i32 {
LL | | let a = if true { 42 } else { return 0; };
@ -268,5 +207,5 @@ LL | | }
|
= help: you could split it up into multiple smaller functions
error: aborting due to 20 previous errors
error: aborting due to 15 previous errors