revive
revive copied to clipboard
False Positive in rule max-control-nesting
Describe the bug
The newly added rule max-control-nesting
added in PR #967, does not correctly handle else if
blocks.
To Reproduce Steps to reproduce the behavior:
- Add the following test to the file
testdata/max-control-nesting.go
:
a := 1
if a == 0 {
} else if a == 1 {
} else if a == 2 {
} else {
}
- Run the tests with
go test ./...
- You will get the following error:
--- FAIL: TestMaxControlNesting (0.00s)
utils_test.go:129: Unexpected problem at max-control-nesting.go:71: control flow nesting exceeds 2
utils_test.go:129: Unexpected problem at max-control-nesting.go:71: control flow nesting exceeds 2
The steps above are for a minimum reproducible example. However, this also fails with the latest version of golangci-lint
, which flagged the following code (default is 5):
if e == "a" {
return 1
} else if e == "b" {
return 2
} else if e == "c" {
return 3
} else if e == "d" {
return 4
} else if e == "e" {
return 5
} else if e == "f" {
return 6
}
Expected behavior
No nesting errors should happen when there is no nestedness in else if
blocks
Desktop (please complete the following information):
- OS: macOS 12.3
- Version of Go: 1.22.0 (when running the tests), 1.21.7 when using golangci-lint
I think, the reason for that is that
if e == "a" {
return 1
} else if e == "b" {
return 2
} else if e == "c" {
return 3
} else if e == "d" {
return 4
} else if e == "e" {
return 5
} else if e == "f" {
return 6
}
is interpreted as
if e == "a" {
return 1
} else {
if e == "b" {
return 2
} else {
if e == "c" {
return 3
} else {
if e == "d" {
return 4
} else {
if e == "e" {
return 5
} else {
if e == "f" {
return 6
}
}
}
}
}
}
If you want a go idiomatic solution that will pass through the rule you can use the following alternative:
switch {
case e == "a":
return 1
case e == "b":
return 2
case e == "c":
return 3
case e == "d":
return 4
case e == "e":
return 5
case e == "f":
return 6
}
It will do the same, but it won't be nesting and it will be more idiomatic to go (it is also shorter than if/else if/else construct).
Hi @sk-, thanks for reporting the issue.
I've developed the rule with the else-if
interpretation described by @denisvmedia because I consider else-if
constructs are indeed nested structures. The rule being not friendly with else-if
guides naturally to write idiomatic Go code (as the above example by @denisvmedia)
I' ll close this issue. Feel free to reopen if you think the current behavior of the rule with else-if
needs to be modified.