empty-switch-cases: report empty switch cases

[ccoVeille]

Is your feature request related to a problem? Please describe.

People coming from other languages can do error with the way switch statement works in Go

switch a {
case 1:
case 2:
 	foo()
default:
	return err
}

// something else

In go case 1: do nothing, while in a lot of language case 1 falls through case 2:

Some people would expect the code to behave like this

switch a {
case 1, 2:
 	foo()
default:
	return err
}

// something else

or

switch a {
case 1:
	fallthrough
case 2:
 	foo()
default:
	return err
}

// something else

or

switch a {
case 1:
	foo()
case 2:
	foo()
default:
	return err
}

// something else

Note: The following pattern is currently reported by identical-switch-branches rule

switch a {
case 1:
case 2:
default:
	return err
}

// something else

But not for the following ones

switch a {
case 1:
case 2:
   somecode()
default:
return err
}

We have to be careful because, there are also edge cases, that is currently reported by identical-switch-branches

switch a {
case 1:
case 2:
   // a comment
default:
return err
}

switch a {
case 1:
	// another comment
case 2:
   // a comment
default:
return err
}

Describe the solution you'd like

A linter closes to existing ones that would report this as being a possible source of error

switch a {
case 1:
case 2:
	foo()
default:
	return err
}

// something else

The confidence would then be around 0.8, or less.

Maybe the failure could be about:

• adding a comment for case 1 or group case 1,2:
• suggest moving the case to another place

The issue I have is the fact I'm unsure what to suggest, we don't have such a thing like put empty case at the bottom:

switch a {
default:
	return err
case 1,2:
}

// something else

It's not idiomatic, because Gophers who knows how switch works won't do the error.

I feel like it should be in a dedicated rule, people might want enforce-switch-style for default and create a new one

We can choose to put it in identical-switch-branches, or enforce-switch-style, but I would prefer if we don't as I feel they are not about reporting possible bugs, but purely stylistic.

Naming is hard, but I would say empty-switch-cases (plural or not), the issue is the fact an empty case is followed by something non, empty

Describe alternatives you've considered

N/A

Additional context

• Add "emptycase" linter golangci/golangci-lint#3489 (comment)

And https://github.com/gostaticanalysis/emptycase but we will need to review it, as I'm unsure about what it reports.

cc @danielle-tfh who recently commented the golangci-lint issue
cc @alexandear I talked with about the idea of adding this to revive

note: this looks like a bit to the perimeter of enforce-switch-style, but this one is for now focused on the position of default

I don't think we can reuse enforce-switch-style

[denisvmedia]

Note, we should take into account a possibility of cases with expressions (not a constant value). Expressions can have a side effect, and that can be deliberate (although at the same time can be confusing). Which (as a side note) leads me to an idea of a separate rule (if it doesn't exist yet) to lint against expressions in case.

[ccoVeille]

Note, we should take into account a possibility of cases with expressions (not a constant value). Expressions can have a side effect, and that can be deliberate (although at the same time can be confusing).

Do you have an example to make sure I get the point?

Which (as a side note) leads me to an idea of a separate rule (if it doesn't exist yet) to lint against expressions in case.

Let's open an new issue then, because if we talk here. My issue will be spammed :)

[denisvmedia]

Consider these:

switch f("switch", 2) {
case f("case1", 1):
case f("case2", 2):
}

switch x {
case f("a", 1), f("b", 2):
    fmt.Println("matched")
}

Go playground: https://go.dev/play/p/4Dd0uxM4NOt

[ccoVeille]

I feel lucky enough to never have faced such piece of code