1
0
mirror of https://github.com/mgechev/revive.git synced 2025-06-02 22:57:22 +02:00

fix: cognitive complexity nesting with if-else chains (#1268)

* test: prove bug in cognitive complexity rule

* fix: cognitive complexity nesting with if-else chains

Currently, an if-else chain will increase the nesting level and add the
nesting increment for every addition `else if` statement in an if-else
chain. This is incorrect behaviour; an `else if` statement should
increment complexity by 1 (regardless of current nesting level) and
leave the nesting level as-is.

For example, the following should yield a total complexity of 5:
```
for { // +1
    if a { // +2 (nesting = 1)
        foo()
    } else if b { // +1
        bar()
    } else if c { // +1
        baz()
    }
}
```

but the current implementation incorrectly increments the nesting level
with each `else if` and adds the nesting increment where it shouldn't:
```
for { // +1
    if a { // +2 (nesting = 1)
        foo()
    } else if b { // +3 (nesting = 2)
        bar()
    } else if c { // +4 (nesting = 3)
        baz()
    }
}
```
This commit is contained in:
AxiomaticFixedChimpanzee 2025-03-13 08:31:26 +01:00 committed by GitHub
parent 6d0498cb97
commit b77bb1a9db
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 37 additions and 2 deletions

View File

@ -98,8 +98,7 @@ func (v *cognitiveComplexityVisitor) subTreeComplexity(n ast.Node) int {
func (v *cognitiveComplexityVisitor) Visit(n ast.Node) ast.Visitor {
switch n := n.(type) {
case *ast.IfStmt:
targets := []ast.Node{n.Cond, n.Body, n.Else}
v.walk(1, targets...)
v.walkIfElse(n)
return nil
case *ast.ForStmt:
targets := []ast.Node{n.Cond, n.Body}
@ -156,6 +155,29 @@ func (v *cognitiveComplexityVisitor) walk(complexityIncrement int, targets ...as
v.nestingLevel = nesting
}
func (v *cognitiveComplexityVisitor) walkIfElse(n *ast.IfStmt) {
var w func(n *ast.IfStmt)
w = func(n *ast.IfStmt) {
ast.Walk(v, n.Cond)
ast.Walk(v, n.Body)
if n.Else != nil {
if elif, ok := n.Else.(*ast.IfStmt); ok {
v.complexity++
w(elif)
} else {
ast.Walk(v, n.Else)
}
}
}
// Nesting level is incremented in 'if' and 'else' blocks, but only the first 'if' in an 'if-else-if' chain sees its
// complexity increased by the nesting level.
v.complexity += 1 + v.nestingLevel
v.nestingLevel++
w(n)
v.nestingLevel--
}
func (*cognitiveComplexityVisitor) binExpComplexity(n *ast.BinaryExpr) int {
calculator := binExprComplexityCalculator{opsStack: []token.Token{}}

View File

@ -291,3 +291,16 @@ func Walk(t *Tree, ch chan int) { // MATCH /function Walk has cognitive complexi
ch <- t.Value
Walk(t.Right, ch) // +1
}
// Test if-else if chains
func chainedIfElse(a, b, c, d bool) { // MATCH /function chainedIfElse has cognitive complexity 4 (> max enabled 0)/
if a { // +1
foo()
} else if b && c { // +2
bar()
} else if d { // +1
baz()
} else {
qux()
}
}