Skip to content

Commit 456abcf

Browse files
authored
Add support for newline-after-block (#221)
* Add support for `newline-after-block` This adds a new check type to enforce newline after any block (closing `}`). In addition, it also makes the check for empty line while using `case-max-lines` a bit more strict by now _not_ counting a trailing comment as an empty line. While doing this, there's a new column based logic to figure out where the newline should go in the end of a case. If it's aligned with the last statement of the case body the line is inserted after. If it's aligned with then next case arm, the line is inserted before the comment. While working on this we're also moving to a line based check that uses `LineStart` to make replacements and with that simplifying some comment parsing since we only need to know the pos where the line starts. This also allowed to add support to fix removal of whitespaces both before _and_ after leading comments in blocks. * Use `ast.Pos` when checking comments We don't need to check line number to find relevant comments, if we instead use the `ast.Pos` as long as possible we save a ton of conversions and gets some speed improvement. * Make `isErrNotNilCheck` check stmt type Removes extra check for statement type before calling function. Also adds a more detailed comment to why we skip complex groups. * Don't create `CommentMap` for file * Improve trailing case comments handling * Add support to remove newline after left-aligned comment Similar to how we do support trailing comments in a block, but no newline after the comment, we want to support left-aligned comments in case bodies but keep them flush against the next case. * Improve method name * Use constant instead of magic number * Improve finding comments after blocks * Consider comments when checking case total lines * Remove more `NewCommentMap` calls * Skip `FuncLit` for newline after block
1 parent 22e3aba commit 456abcf

24 files changed

+3399
-177
lines changed

CHECKS.md

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
- [`inc-dec`](#inc-dec)
1919
- [`label`](#label)
2020
- [`leading-whitespace`](#leading-whitespace)
21+
- [`newline-after-block`](#newline-after-block)
2122
- [`range`](#range)
2223
- [`return`](#return)
2324
- [`select`](#select)
@@ -28,6 +29,7 @@
2829
- [Configuration](#configuration)
2930
- [`allow-first-in-block`](#allow-first-in-block)
3031
- [`allow-whole-block`](#allow-whole-block)
32+
- [`case-max-lines`](#case-max-lines)
3133

3234
## Checks
3335

@@ -1282,6 +1284,68 @@ if true {
12821284

12831285
</tbody></table>
12841286

1287+
### `newline-after-block`
1288+
1289+
Block statements (`if`, `for`, `switch`, etc.) should be followed by a blank
1290+
line to visually separate them from subsequent code.
1291+
1292+
> **NOTE** An exception is made for `defer` statements that follow an
1293+
> `if err != nil` block when the defer references a variable assigned on the
1294+
> line above the if statement. This is a common pattern for resource cleanup.
1295+
1296+
<table>
1297+
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
1298+
<tbody>
1299+
<tr><td valign="top">
1300+
1301+
```go
1302+
if true {
1303+
fmt.Println("hello")
1304+
}
1305+
fmt.Println("world") // 1
1306+
1307+
for i := 0; i < 3; i++ {
1308+
fmt.Println(i)
1309+
}
1310+
x := 1 // 2
1311+
```
1312+
1313+
</td><td valign="top">
1314+
1315+
```go
1316+
if true {
1317+
fmt.Println("hello")
1318+
}
1319+
1320+
fmt.Println("world")
1321+
1322+
for i := 0; i < 3; i++ {
1323+
fmt.Println(i)
1324+
}
1325+
1326+
x := 1
1327+
1328+
// Exception: defer after error check
1329+
f, err := os.Open("file.txt")
1330+
if err != nil {
1331+
return err
1332+
}
1333+
defer f.Close()
1334+
```
1335+
1336+
</td></tr>
1337+
1338+
<tr><td valign="top">
1339+
1340+
<sup>1</sup> Missing whitespace after block
1341+
1342+
<sup>2</sup> Missing whitespace after block
1343+
1344+
</td><td valign="top">
1345+
1346+
</td></tr>
1347+
</tbody></table>
1348+
12851349
### `trailing-whitespace`
12861350

12871351
<table>
@@ -1357,3 +1421,126 @@ if anotherVariable {
13571421
}
13581422
}
13591423
```
1424+
1425+
### `case-max-lines`
1426+
1427+
When set to a value greater than 0, case clauses in `switch` and `select`
1428+
statements that exceed this number of lines will require a blank line before the
1429+
next case. Setting this to 1 will make it always enabled.
1430+
1431+
Comments between case clauses are handled based on their indentation:
1432+
1433+
- **Indented comments** (deeper than `case`) are treated as trailing comments
1434+
that belong to the current case body.
1435+
- **Left-aligned comments** (at the same level as `case`) are treated as leading
1436+
comments that belong to the next case.
1437+
1438+
The blank line is placed at the transition point between trailing and leading
1439+
content. This means:
1440+
1441+
- If all comments are indented, the blank line goes before the next `case`.
1442+
- If all comments are left-aligned, the blank line goes after the last
1443+
statement.
1444+
- If comments transition from indented to left-aligned, the blank line goes at
1445+
the transition point.
1446+
1447+
Additionally, left-aligned comments must be flush against the next `case` - no
1448+
blank line is allowed between them. This ensures consistent formatting where
1449+
leading comments are visually attached to the case they describe.
1450+
1451+
<table>
1452+
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
1453+
<tbody>
1454+
<tr><td valign="top">
1455+
1456+
```go
1457+
switch n {
1458+
case 1:
1459+
fmt.Println("hello")
1460+
case 2: // 1
1461+
fmt.Println("world")
1462+
}
1463+
1464+
switch n {
1465+
case 1:
1466+
fmt.Println("hello")
1467+
// Trailing comment
1468+
case 2: // 2
1469+
fmt.Println("world")
1470+
}
1471+
1472+
switch n {
1473+
case 1:
1474+
fmt.Println("hello")
1475+
// Trailing comment
1476+
// Leading comment
1477+
case 2: // 3
1478+
fmt.Println("world")
1479+
}
1480+
1481+
switch n {
1482+
case 1:
1483+
fmt.Println("hello")
1484+
// Leading comment
1485+
1486+
case 2: // 4
1487+
fmt.Println("world")
1488+
}
1489+
```
1490+
1491+
</td><td valign="top">
1492+
1493+
```go
1494+
switch n {
1495+
case 1:
1496+
fmt.Println("hello")
1497+
1498+
case 2:
1499+
fmt.Println("world")
1500+
}
1501+
1502+
switch n {
1503+
case 1:
1504+
fmt.Println("hello")
1505+
// Trailing comment
1506+
1507+
case 2:
1508+
fmt.Println("world")
1509+
}
1510+
1511+
switch n {
1512+
case 1:
1513+
fmt.Println("hello")
1514+
// Trailing comment
1515+
1516+
// Leading comment
1517+
case 2:
1518+
fmt.Println("world")
1519+
}
1520+
1521+
switch n {
1522+
case 1:
1523+
fmt.Println("hello")
1524+
1525+
// Leading comment
1526+
case 2:
1527+
fmt.Println("world")
1528+
}
1529+
```
1530+
1531+
</td></tr>
1532+
1533+
<tr><td valign="top">
1534+
1535+
<sup>1</sup> Missing blank line after case body
1536+
1537+
<sup>2</sup> Missing blank line after trailing comment
1538+
1539+
<sup>3</sup> Missing blank line at transition (after trailing comment)
1540+
1541+
<sup>4</sup> Unnecessary blank line before case (after leading comment)
1542+
1543+
</td><td valign="top">
1544+
1545+
</td></tr>
1546+
</tbody></table>

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ For more details and examples, see [CHECKS](CHECKS.md).
7373
-**err** - Error checking must follow immediately after the error variable
7474
is assigned
7575
-**leading-whitespace** - Disallow leading empty lines in blocks
76+
-**newline-after-block** - Require empty line after block statements
7677
-**trailing-whitespace** - Disallow trailing empty lines in blocks
7778

7879
### Configuration
@@ -167,6 +168,7 @@ linters:
167168
disable:
168169
- assign-exclusive
169170
- assign-expr
171+
- newline-after-block
170172
```
171173
172174
## See also

analyzer_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,23 @@ func TestWithConfig(t *testing.T) {
108108
config.Checks.Add(CheckAppend)
109109
},
110110
},
111+
{
112+
subdir: "all_enabled",
113+
configFn: func(config *Configuration) {
114+
config.Checks = AllChecks()
115+
config.CaseMaxLines = 1
116+
},
117+
},
118+
{
119+
subdir: "newline_after_block",
120+
configFn: func(config *Configuration) {
121+
config.Checks = NoChecks()
122+
config.Checks.Add(CheckCaseTrailingNewline)
123+
config.Checks.Add(CheckNewlineAfterBlock)
124+
125+
config.CaseMaxLines = 1
126+
},
127+
},
111128
} {
112129
t.Run(tc.subdir, func(t *testing.T) {
113130
t.Parallel()

config.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ const (
7070
CheckErr
7171
CheckLeadingWhitespace
7272
CheckTrailingWhitespace
73+
CheckNewlineAfterBlock
7374

7475
//nolint:godoclint // No need to document
7576
// CheckTypes only used for reporting.
@@ -102,6 +103,7 @@ func (c CheckType) String() string {
102103
"err",
103104
"leading-whitespace",
104105
"trailing-whitespace",
106+
"newline-after-block",
105107
//
106108
"case-trailing-newline",
107109
}[c]
@@ -211,6 +213,7 @@ func AllChecks() CheckSet {
211213
c := DefaultChecks()
212214
c.Add(CheckAssignExclusive)
213215
c.Add(CheckAssignExpr)
216+
c.Add(CheckNewlineAfterBlock)
214217

215218
return c
216219
}
@@ -274,6 +277,8 @@ func CheckFromString(s string) (CheckType, error) {
274277
return CheckLeadingWhitespace, nil
275278
case "trailing-whitespace":
276279
return CheckTrailingWhitespace, nil
280+
case "newline-after-block":
281+
return CheckNewlineAfterBlock, nil
277282
default:
278283
return CheckInvalid, fmt.Errorf("invalid check '%s'", s)
279284
}

config_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ func TestCheckSet(t *testing.T) {
8383
func TestToAndFromString(t *testing.T) {
8484
t.Parallel()
8585

86-
maxCheckNumber := 23
86+
maxCheckNumber := 24
8787

8888
for n := range maxCheckNumber {
8989
check := CheckType(n)

testdata/src/default_config/whitespace/leading_whitespace.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,26 @@ func commentThenLeadingWhitespace() { // want +2 `unnecessary whitespace \(leadi
2525
fmt.Println("Hello, World")
2626
}
2727

28+
// want +3 `unnecessary whitespace \(leading-whitespace\)`
29+
// want +4 `unnecessary whitespace \(leading-whitespace\)`
30+
func whitespaceBeforeAndAfterComment() {
31+
32+
// Whitespace before and after comment
33+
34+
fmt.Println("Hello, World")
35+
}
36+
37+
// want +3 `unnecessary whitespace \(leading-whitespace\)`
38+
// want +6 `unnecessary whitespace \(leading-whitespace\)`
39+
func whitespaceBeforeAndAfterMultpleComment() {
40+
41+
// Whitespace before and after comment
42+
43+
// Some more comments
44+
45+
fmt.Println("Hello, World")
46+
}
47+
2848
func noWhitespace() {
2949
fmt.Println("Hello, World")
3050
}

testdata/src/default_config/whitespace/leading_whitespace.go.golden

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,22 @@ func commentThenLeadingWhitespace() { // want +2 `unnecessary whitespace \(leadi
2222
fmt.Println("Hello, World")
2323
}
2424

25+
// want +3 `unnecessary whitespace \(leading-whitespace\)`
26+
// want +4 `unnecessary whitespace \(leading-whitespace\)`
27+
func whitespaceBeforeAndAfterComment() {
28+
// Whitespace before and after comment
29+
fmt.Println("Hello, World")
30+
}
31+
32+
// want +3 `unnecessary whitespace \(leading-whitespace\)`
33+
// want +6 `unnecessary whitespace \(leading-whitespace\)`
34+
func whitespaceBeforeAndAfterMultpleComment() {
35+
// Whitespace before and after comment
36+
37+
// Some more comments
38+
fmt.Println("Hello, World")
39+
}
40+
2541
func noWhitespace() {
2642
fmt.Println("Hello, World")
2743
}

0 commit comments

Comments
 (0)