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.

Author: bhandras

dsl/action.go

@@ -5163,6 +5163,18 @@ func (c *blankLineBatchContext) initConditions() {
 }
 
 func (c *blankLineBatchContext) maybeInsertBlankBefore(n ast.Node) {
+	c.maybeInsertBlankBeforeNode(n, true)
+}
+
+func (c *blankLineBatchContext) maybeInsertBlankBeforeClause(n ast.Node) {
+	c.maybeInsertBlankBeforeNode(
+		n, shouldAttachLeadingCommentsForClause(c.ctx, n),
+	)
+}
+
+func (c *blankLineBatchContext) maybeInsertBlankBeforeNode(n ast.Node,
+	attachLeadingComments bool) {
+
 	if n == nil {
 		return
 	}
@@ -5171,7 +5183,9 @@ func (c *blankLineBatchContext) maybeInsertBlankBefore(n ast.Node) {
 		return
 	}
 	ls := lineStart(c.ctx.Source, start)
-	ls = leadingCommentBlockLineStart(c.ctx.Source, ls)
+	if attachLeadingComments {
+		ls = leadingCommentBlockLineStart(c.ctx.Source, ls)
+	}
 	if ls <= 0 || hasBlankLineBeforeLineStart(c.ctx.Source, ls) {
 		return
 	}
@@ -5191,7 +5205,13 @@ func (c *blankLineBatchContext) inspectFile(file *ast.File) {
 			case *ast.CaseClause:
 				c.maybeRemoveBlankAfterSingleLineBlockHeader(n)
 				if c.caseCond.Eval(caps, c.ctx) {
-					c.maybeInsertBlankBefore(n)
+					c.maybeInsertBlankBeforeClause(n)
+				}
+
+			case *ast.CommClause:
+				c.maybeRemoveBlankAfterSingleLineBlockHeader(n)
+				if c.caseCond.Eval(caps, c.ctx) {
+					c.maybeInsertBlankBeforeClause(n)
 				}
 
 			case *ast.ReturnStmt:
@@ -5275,6 +5295,14 @@ func (c *blankLineBatchContext) maybeRemoveBlankAfterSingleLineBlockHeader(
 		headerStart = c.ctx.Fset.Position(n.Pos())
 		headerEnd = c.ctx.Fset.Position(n.Colon)
 
+	case *ast.CommClause:
+		if n == nil || len(n.Body) == 0 {
+			return
+		}
+		first = n.Body[0]
+		headerStart = c.ctx.Fset.Position(n.Pos())
+		headerEnd = c.ctx.Fset.Position(n.Colon)
+
 	default:
 		return
 	}
@@ -5468,6 +5496,79 @@ func (a *InsertBlankBeforeAction) Execute(caps Captures, ctx *Context) ([]byte,
 	return out, true
 }
 
+// InsertBlankBeforeClauseAction inserts a blank line before a switch/select
+// clause label.
+type InsertBlankBeforeClauseAction struct {
+	Target string
+}
+
+// Execute implements Action for InsertBlankBeforeClauseAction.
+func (a *InsertBlankBeforeClauseAction) Execute(caps Captures, ctx *Context) (
+	[]byte, bool) {
+
+	node := resolveTarget(caps, a.Target)
+	if node == nil {
+		return nil, false
+	}
+
+	pos := ctx.Fset.Position(node.Pos())
+	ls := lineStart(ctx.Source, pos.Offset)
+	if shouldAttachLeadingCommentsForClause(ctx, node) {
+		ls = leadingCommentBlockLineStart(ctx.Source, ls)
+	}
+	if hasBlankLineBeforeLineStart(ctx.Source, ls) {
+		return nil, false
+	}
+
+	out, err := ApplySingleEdit(ctx.Source, ls, ls, []byte("\n"))
+	if err != nil {
+		return nil, false
+	}
+
+	return out, true
+}
+
+func shouldAttachLeadingCommentsForClause(ctx *Context, node ast.Node) bool {
+	parent, ok := ctx.Parent(node).(*ast.BlockStmt)
+	if !ok || parent == nil {
+		return true
+	}
+
+	for i, stmt := range parent.List {
+		if stmt != node {
+			continue
+		}
+		if i == 0 {
+			return true
+		}
+
+		return clauseBodyLen(parent.List[i-1]) > 0
+	}
+
+	return true
+}
+
+func clauseBodyLen(stmt ast.Stmt) int {
+	switch n := stmt.(type) {
+	case *ast.CaseClause:
+		if n == nil {
+			return 0
+		}
+
+		return len(n.Body)
+
+	case *ast.CommClause:
+		if n == nil {
+			return 0
+		}
+
+		return len(n.Body)
+
+	default:
+		return 0
+	}
+}
+
 // InsertBlankAfterAction inserts a blank line after a node if not already
 // present.
 type InsertBlankAfterAction struct {

dsl/condition.go

@@ -215,6 +215,7 @@ func (c *ExprEditSafeCond) Eval(caps Captures, ctx *Context) bool {
 		case CallArgsPolicyForce:
 
 			// OK.
+
 		case CallArgsPolicyAuto:
 			call := enclosingCallForArg(node, ctx)
 			if call == nil {
@@ -1103,8 +1104,8 @@ func (c *IsFinalReturnCond) Eval(caps Captures, ctx *Context) bool {
 	}
 }
 
-// HasPrecedingSiblingCond checks if a case clause has a preceding sibling case.
-// Used to determine if blank line is needed before a case.
+// HasPrecedingSiblingCond checks if a switch/select clause has a preceding
+// sibling clause. Used to determine if a blank line is needed before a clause.
 type HasPrecedingSiblingCond struct {
 	Target string
 }
@@ -1116,8 +1117,10 @@ func (c *HasPrecedingSiblingCond) Eval(caps Captures, ctx *Context) bool {
 		return false
 	}
 
-	caseClause, ok := node.(*ast.CaseClause)
-	if !ok {
+	switch node.(type) {
+	case *ast.CaseClause, *ast.CommClause:
+
+	default:
 		return false
 	}
 
@@ -1133,9 +1136,9 @@ func (c *HasPrecedingSiblingCond) Eval(caps Captures, ctx *Context) bool {
 		return false
 	}
 
-	// Find this case in the block's statements
+	// Find this clause in the block's statements.
 	for i, stmt := range blockStmt.List {
-		if stmt == caseClause {
+		if stmt == node {
 			return i > 0 // Has preceding sibling if not first
 		}
 	}
@@ -2115,6 +2118,9 @@ func (c *IsReturnNeedingBlankCond) Eval(caps Captures, ctx *Context) bool {
 	if !ok {
 		return false
 	}
+	if onlyStmtInClause(ctx, node) {
+		return false
+	}
 	if onlyStmtInBlockWithoutLeadingComment(ctx, node) {
 		return false
 	}
@@ -2167,6 +2173,21 @@ func (c *IsReturnNeedingBlankCond) Eval(caps Captures, ctx *Context) bool {
 	return true
 }
 
+func onlyStmtInClause(ctx *Context, node ast.Node) bool {
+	switch parent := ctx.Parent(node).(type) {
+	case *ast.CaseClause:
+		return parent != nil && len(parent.Body) == 1 &&
+			parent.Body[0] == node
+
+	case *ast.CommClause:
+		return parent != nil && len(parent.Body) == 1 &&
+			parent.Body[0] == node
+
+	default:
+		return false
+	}
+}
+
 func onlyStmtInBlockWithoutLeadingComment(ctx *Context, node ast.Node) bool {
 	block, ok := ctx.Parent(node).(*ast.BlockStmt)
 	if !ok || block == nil || len(block.List) != 1 ||

dsl/func_lit_expand_action.go

@@ -124,6 +124,7 @@ func isTrivialInlineFuncLit(fn *ast.FuncLit) bool {
 		case *ast.Ident, *ast.BasicLit:
 
 			// OK.
+
 		default:
 			return false
 		}

dsl/layout/render.go

@@ -70,6 +70,7 @@ func RenderAt(doc Doc, colLimit int, tabStop int, indent string,
 		case ForceBreak:
 
 			// No output; acts only as a fit-check barrier.
+
 		case Concat:
 			// Push in reverse so we render in order.
 			for i := len(d) - 1; i >= 0; i-- {
@@ -194,6 +195,7 @@ func fitsAt(doc Doc, remaining int, tabStop int, startCol int) bool {
 		case SoftLine:
 
 			// Flat soft line is empty.
+
 		case ForceBreak:
 			return false
 

dsl/rules.go

@@ -917,7 +917,24 @@ func BlankLineRulesWithOptions(opts BlankLineOptions) []Rule {
 				},
 			},
 			Priority: 10,
-			Action: &InsertBlankBeforeAction{
+			Action: &InsertBlankBeforeClauseAction{
+				Target: "node",
+			},
+		},
+		{
+			Name: "blank_before_comm_clause",
+			Pattern: &NodePattern{
+				Type: "CommClause",
+			},
+			When: &AndCond{
+				Conds: []Condition{
+					&HasPrecedingSiblingCond{
+						Target: "node",
+					},
+				},
+			},
+			Priority: 10,
+			Action: &InsertBlankBeforeClauseAction{
 				Target: "node",
 			},
 		},

formatter/pipeline_dsl_blanklines_extras_test.go

@@ -169,6 +169,77 @@ func record() {}
 	require.NoError(t, err)
 }
 
+func TestDSLBlankLinesNative_SelectCasesSeparateClausesNotHeaders(
+	t *testing.T) {
+
+	const in = `package p
+
+import "context"
+
+func f(ctx context.Context, done chan struct{}) error {
+	select {
+	case <-done:
+		// Work completed.
+		return nil
+	case <-ctx.Done():
+		// Context cancelled before work finished.
+		close(done)
+		return ctx.Err()
+	}
+}
+`
+
+	p := NewPipeline(
+		PipelineConfig{
+			ColumnLimit:            80,
+			TabStop:                8,
+			UseDSLBlankLines:       true,
+			UseDSLBlankLinesNative: true,
+		},
+	)
+
+	first := p.Format([]byte(in))
+	second := p.Format(first)
+	require.Equal(t, string(first), string(second))
+
+	out := string(first)
+	require.Contains(
+		t, out, "case <-done:\n		// Work "+
+			"completed.\n		return nil\n\n	case "+
+			"<-ctx.Done():",
+	)
+	require.Contains(
+		t, out,
+		"close(done)\n\n\t\treturn ctx.Err()\n\t}",
+	)
+	require.NotContains(t, out, "case <-done:\n\n		// Work "+
+		"completed.")
+
+	const commentOnlyCase = `package p
+
+func f(done chan struct{}, cancel chan struct{}) {
+	select {
+	case <-done:
+		// Good.
+	case <-cancel:
+		return
+	}
+}
+`
+
+	commentOnlyOut := string(p.Format([]byte(commentOnlyCase)))
+	require.Contains(
+		t, commentOnlyOut,
+		"case <-done:\n		// Good.\n\n	case <-cancel:",
+	)
+	require.NotContains(t, commentOnlyOut, "case "+
+		"<-done:\n\n		// Good.")
+
+	fset := token.NewFileSet()
+	_, err := parser.ParseFile(fset, "out.go", first, parser.AllErrors)
+	require.NoError(t, err)
+}
+
 func TestDSLBlankLinesNative_DoesNotBlankBeforeOnlyWrappedReturn(t *testing.T) {
 	const in = `package p