Skip to content

Commit 1b7f7ec

Browse files
authored
Fix legacy formatter behaviour for bodies with no comments or decls (#741)
There was a bug with the legacy formatting options, where empty bodies, without comments, etc., were still being broken. This fixes that and adds a test case.
1 parent 03eb650 commit 1b7f7ec

5 files changed

Lines changed: 25 additions & 11 deletions

File tree

experimental/ast/printer/decl.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -338,16 +338,20 @@ func (p *printer) printBody(body ast.DeclBody) {
338338
forceBroken := triviaHasComments(trivia) ||
339339
len(closeComments) > 0 ||
340340
p.scopeHasAttachedComments(body.Braces())
341-
if !forceBroken && !p.bodyShouldBreak(openTok, closeTok) {
341+
if !forceBroken && !p.bodyShouldBreak(openTok, closeTok, body.Decls().Len()) {
342342
decls := body.Decls()
343-
p.withGroup(func(p *printer) {
344-
p.withIndent(func(indented *printer) {
345-
for i := range decls.Len() {
346-
indented.printDecl(decls.At(i), gapSoftline)
347-
}
343+
// Only create the indented [dom.Group] in the case where there are decls
344+
// to avoid an indent in the flat case with no decls.
345+
if decls.Len() > 0 {
346+
p.withGroup(func(p *printer) {
347+
p.withIndent(func(indented *printer) {
348+
for i := range decls.Len() {
349+
indented.printDecl(decls.At(i), gapSoftline)
350+
}
351+
})
352+
p.push(tagSoftlineFlat, tagSoftbreak)
348353
})
349-
p.push(tagSoftlineFlat, tagSoftbreak)
350-
})
354+
}
351355
p.printToken(closeTok, gapNone)
352356
return
353357
}

experimental/ast/printer/format.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,20 +125,20 @@ func (p *printer) literalShouldBreak(openTok, closeTok token.Token, count int) b
125125
// decl-bearing body scope (`{ ... }` on message, enum, service,
126126
// oneof, extend, or RPC method):
127127
//
128-
// - [LayoutStrict]: always broken. Callers handle the empty-body
128+
// - [LayoutStrict]: always broken for >0 decls. Callers handle the empty-body
129129
// case (rendered as `{}`) before consulting this helper.
130130
//
131131
// - [LayoutDynamic]: broken if and only if source had a newline between open
132132
// and close brace.
133133
//
134134
// Callers should OR the result with their own forceBroken signal
135135
// (e.g. for scope-attached comments that require expansion).
136-
func (p *printer) bodyShouldBreak(openTok, closeTok token.Token) bool {
136+
func (p *printer) bodyShouldBreak(openTok, closeTok token.Token, declsLen int) bool {
137137
switch p.options.Formatting.BodyLayout {
138138
case LayoutDynamic:
139139
return !sourceWasFlat(openTok, closeTok)
140140
default: // LayoutStrict
141-
return true
141+
return declsLen > 0
142142
}
143143
}
144144

experimental/ast/printer/testdata/format/basic.proto

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,8 @@ import "acme/payment/v1/payment.proto"; /* trailing */
2222
message Foo { /* Foo */
2323
string name = 1;
2424
}
25+
26+
message Bar {
27+
28+
29+
}

experimental/ast/printer/testdata/format/basic.proto.default.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,6 @@ option optimize_for = SPEED;
1818
message Foo { /* Foo */
1919
string name = 1;
2020
}
21+
22+
message Bar {
23+
}

experimental/ast/printer/testdata/format/basic.proto.legacy.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,5 @@ option optimize_for = SPEED;
1818
message Foo { /* Foo */
1919
string name = 1;
2020
}
21+
22+
message Bar {}

0 commit comments

Comments
 (0)