Suggestion: Force newlines between case

[akarl]

Cases in switch and select should force an empty newline between the cases unless the case block is X (2?) lines long.

Should have newlines:

switch foo {
case "a":
    ...
    ...
    ...
    ...
    ...
    ...
    
case "b":
    ...
    ...
    ...
    ...
    ...
    ...
    
case "c":
    ...
    ...
    ...
    ...
    ...
    ...
}

Should not have newlines

switch foo {
case "a":
    ...
case "b":
    ...
case "c":
    ...
}

[bombsimon]

@akarl Thanks for the report!

This is configurable with allow-case-trailing-whitespace but only to always allow or never allow. Do you suggest making this an integer of number of lines instead or do you want this in addition? Maybe set it to 0 to always allow?

[akarl]

Yes, I would like to have it so that it forces me to add newlines after each case block unless it’s very short.

…

Sent from my iPhone

On 22 Oct 2019, at 14:44, Simon Sawert ***@***.***> wrote:  @akarl Thanks for the report! This is configurable with allow-case-trailing-whitespace but only to always allow or never allow. Do you suggest making this an integer of number of lines instead or do you want this in addition? Maybe set it to 0 to always allow? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[bombsimon]

So just a heads up, I've been looking into this and I have no idea how I would combine this with allow-trailing-comments. The combination of configuration could get quite messy.

• Allow trailing comments, Force traling whitesapce 0
  You can end a case with a comment, a whitespace or none

• Don't allow trailing comments, Force trailing whitespace 0
  You cannot end a case with a comment but you can end it with a whitespace

• Allow traling comments, Force trailing whitespace 1
  You can end your case with a comment bud you must always end with a whitespace (1 line or more). Should the comment be considered a whitespace? If not, this gets tricky

• Don't allow trailing comments, Force trailing whitespace 1
  You cannot end your case with a comment but you must always end with a whitespace (1 lien or more). Same as above

• Allow traling comment, Force traling whitespace at n>1
  You can end with a comment but you may not have a whitespace if case has more than n lines where n > 1.

• Don't allow trailing comment, Force trailing whitespace at n>1
  You can never end with a trailing comment and you may not have a whitespace if case size is < n, however you must have a whitespace if case size is >= n.

So theres a big combination to start with and everything get's more complicated since the size is calculated by checking the start of the case to the start of the next case. To check for traling whitespaces we check the diff between the next statement and the last statement in the case body. However, if this is more than 1 we don't know if it's comments or just empty lines. Enter the case problem!

So in regular block bodies, the trailing comment(s) is mapped to the last statement in the block. This is not the case for switch/select cases. To get a hold of the trailing comments in cases you must get them by using a case statement (or the actual case expression. This get's even more complicated if you have comments on the same line as the case itself like this:

switch "x" {
case "a": // Tadaa
    fmt.Println("x")
    // Boho
case "b":
    fmt.Println("x")
}

I think this playground simulates the problem quite well.

So now the only option I can think of to get the information about the comments would be to iterate through all comments for the case clause (for each case) and check all comments. If they have a position > the last statement in the case and < the next statement case line they are a part of the trailing case.

This feels ugly, my code got really messy, I access the comment map multiple times and even after that I still need to if-ninja myself to ensure the combination between allowing trailing comments and forcing/forbidding trailing whitespaces.

This makes me wonder if this is actually a good check because... Why would you pick on this? And why force to remove the newlines in some cases but not others? This issue is about forcing to add them, maybe we should stop forcing to remove them (for small blocks).

Please feel free to see if you can come up with a better solution, if you know something about the comment map I'm missing to get a hold of the last comment(s) after the last statement and how you feel about forcing/forbidding newlines in case blocks!

[bombsimon]

@akarl I've created a PR with a proposal but I'm not sure I'm satisfied with the result. The main issue here is that if I configure the current setup with 2 as you proposed that also includes that I'm forced to remove them (like the current setup) if it's only 1 line. Maybe that's what we want? If I always want a newline I can set it to 1 but if I set it to 0 everything is allowed but I will never be forced to add (which this issue suggests).

I know you usually have good ideas so I would love to hear what you think about this and how to implement it? Should there be two options, one from when to force and one from where to forbid? Or should I just drop support to forbid trailing newlines? I mean, <= 2 lines you can chose to have a newline or not but > 2 you must have it (when configured to 2)?

Also, what's your point for view on comments? If you see the PR most of the time went into handling comments. Should a comment be seen as the same as a newline or should it be seen as a separate thing? Right now you can enable to support trailing comments but it's off by default.

All input and testing would be much appreciated! Thanks!

[bombsimon]

I had a really short discussion with @akarl outside of GitHub about this and I think we ended up with the consensus that case blocks are complex and that wsl shouldn't try too hard to find whitespaces both to add and remove. In fact, @akarl had a valid point that the main purpose of this linter is to add newlines so I just removed the checks for when to remove them.

Also, I think that case blocks have far better reasons to end with comments than other blocks so I don't think wsl should not interfere with that.

So the final solution will be to loosen up on case blocks and just add a flag/option to force a max size of a case block (in number of lines) before the end user must add a newline. Since the linter previously forced the end user to remove them no matter what size the block was the first release will have this turned off by default.

[bombsimon]

Resolved from #54