Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce cognitive complexity lint span #4935

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 39 additions & 8 deletions clippy_lints/src/cognitive_complexity.rs
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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FnKind::ItemFn(ident, _, _, _, _) | FnKind::Method(ident, _, _, _) => ident.span,
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
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
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