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.
rust-lang · Dec 22, 2019 · 91a491e · 91a491e
1 parent cfb3320
commit 91a491e
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 150 deletions.
diff --git 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
@@ -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
@@ -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