Skip to content

Incorrect Cognitive Complexity value when nesting for loops #11164

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

Open
epic-64 opened this issue Jul 15, 2023 · 2 comments
Open

Incorrect Cognitive Complexity value when nesting for loops #11164

epic-64 opened this issue Jul 15, 2023 · 2 comments
Labels
C-bug Category: Clippy is not doing the correct thing L-nursery Lint: Currently in the nursery group

Comments

@epic-64
Copy link

epic-64 commented Jul 15, 2023

Summary

I tried the cognitive_complexity lint on a basic code example. The result does not match up.

Input:

fn main() -> Result<()> {
    for _i in 0..10 { // +1
        for _k in 0..10 { // +2
            for _l in 0..10 { // +3
                for _m in 0..10 { // +4
                    for _n in 0..10 { // +5
                        for _o in 0..10 { // +6
                            print!("")
                        }
                    }
                }
            }
        }
    }

    Ok(())
}

Expected result: 21
Actual Result: 7

Additional Details
To my understanding, the cognitive complexty of the main function is 21, as each for loop adds a value of 1, plus 1 for each level of nesting that it is already in.

However the warning I get says that it is 7:

warning: the function has a cognitive complexity of (7/1)
  --> src\main.rs:17:4
   |
17 | fn main() -> Result<()> {
   |    ^^^^
   |
note: the lint level is defined here
  --> src\main.rs:1:9
   |
1  | #![warn(clippy::cognitive_complexity)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: you could split it up into multiple smaller functions
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity

Reproducer

I tried this code:

cargo clippy

Given this source code:

fn main() -> Result<()> {
    for _i in 0..10 { // +1
        for _k in 0..10 { // +2
            for _l in 0..10 { // +3
                for _m in 0..10 { // +4
                    for _n in 0..10 { // +5
                        for _o in 0..10 { // +6
                            print!("")
                        }
                    }
                }
            }
        }
    }

    Ok(())
}

I expected to see this happen:
warning: the function has a cognitive complexity of (21/1)

Instead, this happened:
warning: the function has a cognitive complexity of (7/1)

Version

rustc 1.71.0 (8ede3aae2 2023-07-12)
binary: rustc
commit-hash: 8ede3aae28fe6e4d52b38157d7bfe0d3bceef225
commit-date: 2023-07-12
host: x86_64-pc-windows-msvc
release: 1.71.0
LLVM version: 16.0.5

Additional Labels

No response

@epic-64 epic-64 added the C-bug Category: Clippy is not doing the correct thing label Jul 15, 2023
@epic-64 epic-64 changed the title Incorrect Congnitive Complexity value Incorrect Cognitive Complexity value Jul 15, 2023
@epic-64 epic-64 changed the title Incorrect Cognitive Complexity value Incorrect Cognitive Complexity value when nesting for loops Jul 15, 2023
@epic-64
Copy link
Author

epic-64 commented Jul 15, 2023

I found the following test (comments added by me).
https://github.com/rust-lang/rust-clippy/blob/master/tests/ui/cognitive_complexity.rs#L91
https://github.com/rust-lang/rust-clippy/blob/master/tests/ui/cognitive_complexity.stderr#L10

fn kaboom() {
    let n = 0;
    'a: for i in 0..20 { // +1 (nesting level 0)
        'b: for j in i..20 { // +2 (nesting level 1)
            for k in j..20 { // +3 (nesting level 2)
                if k == 5 { // +4 (nesting level 3)
                    break 'b;
                }
                if j == 3 && k == 6 { // +5 (nesting level 3, +1 for introducting a boolean operation)
                    continue 'a;
                }
                if k == j { // +4 (nesting level 3)
                    continue;
                }
                println!("bake");
            }
        }
        println!("cake");
    }
    
    // Cognitive Complexity: 19
}

By counting I arrive at a value of 19, but the test suite asserts (and measures) a value of 7.
Clearly there is some mismatch of definitions here. I go by the original specification https://www.sonarsource.com/docs/CognitiveComplexity.pdf

@J-ZhengLi J-ZhengLi added the L-nursery Lint: Currently in the nursery group label Apr 2, 2024
@felix91gr
Copy link
Contributor

@epic-64 yeah, sorry about that. I was the last to touch that lint.

In a nutshell: the lint is broken and the entire concept of linting "Cognitive Complexity" is nonsense. Don't believe what SonarSource says about its marvels; it's just not true, fam. That's how they got me.

It's a lint that cannot be accomplished with modern techniques. It's a good few years away, perhaps a decade or more.

#3793 Here you can see the logs of my investigation into it.


@flip1995 is there a way to archive this lint further down than Nursery? Maybe even... deleting it? There's at least a handful of users who've tried it and I don't think it would be honest to implement SonarSource's marketing whitepaper and call it a day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing L-nursery Lint: Currently in the nursery group
Projects
None yet
Development

No branches or pull requests

3 participants