about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2019-09-08 20:09:12 +0000
committerbors <bors@rust-lang.org>2019-09-08 20:09:12 +0000
commitd4a48edbebd088ac9ca9c4cb51bbc5a5d745b9ca (patch)
treed44e5f3a738c5e04b14fa2ae3111f00551760113
parentf30bf69ec7af4366e64916f32566f3a894edd593 (diff)
parent5e7ff6b7058a5d6da2245145441f2ff2f9e69068 (diff)
downloadrust-d4a48edbebd088ac9ca9c4cb51bbc5a5d745b9ca.tar.gz
rust-d4a48edbebd088ac9ca9c4cb51bbc5a5d745b9ca.zip
Auto merge of #4523 - matthewjasper:remove-cfg-usage, r=llogiq
Stop using the HIR CFG

This does slightly change the behavior:

* Functions with unreachable code don't end up being ignored
* `match` guards are counted as +1 for each guard, rather than +1 for each pattern with a guard
* Or patterns don't add to the complexity

changelog: none
-rw-r--r--clippy_lints/src/cognitive_complexity.rs156
-rw-r--r--clippy_lints/src/non_expressive_names.rs5
-rw-r--r--tests/ui/cognitive_complexity.rs42
-rw-r--r--tests/ui/cognitive_complexity.stderr117
4 files changed, 78 insertions, 242 deletions
diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs
index fccec347ef0..3e7f3c980fc 100644
--- a/clippy_lints/src/cognitive_complexity.rs
+++ b/clippy_lints/src/cognitive_complexity.rs
@@ -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 @@ impl CognitiveComplexity {
             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 @@ impl CognitiveComplexity {
             (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 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CognitiveComplexity {
     }
 }
 
-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
-            ),
-        );
-    }
-}
diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs
index a9977a93b32..88bf52b1e8d 100644
--- a/clippy_lints/src/non_expressive_names.rs
+++ b/clippy_lints/src/non_expressive_names.rs
@@ -136,6 +136,9 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for SimilarNamesNameVisitor<'a, 'tcx, 'b> {
                     }
                 }
             },
+            // just go through the first pattern, as either all patterns
+            // bind the same bindings or rustc would have errored much earlier
+            PatKind::Or(ref pats) => self.visit_pat(&pats[0]),
             _ => walk_pat(self, pat),
         }
     }
@@ -325,8 +328,6 @@ impl<'a, 'tcx> Visitor<'tcx> for SimilarNamesLocalVisitor<'a, 'tcx> {
         self.single_char_names.push(vec![]);
 
         self.apply(|this| {
-            // just go through the first pattern, as either all patterns
-            // bind the same bindings or rustc would have errored much earlier
             SimilarNamesNameVisitor(this).visit_pat(&arm.pat);
             this.apply(|this| walk_expr(this, &arm.body));
         });
diff --git a/tests/ui/cognitive_complexity.rs b/tests/ui/cognitive_complexity.rs
index 7c81cc73d3c..0f7857a45d1 100644
--- a/tests/ui/cognitive_complexity.rs
+++ b/tests/ui/cognitive_complexity.rs
@@ -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; };
diff --git a/tests/ui/cognitive_complexity.stderr b/tests/ui/cognitive_complexity.stderr
index 32a56f00c5d..1fca4f5bd82 100644
--- a/tests/ui/cognitive_complexity.stderr
+++ b/tests/ui/cognitive_complexity.stderr
@@ -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