False Positive in rule max-control-nesting

[sk-]

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:

1. 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 {
   }

2. Run the tests with go test ./...
3. 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

[denisvmedia]

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).

[chavacava]

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)

[chavacava]

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.