Inconsistent formatting of switch cases with and without content

[ainar-g]

Before:

switch x {
case
	longConstantName1,
	longConstantName2:
	// A comment.
	fmt.Println(x)
case
	longConstantName3,
	longConstantName4:
	// Do nothing.
default:
	// Another comment.
	fmt.Println(x * 2)
}

After:

switch x {
case
        longConstantName1,
        longConstantName2:
        // A comment.
        fmt.Println(x)
case longConstantName3, longConstantName4:
        // Do nothing.
default:
        // Another comment.
        fmt.Println(x * 2)
}

Notice how the first case remains multiline (the desired behaviour) while the second case is put onto a single line (not the desired behaviour). The only reason between the two seems to be that the first case has code in it while the second doesn't.

gofumpt --version

v0.3.2-0.20220322194931-6b144e85e83b

Full code: https://go.dev/play/p/cCkatTotVs_L.

I'm ready to provide additional data if that helps.

[mvdan]

Out of curiosity, why is joining the lines not the desired behavior? The logic is that if the case is short enough, it should be a single line. The buggy behavior is the second case; see @oiyoo's PR.

[ainar-g]

@mvdan, basically, we'd like consistent formatting of case values, regardless of the body of the branch.

In our projects, we have the following convention: if there is only one case value, leave it on the same line as case; otherwise, put every value on its own line. Again, regardless of the branch body. This is done in order to make the values more visible, make diffs cleaner (making value additions and removals more obvious), make it easier to (re)sort the values (i.e. using Vi(m)'s :sort), and also to make it easier to add comments to values later, if necessary. And, of course, to have a simple and consistent convention to follow.

So, I feel like the current behaviour is confusing and inconsistent, because it seems to not change the formatting of the case values in any other case apart from this one.

[mvdan]

we'd like consistent formatting of case values, regardless of the body of the branch.

Right, and Takbir's patch fixes that. The fact that the body was part of the formatting algorithm was a bug.

if there is only one case value, leave it on the same line as case; otherwise, put every value on its own line

FYI, that is not what gofumpt enforces right now (with Takbir's fix). All we do is that, if a case clause's expression list is short enough to fit comfortably within a single line, we remove the newlines to make it so.

I can see your argument about shorter diffs and easier line-based editing, and perhaps we could tweak the formatting to not kick in when there is a newline before the first element, presumably meaning that the user wants this line-based formatting. However, I'll also admit I don't think this formatting really works in practice due to the trailing tokens; all elements end with a comma, except the last one, which ends in a colon.

[mvdan]

This issue is now closed as the bug is fixed, but I'm happy to further discuss the topic of newline-separated case expressions, either here or in a new issue.

[ainar-g]

I guess it's fine as long as it's automated and consistent. Regarding the trailing tokens, I have tried my best a few years ago, heh. Thanks anyway!