From 91a491e68eede0280cab2cce6b53e5923d7cd140 Mon Sep 17 00:00:00 2001 From: Krishna Veera Reddy Date: Sat, 21 Dec 2019 18:07:53 -0800 Subject: [PATCH] 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. --- clippy_lints/src/cognitive_complexity.rs | 47 ++++- tests/ui/cognitive_complexity.rs | 22 ++ tests/ui/cognitive_complexity.stderr | 194 ++++++------------ .../ui/cognitive_complexity_attr_used.stderr | 12 +- 4 files changed, 125 insertions(+), 150 deletions(-) diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index 9f2e939b0..405e7335e 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -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. @@ -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; } @@ -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, @@ -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); } } diff --git a/tests/ui/cognitive_complexity.rs b/tests/ui/cognitive_complexity.rs index 0f7857a45..1d3fe4055 100644 --- a/tests/ui/cognitive_complexity.rs +++ b/tests/ui/cognitive_complexity.rs @@ -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"); + } + } +} diff --git a/tests/ui/cognitive_complexity.stderr b/tests/ui/cognitive_complexity.stderr index 1fca4f5bd..a0ddc673a 100644 --- a/tests/ui/cognitive_complexity.stderr +++ b/tests/ui/cognitive_complexity.stderr @@ -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 diff --git a/tests/ui/cognitive_complexity_attr_used.stderr b/tests/ui/cognitive_complexity_attr_used.stderr index d06066972..f5ff53dda 100644 --- a/tests/ui/cognitive_complexity_attr_used.stderr +++ b/tests/ui/cognitive_complexity_attr_used.stderr @@ -1,14 +1,8 @@ error: the function has a cognitive complexity of (3/0) - --> $DIR/cognitive_complexity_attr_used.rs:9:1 + --> $DIR/cognitive_complexity_attr_used.rs:9:4 | -LL | / fn kaboom() { -LL | | if 42 == 43 { -LL | | panic!(); -LL | | } else if "cake" == "lie" { -LL | | println!("what?"); -LL | | } -LL | | } - | |_^ +LL | fn kaboom() { + | ^^^^^^ | = note: `-D clippy::cognitive-complexity` implied by `-D warnings` = help: you could split it up into multiple smaller functions