Skip to content

Commit a7bc7b7

Browse files
bombsimonbombsimon
committed
Improve trailing case comments handling
1 parent 63c7e0d commit a7bc7b7

File tree

3 files changed

+167
-74
lines changed

3 files changed

+167
-74
lines changed

‎testdata/src/with_config/case_max_lines/case.go‎

Lines changed: 71 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package testpkg
22

33
import "context"
44

5-
func fn1(n int) {
5+
func noComments(n int) {
66
switch n {
77
case 1:
88
n++
@@ -13,12 +13,6 @@ func fn1(n int) {
1313
n++
1414
n++
1515
n++ // want `missing whitespace below this line \(case-trailing-newline\)`
16-
case 4:
17-
n++
18-
n++
19-
n++
20-
n++ // want +1 `missing whitespace below this line \(case-trailing-newline\)`
21-
// Trailing comment
2216
default:
2317
n++
2418
n++
@@ -27,7 +21,7 @@ func fn1(n int) {
2721
}
2822
}
2923

30-
func fn2(ctx context.Context, ch1 chan struct{}) {
24+
func selectStmt(ctx context.Context, ch1 chan struct{}) {
3125
select {
3226
case <-ctx.Done():
3327
_ = 1
@@ -40,40 +34,97 @@ func fn2(ctx context.Context, ch1 chan struct{}) {
4034
}
4135
}
4236

43-
// Test comment permutations: no comments, leading only, trailing only, both (single group).
44-
func fnCommentPermutations(n int) {
37+
// Single comment: leading (left-aligned) or trailing (indented).
38+
func singleComment(n int) {
4539
switch n {
4640
case 1:
4741
n++
4842
n++
4943
n++ // want `missing whitespace below this line \(case-trailing-newline\)`
50-
// Leading comment
44+
// Leading comment (left-aligned)
5145
case 2:
5246
n++
5347
n++
5448
n++ // want +1 `missing whitespace below this line \(case-trailing-newline\)`
55-
// Trailing comment
56-
case 3:
49+
// Trailing comment (indented)
50+
default:
51+
n++
52+
}
53+
}
54+
55+
// All comments indented (trailing) - blank line goes before next case.
56+
func allIndented(n int) {
57+
switch n {
58+
case 1:
5759
n++
5860
n++
59-
n++ // want +2 `missing whitespace below this line \(case-trailing-newline\)`
60-
// Trailing comment
61-
// Leading comment (same group as above - adjacent)
61+
n++ // want +3 `missing whitespace below this line \(case-trailing-newline\)`
62+
// First group
63+
64+
// Second group (both indented)
6265
default:
6366
n++
6467
}
6568
}
6669

67-
// Test multiple comment groups - should be skipped (no report).
68-
func fnMultipleCommentGroups(n int) {
70+
// All comments left-aligned (leading) - blank line goes after last statement.
71+
func allLeftAligned(n int) {
6972
switch n {
7073
case 1:
7174
n++
7275
n++
76+
n++ // want `missing whitespace below this line \(case-trailing-newline\)`
77+
// First group
78+
79+
// Second group (both left-aligned)
80+
default:
7381
n++
74-
// First group
82+
}
83+
}
84+
85+
// Mixed groups - first indented, second left-aligned.
86+
// Already has blank line at transition - no report.
87+
func mixedGroupsCorrect(n int) {
88+
switch n {
89+
case 1:
90+
n++
91+
n++
92+
n++
93+
// Indented group
94+
95+
// Left-aligned group
96+
case 2:
97+
n++
98+
n++
99+
n++
100+
// Indented group
101+
102+
// Left-aligned group (unnecessary blank line after this - not currently checked)
103+
104+
default:
105+
n++
106+
}
107+
}
108+
109+
// Mixed groups without blank line - needs fix.
110+
func mixedGroupsNeedsFix(n int) {
111+
switch n {
112+
case 1:
113+
n++
114+
n++
115+
n++ // want +1 `missing whitespace below this line \(case-trailing-newline\)`
116+
// Indented group
117+
// Left-aligned group
118+
case 2:
119+
n++
120+
n++
121+
n++ // want +3 `missing whitespace below this line \(case-trailing-newline\)`
122+
// Indented group 1
123+
124+
// Indented group 2
125+
// Left-aligned group 1
75126

76-
// Second group (separated by blank line)
127+
// Left-aligned group 2
77128
default:
78129
n++
79130
}

‎testdata/src/with_config/case_max_lines/case.go.golden‎

Lines changed: 74 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package testpkg
22

33
import "context"
44

5-
func fn1(n int) {
5+
func noComments(n int) {
66
switch n {
77
case 1:
88
n++
@@ -14,13 +14,6 @@ func fn1(n int) {
1414
n++
1515
n++ // want `missing whitespace below this line \(case-trailing-newline\)`
1616

17-
case 4:
18-
n++
19-
n++
20-
n++
21-
n++ // want +1 `missing whitespace below this line \(case-trailing-newline\)`
22-
// Trailing comment
23-
2417
default:
2518
n++
2619
n++
@@ -29,7 +22,7 @@ func fn1(n int) {
2922
}
3023
}
3124

32-
func fn2(ctx context.Context, ch1 chan struct{}) {
25+
func selectStmt(ctx context.Context, ch1 chan struct{}) {
3326
select {
3427
case <-ctx.Done():
3528
_ = 1
@@ -43,43 +36,103 @@ func fn2(ctx context.Context, ch1 chan struct{}) {
4336
}
4437
}
4538

46-
// Test comment permutations: no comments, leading only, trailing only, both (single group).
47-
func fnCommentPermutations(n int) {
39+
// Single comment: leading (left-aligned) or trailing (indented).
40+
func singleComment(n int) {
4841
switch n {
4942
case 1:
5043
n++
5144
n++
5245
n++ // want `missing whitespace below this line \(case-trailing-newline\)`
5346

54-
// Leading comment
47+
// Leading comment (left-aligned)
5548
case 2:
5649
n++
5750
n++
5851
n++ // want +1 `missing whitespace below this line \(case-trailing-newline\)`
59-
// Trailing comment
52+
// Trailing comment (indented)
6053

61-
case 3:
54+
default:
55+
n++
56+
}
57+
}
58+
59+
// All comments indented (trailing) - blank line goes before next case.
60+
func allIndented(n int) {
61+
switch n {
62+
case 1:
6263
n++
6364
n++
64-
n++ // want +2 `missing whitespace below this line \(case-trailing-newline\)`
65-
// Trailing comment
66-
// Leading comment (same group as above - adjacent)
65+
n++ // want +3 `missing whitespace below this line \(case-trailing-newline\)`
66+
// First group
67+
68+
// Second group (both indented)
6769

6870
default:
6971
n++
7072
}
7173
}
7274

73-
// Test multiple comment groups - should be skipped (no report).
74-
func fnMultipleCommentGroups(n int) {
75+
// All comments left-aligned (leading) - blank line goes after last statement.
76+
func allLeftAligned(n int) {
7577
switch n {
7678
case 1:
7779
n++
7880
n++
81+
n++ // want `missing whitespace below this line \(case-trailing-newline\)`
82+
83+
// First group
84+
85+
// Second group (both left-aligned)
86+
default:
7987
n++
80-
// First group
88+
}
89+
}
90+
91+
// Mixed groups - first indented, second left-aligned.
92+
// Already has blank line at transition - no report.
93+
func mixedGroupsCorrect(n int) {
94+
switch n {
95+
case 1:
96+
n++
97+
n++
98+
n++
99+
// Indented group
100+
101+
// Left-aligned group
102+
case 2:
103+
n++
104+
n++
105+
n++
106+
// Indented group
107+
108+
// Left-aligned group (unnecessary blank line after this - not currently checked)
109+
110+
default:
111+
n++
112+
}
113+
}
114+
115+
// Mixed groups without blank line - needs fix.
116+
func mixedGroupsNeedsFix(n int) {
117+
switch n {
118+
case 1:
119+
n++
120+
n++
121+
n++ // want +1 `missing whitespace below this line \(case-trailing-newline\)`
122+
// Indented group
123+
124+
// Left-aligned group
125+
case 2:
126+
n++
127+
n++
128+
n++ // want +3 `missing whitespace below this line \(case-trailing-newline\)`
129+
// Indented group 1
130+
131+
// Indented group 2
132+
133+
// Left-aligned group 1
81134

82-
// Second group (separated by blank line)
135+
// Left-aligned group 2
83136
default:
84137
n++
85138
}

‎wsl.go‎

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -975,53 +975,42 @@ func (w *WSL) checkCaseTrailingNewline(body []ast.Stmt, cursor *Cursor) {
975975
nextCaseLine := w.lineFor(nextCase.Pos())
976976
nextCaseCol := w.fset.PositionFor(nextCase.Pos(), false).Column
977977

978-
// Find the effective end of case body content (including trailing comments)
979-
// and the start of next case content (leading comment or case itself).
980-
// Trailing comments are indented like statements (col > case col).
981-
// Leading comments are at case indentation (col <= case col).
978+
// Find transition point between trailing content (indented) and leading
979+
// content (left-aligned). Trailing comments belong to current case, leading
980+
// comments belong to next case. The blank line goes at the transition.
982981
contentEndLine := lastStmtEndLine
983982
contentEndPos := lastStmt.End()
984983
nextContentPos := nextCase.Pos()
985984
nextContentLine := nextCaseLine
986985

987-
// Filter comments between last statement and next case only.
988-
var commentGroups []*ast.CommentGroup
989-
990-
for _, cg := range w.file.Comments {
991-
if cg.Pos() >= nextCase.Pos() {
986+
COMMENTS:
987+
for _, commentGroup := range w.file.Comments {
988+
if commentGroup.Pos() >= nextCase.Pos() {
992989
break
993990
}
994991

995-
if cg.Pos() <= lastStmt.End() {
992+
if commentGroup.End() <= lastStmt.End() {
996993
continue
997994
}
998995

999-
cgLine := w.lineFor(cg.Pos())
1000-
if cgLine > lastStmtEndLine && cgLine < nextCaseLine {
1001-
commentGroups = append(commentGroups, cg)
1002-
}
1003-
}
996+
for _, comment := range commentGroup.List {
997+
commentLine := w.lineFor(comment.Pos())
998+
if commentLine <= lastStmtEndLine || commentLine >= nextCaseLine {
999+
continue
1000+
}
10041001

1005-
// Multiple comment groups are left as-is and does not add any report. This
1006-
// is because multiple groups can have different indentation and it's hard
1007-
// and costly to try to figure out where in such situation we should add the
1008-
// empty line.
1009-
if len(commentGroups) > 1 {
1010-
return
1011-
}
1002+
col := w.fset.PositionFor(comment.Pos(), false).Column
1003+
if col <= nextCaseCol {
1004+
// Left-aligned: blank line goes before this
1005+
nextContentPos = comment.Pos()
1006+
nextContentLine = commentLine
10121007

1013-
if len(commentGroups) == 1 {
1014-
cg := commentGroups[0]
1015-
commentCol := w.fset.PositionFor(cg.Pos(), false).Column
1008+
break COMMENTS
1009+
}
10161010

1017-
if commentCol > nextCaseCol {
1018-
// Trailing comment (indented like statements) - blank line goes after it.
1019-
contentEndLine = w.lineFor(cg.End())
1020-
contentEndPos = cg.End()
1021-
} else {
1022-
// Leading comment (indented like case) - blank line goes before it.
1023-
nextContentPos = cg.Pos()
1024-
nextContentLine = w.lineFor(cg.Pos())
1011+
// Indented: extend trailing content
1012+
contentEndPos = comment.End()
1013+
contentEndLine = w.lineFor(comment.End())
10251014
}
10261015
}
10271016

0 commit comments

Comments
 (0)