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

@@ -1010,7 +1010,6 @@ func (a *BreakArithmeticChainLayoutAction) Execute(caps Captures,
 
 	switch binExpr.Op {
 	case token.ADD, token.SUB, token.MUL, token.QUO, token.REM:
-
 	default:
 		return nil, false
 	}
@@ -5163,6 +5162,21 @@ func (c *blankLineBatchContext) initConditions() {
 }
 
 func (c *blankLineBatchContext) maybeInsertBlankBefore(n ast.Node) {
+	c.maybeInsertBlankBeforeNode(n, true)
+}
+
+func (c *blankLineBatchContext) maybeInsertBlankBeforeClause(n ast.Node) {
+	insert, attachLeadingComments := clauseSeparatorBehavior(c.ctx, n)
+	if !insert {
+		return
+	}
+
+	c.maybeInsertBlankBeforeNode(n, attachLeadingComments)
+}
+
+func (c *blankLineBatchContext) maybeInsertBlankBeforeNode(n ast.Node,
+	attachLeadingComments bool) {
+
 	if n == nil {
 		return
 	}
@@ -5171,7 +5185,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 +5207,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:
@@ -5268,12 +5290,34 @@ func (c *blankLineBatchContext) maybeRemoveBlankAfterSingleLineBlockHeader(
 		headerEnd = c.ctx.Fset.Position(n.Body.Lbrace)
 
 	case *ast.CaseClause:
-		if n == nil || len(n.Body) == 0 {
+		if n == nil {
+			return
+		}
+		headerStart = c.ctx.Fset.Position(n.Pos())
+		headerEnd = c.ctx.Fset.Position(n.Colon)
+		if len(n.Body) == 0 {
+			c.maybeRemoveBlankAfterHeaderLine(
+				headerStart, headerEnd,
+			)
+
 			return
 		}
 		first = n.Body[0]
+
+	case *ast.CommClause:
+		if n == nil {
+			return
+		}
 		headerStart = c.ctx.Fset.Position(n.Pos())
 		headerEnd = c.ctx.Fset.Position(n.Colon)
+		if len(n.Body) == 0 {
+			c.maybeRemoveBlankAfterHeaderLine(
+				headerStart, headerEnd,
+			)
+
+			return
+		}
+		first = n.Body[0]
 
 	default:
 		return
@@ -5300,6 +5344,31 @@ func (c *blankLineBatchContext) maybeRemoveBlankAfterSingleLineBlockHeader(
 	c.maybeRemoveBlankBeforeLineStart(lineStartIdx)
 }
 
+func (c *blankLineBatchContext) maybeRemoveBlankAfterHeaderLine(
+	headerStart, headerEnd token.Position) {
+
+	if headerStart.Line != headerEnd.Line {
+		return
+	}
+
+	headerLineEnd := lineEnd(c.ctx.Source, headerEnd.Offset)
+	if headerLineEnd < 0 || headerLineEnd >= len(c.ctx.Source) ||
+		c.ctx.Source[headerLineEnd] != '\n' {
+		return
+	}
+
+	blankLineStart := headerLineEnd + 1
+	blankLineEnd := lineEnd(c.ctx.Source, blankLineStart)
+	if blankLineEnd < blankLineStart || blankLineEnd >= len(c.ctx.Source) {
+		return
+	}
+	if !isWhitespaceOnlyLine(c.ctx.Source[blankLineStart:blankLineEnd]) {
+		return
+	}
+
+	c.b.Delete(blankLineStart, blankLineEnd+1)
+}
+
 func (c *blankLineBatchContext) maybeRemoveBlankBeforeLineStart(
 	lineStartIdx int) {
 
@@ -5468,6 +5537,130 @@ 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())
+	insert, attachLeadingComments := clauseSeparatorBehavior(ctx, node)
+	if !insert {
+		return nil, false
+	}
+
+	ls := lineStart(ctx.Source, pos.Offset)
+	if attachLeadingComments {
+		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 clauseSeparatorBehavior(ctx *Context,
+	node ast.Node) (insert bool, attachLeadingComments bool) {
+
+	parent, ok := ctx.Parent(node).(*ast.BlockStmt)
+	if !ok || parent == nil {
+		return true, true
+	}
+
+	for i, stmt := range parent.List {
+		if stmt != node {
+			continue
+		}
+		if i == 0 {
+			return true, true
+		}
+
+		prev := parent.List[i-1]
+		if clauseBodyLen(prev) > 0 {
+			return true, true
+		}
+
+		if clauseGapHasComment(ctx, prev, node) {
+			return true, false
+		}
+
+		return false, false
+	}
+
+	return true, 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
+	}
+}
+
+func clauseGapHasComment(ctx *Context, prev ast.Stmt, next ast.Node) bool {
+	start := clauseColonOffset(ctx, prev)
+	end := ctx.Fset.Position(next.Pos()).Offset
+	if start < 0 || end < 0 || start >= end || end > len(ctx.Source) {
+		return false
+	}
+
+	gap := ctx.Source[start:end]
+
+	return bytes.Contains(gap, []byte("//")) ||
+		bytes.Contains(gap, []byte("/*"))
+}
+
+func clauseColonOffset(ctx *Context, stmt ast.Stmt) int {
+	var pos token.Pos
+	switch n := stmt.(type) {
+	case *ast.CaseClause:
+		if n == nil {
+			return -1
+		}
+		pos = n.Colon
+
+	case *ast.CommClause:
+		if n == nil {
+			return -1
+		}
+		pos = n.Colon
+
+	default:
+		return -1
+	}
+
+	return ctx.Fset.Position(pos).Offset
+}
+
 // InsertBlankAfterAction inserts a blank line after a node if not already
 // present.
 type InsertBlankAfterAction struct {

dsl/condition.go

@@ -213,8 +213,8 @@ func (c *ExprEditSafeCond) Eval(caps Captures, ctx *Context) bool {
 	if (&IsInCallArgsCond{Target: c.Target}).Eval(caps, ctx) {
 		switch c.CallArgsPolicy {
 		case CallArgsPolicyForce:
-
 			// OK.
+
 		case CallArgsPolicyAuto:
 			call := enclosingCallForArg(node, ctx)
 			if call == nil {
@@ -1103,8 +1103,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 +1116,9 @@ 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 +1134,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 +2116,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 +2171,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 ||
@@ -2175,7 +2194,6 @@ func onlyStmtInBlockWithoutLeadingComment(ctx *Context, node ast.Node) bool {
 	}
 	switch ctx.Parent(block).(type) {
 	case *ast.IfStmt, *ast.ForStmt, *ast.RangeStmt:
-
 	default:
 		return false
 	}

dsl/func_lit_expand_action.go

@@ -122,8 +122,8 @@ func isTrivialInlineFuncLit(fn *ast.FuncLit) bool {
 	for _, res := range ret.Results {
 		switch res.(type) {
 		case *ast.Ident, *ast.BasicLit:
-
 			// OK.
+
 		default:
 			return false
 		}

dsl/layout/render.go

@@ -68,8 +68,8 @@ 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-- {
@@ -192,8 +192,8 @@ 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",
 			},
 		},