Skip to content

Commit de506f6

Browse files
committed
formatter: separate select case clauses
Treat select communication clauses like switch case clauses in the blank-line pass so separator blanks appear between clauses rather than inside the clause header. Suppress blank-before-return insertion when a case or select clause contains only a return statement, even when that return has a leading comment. Add a regression test for select clauses that keeps the first case compact, separates the next case, and preserves the existing blank-before-return behavior for a multi-statement final case.
1 parent 437e389 commit de506f6

4 files changed

Lines changed: 108 additions & 6 deletions

File tree

dsl/action.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5194,6 +5194,12 @@ func (c *blankLineBatchContext) inspectFile(file *ast.File) {
51945194
c.maybeInsertBlankBefore(n)
51955195
}
51965196

5197+
case *ast.CommClause:
5198+
c.maybeRemoveBlankAfterSingleLineBlockHeader(n)
5199+
if c.caseCond.Eval(caps, c.ctx) {
5200+
c.maybeInsertBlankBefore(n)
5201+
}
5202+
51975203
case *ast.ReturnStmt:
51985204
if onlyStmtInBlockWithoutLeadingComment(
51995205
c.ctx, n,
@@ -5275,6 +5281,14 @@ func (c *blankLineBatchContext) maybeRemoveBlankAfterSingleLineBlockHeader(
52755281
headerStart = c.ctx.Fset.Position(n.Pos())
52765282
headerEnd = c.ctx.Fset.Position(n.Colon)
52775283

5284+
case *ast.CommClause:
5285+
if n == nil || len(n.Body) == 0 {
5286+
return
5287+
}
5288+
first = n.Body[0]
5289+
headerStart = c.ctx.Fset.Position(n.Pos())
5290+
headerEnd = c.ctx.Fset.Position(n.Colon)
5291+
52785292
default:
52795293
return
52805294
}

dsl/condition.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,8 +1103,8 @@ func (c *IsFinalReturnCond) Eval(caps Captures, ctx *Context) bool {
11031103
}
11041104
}
11051105

1106-
// HasPrecedingSiblingCond checks if a case clause has a preceding sibling case.
1107-
// Used to determine if blank line is needed before a case.
1106+
// HasPrecedingSiblingCond checks if a switch/select clause has a preceding
1107+
// sibling clause. Used to determine if a blank line is needed before a clause.
11081108
type HasPrecedingSiblingCond struct {
11091109
Target string
11101110
}
@@ -1116,8 +1116,10 @@ func (c *HasPrecedingSiblingCond) Eval(caps Captures, ctx *Context) bool {
11161116
return false
11171117
}
11181118

1119-
caseClause, ok := node.(*ast.CaseClause)
1120-
if !ok {
1119+
switch node.(type) {
1120+
case *ast.CaseClause, *ast.CommClause:
1121+
1122+
default:
11211123
return false
11221124
}
11231125

@@ -1133,9 +1135,9 @@ func (c *HasPrecedingSiblingCond) Eval(caps Captures, ctx *Context) bool {
11331135
return false
11341136
}
11351137

1136-
// Find this case in the block's statements
1138+
// Find this clause in the block's statements.
11371139
for i, stmt := range blockStmt.List {
1138-
if stmt == caseClause {
1140+
if stmt == node {
11391141
return i > 0 // Has preceding sibling if not first
11401142
}
11411143
}
@@ -2115,6 +2117,9 @@ func (c *IsReturnNeedingBlankCond) Eval(caps Captures, ctx *Context) bool {
21152117
if !ok {
21162118
return false
21172119
}
2120+
if onlyStmtInClause(ctx, node) {
2121+
return false
2122+
}
21182123
if onlyStmtInBlockWithoutLeadingComment(ctx, node) {
21192124
return false
21202125
}
@@ -2167,6 +2172,21 @@ func (c *IsReturnNeedingBlankCond) Eval(caps Captures, ctx *Context) bool {
21672172
return true
21682173
}
21692174

2175+
func onlyStmtInClause(ctx *Context, node ast.Node) bool {
2176+
switch parent := ctx.Parent(node).(type) {
2177+
case *ast.CaseClause:
2178+
return parent != nil && len(parent.Body) == 1 &&
2179+
parent.Body[0] == node
2180+
2181+
case *ast.CommClause:
2182+
return parent != nil && len(parent.Body) == 1 &&
2183+
parent.Body[0] == node
2184+
2185+
default:
2186+
return false
2187+
}
2188+
}
2189+
21702190
func onlyStmtInBlockWithoutLeadingComment(ctx *Context, node ast.Node) bool {
21712191
block, ok := ctx.Parent(node).(*ast.BlockStmt)
21722192
if !ok || block == nil || len(block.List) != 1 ||

dsl/rules.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,23 @@ func BlankLineRulesWithOptions(opts BlankLineOptions) []Rule {
921921
Target: "node",
922922
},
923923
},
924+
{
925+
Name: "blank_before_comm_clause",
926+
Pattern: &NodePattern{
927+
Type: "CommClause",
928+
},
929+
When: &AndCond{
930+
Conds: []Condition{
931+
&HasPrecedingSiblingCond{
932+
Target: "node",
933+
},
934+
},
935+
},
936+
Priority: 10,
937+
Action: &InsertBlankBeforeAction{
938+
Target: "node",
939+
},
940+
},
924941

925942
// Rule: Blank line before return (if not after block open or
926943
// case:)

formatter/pipeline_dsl_blanklines_extras_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,57 @@ func record() {}
169169
require.NoError(t, err)
170170
}
171171

172+
func TestDSLBlankLinesNative_SelectCasesSeparateClausesNotHeaders(
173+
t *testing.T) {
174+
175+
const in = `package p
176+
177+
import "context"
178+
179+
func f(ctx context.Context, done chan struct{}) error {
180+
select {
181+
case <-done:
182+
// Work completed.
183+
return nil
184+
case <-ctx.Done():
185+
// Context cancelled before work finished.
186+
close(done)
187+
return ctx.Err()
188+
}
189+
}
190+
`
191+
192+
p := NewPipeline(
193+
PipelineConfig{
194+
ColumnLimit: 80,
195+
TabStop: 8,
196+
UseDSLBlankLines: true,
197+
UseDSLBlankLinesNative: true,
198+
},
199+
)
200+
201+
first := p.Format([]byte(in))
202+
second := p.Format(first)
203+
require.Equal(t, string(first), string(second))
204+
205+
out := string(first)
206+
require.Contains(
207+
t, out, "case <-done:\n // Work "+
208+
"completed.\n return nil\n\n case "+
209+
"<-ctx.Done():",
210+
)
211+
require.Contains(
212+
t, out,
213+
"close(done)\n\n\t\treturn ctx.Err()\n\t}",
214+
)
215+
require.NotContains(t, out, "case <-done:\n\n // Work "+
216+
"completed.")
217+
218+
fset := token.NewFileSet()
219+
_, err := parser.ParseFile(fset, "out.go", first, parser.AllErrors)
220+
require.NoError(t, err)
221+
}
222+
172223
func TestDSLBlankLinesNative_DoesNotBlankBeforeOnlyWrappedReturn(t *testing.T) {
173224
const in = `package p
174225

0 commit comments

Comments
 (0)