Skip to content

Commit

Permalink
Auto merge of #4935 - krishna-veerareddy:issue-4241-cognitive-complex…
Browse files Browse the repository at this point in the history 
…ity-lint-span, r=phansch

Reduce cognitive complexity lint span

Currently the cognitive complexity lint spans the entire function body making it really difficult to read and refactor the code in editors. To fix this we reduce the lint span to the function name.

changelog: Reduce cognitive complexity lint span

Fixes #4241
  • Loading branch information
@bors
bors committed Dec 22, 2019
2 parents cfb3320 + 91a491e commit 7b670a9
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 150 deletions.
47 changes: 39 additions & 8 deletions clippy_lints/src/cognitive_complexity.rs
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
//! calculate cognitive complexity and warn about overly complex functions

use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc::hir::intravisit::{walk_expr, FnKind, NestedVisitorMap, Visitor};
use rustc::hir::*;
use rustc::impl_lint_pass;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc_session::declare_tool_lint;
use syntax::ast::Attribute;
use syntax::source_map::Span;
use syntax_pos::BytePos;

use crate::utils::{match_type, paths, span_help_and_lint, LimitStack};
use crate::utils::{match_type, paths, snippet_opt, span_help_and_lint, LimitStack};

declare_clippy_lint! {
/// **What it does:** Checks for methods with high cognitive complexity.
Expand Down Expand Up @@ -41,8 +42,16 @@ impl CognitiveComplexity {
impl_lint_pass!(CognitiveComplexity => [COGNITIVE_COMPLEXITY]);

impl CognitiveComplexity {
fn check<'a, 'tcx>(&mut self, cx: &'a LateContext<'a, 'tcx>, body: &'tcx Body, span: Span) {
if span.from_expansion() {
#[allow(clippy::cast_possible_truncation)]
fn check<'a, 'tcx>(
&mut self,
cx: &'a LateContext<'a, 'tcx>,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl,
body: &'tcx Body,
body_span: Span,
) {
if body_span.from_expansion() {
return;
}

Expand All @@ -64,11 +73,33 @@ impl CognitiveComplexity {
if rust_cc >= ret_adjust {
rust_cc -= ret_adjust;
}

if rust_cc > self.limit.limit() {
let fn_span = match kind {
FnKind::ItemFn(ident, _, _, _, _) | FnKind::Method(ident, _, _, _) => ident.span,
FnKind::Closure(_) => {
let header_span = body_span.with_hi(decl.output.span().lo());
let pos = snippet_opt(cx, header_span).and_then(|snip| {
let low_offset = snip.find('|')?;
let high_offset = 1 + snip.get(low_offset + 1..)?.find('|')?;
let low = header_span.lo() + BytePos(low_offset as u32);
let high = low + BytePos(high_offset as u32 + 1);

Some((low, high))
});

if let Some((low, high)) = pos {
Span::new(low, high, header_span.ctxt())
} else {
return;
}
},
};

span_help_and_lint(
cx,
COGNITIVE_COMPLEXITY,
span,
fn_span,
&format!(
"the function has a cognitive complexity of ({}/{})",
rust_cc,
Expand All @@ -84,15 +115,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CognitiveComplexity {
fn check_fn(
&mut self,
cx: &LateContext<'a, 'tcx>,
_: intravisit::FnKind<'tcx>,
_: &'tcx FnDecl,
kind: FnKind<'tcx>,
decl: &'tcx FnDecl,
body: &'tcx Body,
span: Span,
hir_id: HirId,
) {
let def_id = cx.tcx.hir().local_def_id(hir_id);
if !cx.tcx.has_attr(def_id, sym!(test)) {
self.check(cx, body, span);
self.check(cx, kind, decl, body, span);
}
}

Expand Down
22 changes: 22 additions & 0 deletions tests/ui/cognitive_complexity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,3 +371,25 @@ fn early_ret() -> i32 {
_ => return 6,
}
}

#[clippy::cognitive_complexity = "1"]
fn closures() {
let x = |a: i32, b: i32| -> i32 {
if true {
println!("moo");
}

a + b
};
}

struct Moo;

#[clippy::cognitive_complexity = "1"]
impl Moo {
fn moo(&self) {
if true {
println!("moo");
}
}
}
194 changes: 61 additions & 133 deletions tests/ui/cognitive_complexity.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,211 +1,139 @@
error: the function has a cognitive complexity of (28/25)
--> $DIR/cognitive_complexity.rs:6:1
--> $DIR/cognitive_complexity.rs:6:4
|
LL | / fn main() {
LL | | if true {
LL | | println!("a");
LL | | }
... |
LL | | }
LL | | }
| |_^
LL | fn main() {
| ^^^^
|
= 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/1)
--> $DIR/cognitive_complexity.rs:91:1
--> $DIR/cognitive_complexity.rs:91:4
|
LL | / fn kaboom() {
LL | | let n = 0;
LL | | 'a: for i in 0..20 {
LL | | 'b: for j in i..20 {
... |
LL | | }
LL | | }
| |_^
LL | fn kaboom() {
| ^^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:149:1
--> $DIR/cognitive_complexity.rs:149:4
|
LL | / fn baa() {
LL | | let x = || match 99 {
LL | | 0 => 0,
LL | | 1 => 1,
... |
LL | | }
LL | | }
| |_^
LL | fn baa() {
| ^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:150:13
|
LL | let x = || match 99 {
| _____________^
LL | | 0 => 0,
LL | | 1 => 1,
LL | | 2 => 2,
... |
LL | | _ => 42,
LL | | };
| |_____^
LL | let x = || match 99 {
| ^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:167:1
--> $DIR/cognitive_complexity.rs:167:4
|
LL | / fn bar() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | _ => println!("bye"),
LL | | }
LL | | }
| |_^
LL | fn bar() {
| ^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:186:1
--> $DIR/cognitive_complexity.rs:186:4
|
LL | / fn barr() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | 1 => println!("bla"),
... |
LL | | }
LL | | }
| |_^
LL | fn barr() {
| ^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (3/1)
--> $DIR/cognitive_complexity.rs:196:1
--> $DIR/cognitive_complexity.rs:196:4
|
LL | / fn barr2() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | 1 => println!("bla"),
... |
LL | | }
LL | | }
| |_^
LL | fn barr2() {
| ^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:212:1
--> $DIR/cognitive_complexity.rs:212:4
|
LL | / fn barrr() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | 1 => panic!("bla"),
... |
LL | | }
LL | | }
| |_^
LL | fn barrr() {
| ^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (3/1)
--> $DIR/cognitive_complexity.rs:222:1
--> $DIR/cognitive_complexity.rs:222:4
|
LL | / fn barrr2() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | 1 => panic!("bla"),
... |
LL | | }
LL | | }
| |_^
LL | fn barrr2() {
| ^^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:238:1
--> $DIR/cognitive_complexity.rs:238:4
|
LL | / fn barrrr() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | 1 => println!("bla"),
... |
LL | | }
LL | | }
| |_^
LL | fn barrrr() {
| ^^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (3/1)
--> $DIR/cognitive_complexity.rs:248:1
--> $DIR/cognitive_complexity.rs:248:4
|
LL | / fn barrrr2() {
LL | | match 99 {
LL | | 0 => println!("hi"),
LL | | 1 => println!("bla"),
... |
LL | | }
LL | | }
| |_^
LL | fn barrrr2() {
| ^^^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:264:1
--> $DIR/cognitive_complexity.rs:264:4
|
LL | / fn cake() {
LL | | if 4 == 5 {
LL | | println!("yea");
LL | | } else {
... |
LL | | println!("whee");
LL | | }
| |_^
LL | fn cake() {
| ^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (4/1)
--> $DIR/cognitive_complexity.rs:274:1
--> $DIR/cognitive_complexity.rs:274:8
|
LL | / pub fn read_file(input_path: &str) -> String {
LL | | use std::fs::File;
LL | | use std::io::{Read, Write};
LL | | use std::path::Path;
... |
LL | | }
LL | | }
| |_^
LL | pub fn read_file(input_path: &str) -> String {
| ^^^^^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:305:1
--> $DIR/cognitive_complexity.rs:305:4
|
LL | / fn void(void: Void) {
LL | | if true {
LL | | match void {}
LL | | }
LL | | }
| |_^
LL | fn void(void: Void) {
| ^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (8/1)
--> $DIR/cognitive_complexity.rs:356:1
--> $DIR/cognitive_complexity.rs:356:4
|
LL | / fn early_ret() -> i32 {
LL | | let a = if true { 42 } else { return 0; };
LL | | let a = if a < 99 { 42 } else { return 0; };
LL | | let a = if a < 99 { 42 } else { return 0; };
... |
LL | | }
LL | | }
| |_^
LL | fn early_ret() -> i32 {
| ^^^^^^^^^
|
= help: you could split it up into multiple smaller functions

error: aborting due to 15 previous errors
error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:377:13
|
LL | let x = |a: i32, b: i32| -> i32 {
| ^^^^^^^^^^^^^^^^
|
= help: you could split it up into multiple smaller functions

error: the function has a cognitive complexity of (2/1)
--> $DIR/cognitive_complexity.rs:390:8
|
LL | fn moo(&self) {
| ^^^
|
= help: you could split it up into multiple smaller functions

error: aborting due to 17 previous errors

Loading

0 comments on commit 7b670a9

Please sign in to comment.