diff --git a/formatter/body.go b/formatter/body.go index 2744a19..1698303 100644 --- a/formatter/body.go +++ b/formatter/body.go @@ -44,7 +44,7 @@ func (f *formatter) bracedBody(fields []*syntax.Field) doc.Doc { doc.IfBreak(doc.SoftLine, doc.Text(" ")), doc.Text("}"), } - if f.opts.BreakStructs { + if f.opts.BreakStructs || hasTrailingDelim(fields[len(fields)-1].Sep) { // BreakParent inside the group forces it to the broken layout. content = doc.Concat{doc.BreakParent, content} } @@ -65,7 +65,7 @@ func (f *formatter) bracedEnumBody(values []*syntax.EnumValue) doc.Doc { doc.IfBreak(doc.SoftLine, doc.Text(" ")), doc.Text("}"), } - if f.opts.BreakEnums { + if f.opts.BreakEnums || hasTrailingDelim(values[len(values)-1].Sep) { content = doc.Concat{doc.BreakParent, content} } return doc.GroupID(bodyID, content) @@ -80,7 +80,10 @@ func (f *formatter) closingTrivia(last syntax.Node) []doc.Doc { var parts []doc.Doc if len(close.Leading) > 0 { parts = append(parts, doc.HardLine) + prevBlank := 0 for i, c := range close.Leading { + parts = append(parts, f.blankLineDocs(c.BlankLinesBefore-prevBlank, doc.HardLine)...) + prevBlank = c.BlankLinesBefore parts = append(parts, doc.Text(c.Text)) if i < len(close.Leading)-1 { parts = append(parts, doc.HardLine) @@ -136,14 +139,12 @@ func (f *formatter) service(v *syntax.Service) doc.Doc { return doc.Concat(out) } -// function formats a service method with the signature escalation: -// -// 1. the whole signature on one line, when it fits; -// 2. arguments on one line, the throws clause broken; -// 3. arguments and throws clause both broken. -// -// Each state is tried in order by the conditional group, so the escalation -// is decided by the remaining width at the function's position. +// function formats a service method. The signature escalates via nested +// groups: the whole signature folds when it fits, otherwise the throws +// clause unfolds while the arguments stay flat, and the arguments unfold +// last. Each clause is forced independently — comments, blank lines, or a +// trailing delimiter in throws never break the arguments, because the +// throws clause is a sibling group, not an ancestor. func (f *formatter) function(v *syntax.Function) doc.Doc { parts := append(f.leadingComments(v), f.functionBody(v)) parts = append(parts, f.trailingComments(v)...) @@ -151,23 +152,62 @@ func (f *formatter) function(v *syntax.Function) doc.Doc { } func (f *formatter) functionBody(v *syntax.Function) doc.Doc { - // Comments or blank lines inside the argument or throws lists force the - // multiline layout: the flat states would drop them. - if f.fieldsForcedBroken(v.Args) || v.Throws != nil && f.fieldsForcedBroken(v.Throws.Fields) { + // Comments or blank lines in the arguments force the multiline layout: + // the flat argument group would drop them. + if f.fieldsForcedBroken(v.Args) || hasTrailingDelim(lastSep(v.Args)) { return f.functionBrokenArgs(v) } + // The argument group folds to "(a, b)" when it fits and unfolds to one + // field per line otherwise. + args := doc.Group(doc.IfBreak( + f.brokenParens("(", ")", v.Args), + doc.Concat{doc.Text("("), f.flatFieldsJoin(v.Args), doc.Text(")")}, + )) + if v.Throws == nil { - return doc.ConditionalGroup(0, - f.functionFlat(v, false), - f.functionBrokenArgs(v), - ) + return doc.Group(doc.Concat{ + doc.Text(f.functionHeader(v)), + args, + f.annotationsDoc(v.Annotations), + }) + } + + // The throws clause unfolds when the outer group breaks. A trailing + // delimiter, comment, or blank line inside it forces the unfold with a + // break parent; being a sibling of the args group, it cannot break the + // arguments themselves. + throws := doc.Concat{doc.Text(" throws "), f.brokenParens("(", ")", v.Throws.Fields)} + if f.fieldsForcedBroken(v.Throws.Fields) || hasTrailingDelim(lastSep(v.Throws.Fields)) { + throws = doc.Concat{throws, doc.BreakParent} } - return doc.ConditionalGroup(0, - f.functionFlat(v, false), - f.functionFlat(v, true), - f.functionBrokenArgs(v), - ) + + return doc.Group(doc.Concat{ + doc.Text(f.functionHeader(v)), + args, + doc.IfBreak( + throws, + doc.Concat{doc.Text(" throws ("), f.flatFieldsJoin(v.Throws.Fields), doc.Text(")")}, + ), + f.annotationsDoc(v.Annotations), + }) +} + +// lastSep returns the separator of the last list item, or 0 for an empty +// list. +func lastSep(fields []*syntax.Field) syntax.TokenKind { + if len(fields) == 0 { + return 0 + } + return fields[len(fields)-1].Sep +} + +// hasTrailingDelim reports whether the separator of the last list item is +// present, i.e. the source carries a trailing delimiter. Like zig fmt, a +// trailing delimiter forces the list to unfold when formatting; without +// one, the list may fold when it fits. +func hasTrailingDelim(sep syntax.TokenKind) bool { + return sep != 0 } // functionHeader renders "[oneway] ". @@ -184,32 +224,6 @@ func (f *formatter) functionHeader(v *syntax.Function) string { return out + v.Name.Text } -// functionFlat renders the signature with args flat and throws either flat -// or broken. The states are printed flat (or measured flat), so line docs -// render as spaces; throwsBroken inserts hard lines to break the clause. -func (f *formatter) functionFlat(v *syntax.Function, throwsBroken bool) doc.Doc { - args := f.flatFieldsJoin(v.Args) - parts := []doc.Doc{ - doc.Text(f.functionHeader(v)), - doc.Text("("), - args, - doc.Text(")"), - } - if v.Throws != nil { - if throwsBroken { - parts = append(parts, doc.Text(" throws "), f.brokenParens("(", ")", v.Throws.Fields)) - } else { - parts = append(parts, - doc.Text(" throws ("), - f.flatFieldsJoin(v.Throws.Fields), - doc.Text(")"), - ) - } - } - parts = append(parts, f.annotationsDoc(v.Annotations)) - return doc.Concat(parts) -} - // flatFieldsJoin joins fields with their separators on one line: each // field's own separator when preserving, or a single forced separator per // the FunctionSeparator mode. @@ -301,8 +315,13 @@ func (f *formatter) fieldsForcedBroken(fields []*syntax.Field) bool { } // blankLines returns count hard-line docs for the blank lines before a -// node's first token. +// node's first token. When the node carries leading comments the blank +// lines belong to that run and leadingComments emits them; returning nil +// here keeps them from being printed twice. func (f *formatter) blankLines(n syntax.Node, line doc.Doc) []doc.Doc { + if len(f.token(n.TokStart()).Leading) > 0 { + return nil + } return f.blankLineDocs(f.blankBefore(n), line) } diff --git a/formatter/field.go b/formatter/field.go index 163739f..622b003 100644 --- a/formatter/field.go +++ b/formatter/field.go @@ -67,33 +67,42 @@ func (f *formatter) enumValueList(values []*syntax.EnumValue, bodyID int) doc.Do return doc.Concat(parts) } +// groupedWith reports whether the node joins the alignment group of the +// item before it: no blank line and no comment sits between them. A comment +// in the gap is a visual break, like whitespace. +func (f *formatter) groupedWith(n syntax.Node) bool { + return f.blankBefore(n) < 1 && len(f.token(n.TokStart()).Leading) == 0 +} + // alignmentFor returns the column alignment for field i, or nil when -// alignment is disabled. Alignment is scoped to the blank-line group the -// field belongs to. +// alignment is disabled. Alignment is scoped to the blank-line and comment +// group the field belongs to. func (f *formatter) alignmentFor(fields []*syntax.Field, i int) *columnAlign { if f.opts.Align == AlignDisable { return nil } start := i - for start > 0 && f.blankBefore(fields[start]) < 1 { + for start > 0 && f.groupedWith(fields[start]) { start-- } end := i - for end+1 < len(fields) && f.blankBefore(fields[end+1]) < 1 { + for end+1 < len(fields) && f.groupedWith(fields[end+1]) { end++ } group := fields[start : end+1] a := computeFieldAlign(group) - if !f.alignmentFits(group, a) { + if !f.alignmentFits(group, a) && !f.sourceAligned(fields, start, end) { return nil } return a } -// alignmentFits reports whether column alignment keeps the group within -// printWidth: the padded columns plus the longest field name must fit one -// indent level under the limit. Default values and annotations are ignored; -// a line that long overflows regardless of alignment. +// alignmentFits reports whether column alignment keeps the structural +// content of every line within printWidth: the padded columns plus, per +// field, its name and trailing separator. Trailing comments are excluded — +// a comment may overflow its line without affecting alignment. Default +// values and annotations are ignored; a line that long overflows regardless +// of alignment. func (f *formatter) alignmentFits(fields []*syntax.Field, a *columnAlign) bool { limit := f.opts.PrintWidth - f.opts.TabWidth columns := a.idWidth + 1 @@ -103,11 +112,31 @@ func (f *formatter) alignmentFits(fields []*syntax.Field, a *columnAlign) bool { if f.opts.Align == AlignField { columns += a.typeWidth + 1 } - longestName := 0 + longest := 0 for _, field := range fields { - longestName = maxInt(longestName, len(field.Name.Text)) + w := len(field.Name.Text) + if field.Sep != 0 { + w++ // trailing separator + } + longest = maxInt(longest, w) + } + return columns+longest <= limit +} + +// sourceAligned reports whether the group's field names start at the same +// column in the source. Such a layout is deliberate — preserving it matters +// more than printWidth — so alignment survives the width gate for it. +func (f *formatter) sourceAligned(fields []*syntax.Field, start, end int) bool { + col := 0 + for i := start; i <= end; i++ { + c := f.token(fields[i].Name.TokStart()).Col + if col == 0 { + col = c + } else if c != col { + return false + } } - return columns+longestName <= limit + return true } func (f *formatter) alignmentForEnum(values []*syntax.EnumValue, i int) *columnAlign { @@ -115,11 +144,11 @@ func (f *formatter) alignmentForEnum(values []*syntax.EnumValue, i int) *columnA return nil } start := i - for start > 0 && f.blankBefore(values[start]) < 1 { + for start > 0 && f.groupedWith(values[start]) { start-- } end := i - for end+1 < len(values) && f.blankBefore(values[end+1]) < 1 { + for end+1 < len(values) && f.groupedWith(values[end+1]) { end++ } return computeEnumAlign(values[start : end+1]) diff --git a/formatter/format.go b/formatter/format.go index 5d1a47a..ddfdb93 100644 --- a/formatter/format.go +++ b/formatter/format.go @@ -169,12 +169,23 @@ func (f *formatter) blankBefore(n syntax.Node) int { } // leadingComments returns the comments attached before the node's first -// token, as docs that each end with a hard line. +// token, each ending with a hard line, with the blank lines from the source +// gap distributed exactly as written: before the run, between comments, and +// between the last comment and the node itself. blankLines deliberately +// emits nothing when leading comments exist. func (f *formatter) leadingComments(n syntax.Node) []doc.Doc { + tok := f.token(n.TokStart()) + if len(tok.Leading) == 0 { + return nil + } var parts []doc.Doc - for _, c := range f.token(n.TokStart()).Leading { + prevBlank := 0 + for _, c := range tok.Leading { + parts = append(parts, f.blankLineDocs(c.BlankLinesBefore-prevBlank, doc.HardLine)...) + prevBlank = c.BlankLinesBefore parts = append(parts, doc.Text(c.Text), doc.HardLine) } + parts = append(parts, f.blankLineDocs(tok.BlankLinesBefore-prevBlank, doc.HardLine)...) return parts } @@ -238,9 +249,11 @@ func (f *formatter) document() doc.Doc { if len(eof.Leading) > 0 { if len(f.doc.Nodes) > 0 { parts = append(parts, doc.HardLine) - parts = append(parts, f.blankLineDocs(eof.BlankLinesBefore, doc.HardLine)...) } + prevBlank := 0 for i, c := range eof.Leading { + parts = append(parts, f.blankLineDocs(c.BlankLinesBefore-prevBlank, doc.HardLine)...) + prevBlank = c.BlankLinesBefore parts = append(parts, doc.Text(c.Text)) if i < len(eof.Leading)-1 { parts = append(parts, doc.HardLine) diff --git a/formatter/format_test.go b/formatter/format_test.go index 849dd83..6638c8b 100644 --- a/formatter/format_test.go +++ b/formatter/format_test.go @@ -108,7 +108,7 @@ func TestFormatAnnotations(t *testing.T) { { name: "empty annotation before an enum", src: "@deprecation.Deprecated{}\nenum Status {\n A,\n B,\n}\n", - want: "@deprecation.Deprecated{}\nenum Status { A, B }\n", + want: "@deprecation.Deprecated{}\nenum Status {\n A,\n B,\n}\n", }, { name: "multiple annotations keep order", @@ -315,7 +315,7 @@ func TestFormatStructs(t *testing.T) { { name: "semicolon separators preserved", src: "struct S {\n 1: i32 a;\n 2: string b;\n}", - want: "struct S { 1: i32 a; 2: string b }\n", + want: "struct S {\n 1: i32 a;\n 2: string b;\n}\n", }, { name: "mixed separators preserved per field", @@ -827,6 +827,82 @@ func TestFormatBlankLines(t *testing.T) { } } +func TestFormatCommentBlankLines(t *testing.T) { + tests := []struct { + name string + src string + want string + }{ + { + name: "blank line below leading comment stays below", + src: "struct A {\n 1: i32 a\n}\n\n// above\n\nstruct B {\n 1: i32 b\n}", + want: "struct A { 1: i32 a }\n\n// above\n\nstruct B { 1: i32 b }\n", + }, + { + name: "blank line between field comment and field stays", + src: "struct S {\n 1: i32 a\n // note\n\n 2: i32 b\n}", + want: "struct S {\n 1: i32 a\n // note\n\n 2: i32 b\n}\n", + }, + { + name: "blank lines between comment run members", + src: "struct A {\n 1: i32 a\n}\n\n// one\n\n// two\n\nstruct B {\n 1: i32 b\n}", + want: "struct A { 1: i32 a }\n\n// one\n\n// two\n\nstruct B { 1: i32 b }\n", + }, + { + name: "blank line before closing comment", + src: "struct S {\n 1: i32 a\n\n // close\n}", + want: "struct S {\n 1: i32 a\n\n // close\n}\n", + }, + { + name: "blank line between closing comments", + src: "struct S {\n 1: i32 a\n // one\n\n // two\n}", + want: "struct S {\n 1: i32 a\n // one\n\n // two\n}\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runCase(t, tt.src, testOpts(80), tt.want) + }) + } +} + +func TestFormatAlignmentCommentBreak(t *testing.T) { + tests := []struct { + name string + src string + want string + }{ + { + name: "comment between fields breaks alignment", + src: "struct MobileSuit {\n" + + " 1: required zeon.PropulsionSystemType propulsion_system\n" + + " // the beam cannon\n" + + " 2: required string registry_code\n" + + "}", + want: "struct MobileSuit {\n" + + " 1: required zeon.PropulsionSystemType propulsion_system\n" + + " // the beam cannon\n" + + " 2: required string registry_code\n" + + "}\n", + }, + { + name: "comment between enum values breaks alignment", + src: "enum E {\n AValue = 1\n // separated\n B = 2\n}", + want: "enum E {\n AValue = 1\n // separated\n B = 2\n}\n", + }, + { + name: "trailing line comment does not break alignment", + src: "struct S {\n 1: i32 a // one\n 2: string longer\n}", + want: "struct S {\n 1: i32 a // one\n 2: string longer\n}\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runCase(t, tt.src, testOpts(80), tt.want) + }) + } +} + func TestFormatAlignmentWidth(t *testing.T) { tests := []struct { name string @@ -837,8 +913,8 @@ func TestFormatAlignmentWidth(t *testing.T) { { name: "alignment dropped when the group exceeds printWidth", width: 40, - src: "struct S {\n 1: i32 a\n 2: some_very_long_namespace.SomeVeryLongTypeName b\n}", - want: "struct S {\n 1: i32 a\n 2: some_very_long_namespace.SomeVeryLongTypeName b\n}\n", + src: "struct S {\n 1: i32 a\n 2: federation_special_weapons.MegaParticleCannonType b\n}", + want: "struct S {\n 1: i32 a\n 2: federation_special_weapons.MegaParticleCannonType b\n}\n", }, { name: "alignment kept when the group fits", @@ -846,6 +922,42 @@ func TestFormatAlignmentWidth(t *testing.T) { src: "struct S {\n 1: i32 a\n 2: string longer_name\n}", want: "struct S {\n 1: i32 a\n 2: string longer_name\n}\n", }, + { + name: "source-aligned group keeps alignment over printWidth", + width: 40, + src: "struct S {\n" + + " 1: federation.MobileSuitFrameType frame_type;\n" + + " 2: federation.PropulsionType propulsion_type;\n" + + "}", + want: "struct S {\n" + + " 1: federation.MobileSuitFrameType frame_type;\n" + + " 2: federation.PropulsionType propulsion_type;\n" + + "}\n", + }, + { + name: "trailing comment overflow does not drop alignment", + width: 80, + src: "struct S {\n" + + " 1: federation.SensorType sensor_type; //deprecated, use the mobile suit registry instead\n" + + " 2: federation.MobileSuitFrameType frame_type;\n" + + "}", + want: "struct S {\n" + + " 1: federation.SensorType sensor_type; //deprecated, use the mobile suit registry instead\n" + + " 2: federation.MobileSuitFrameType frame_type;\n" + + "}\n", + }, + { + name: "source-aligned group re-pads to widest type over printWidth", + width: 40, + src: "struct S {\n" + + " 1: federation.MobileSuitFrameType frame_type;\n" + + " 2: federation.PropulsionType propulsion_type;\n" + + "}", + want: "struct S {\n" + + " 1: federation.MobileSuitFrameType frame_type;\n" + + " 2: federation.PropulsionType propulsion_type;\n" + + "}\n", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -853,3 +965,135 @@ func TestFormatAlignmentWidth(t *testing.T) { }) } } + +func TestFormatFullFile(t *testing.T) { + src := `include "gundam/types.thrift" + +namespace * mobile_suit.zeon + +enum Status { + Active, + Decommissioned, +} + +// A mobile suit's combat record. +struct CombatRecord { + 1: required string model_number + 2: optional i64 sorties + 3: optional i64 kills +} + +union MobileSuit { + 1: CombatRecord combat_record + 2: string notes +} + +service Hangar { + void dock(1: string bay) throws ( + 1: exceptions.BayFull bay_full, // bay is occupied + 2: exceptions.SuitMismatch suit_mismatch + ) + + // Deploy resets the telemetry counters. + CombatRecord deploy( + 1: MobileSuit suit, + 2: string pilot, + ) + + oneway void status(1: string bay) +} +` + want := `include "gundam/types.thrift" + +namespace * mobile_suit.zeon + +enum Status { + Active, + Decommissioned, +} + +// A mobile suit's combat record. +struct CombatRecord { + 1: required string model_number + 2: optional i64 sorties + 3: optional i64 kills +} + +union MobileSuit { 1: CombatRecord combat_record 2: string notes } + +service Hangar { + void dock(1: string bay) throws ( + 1: exceptions.BayFull bay_full, // bay is occupied + 2: exceptions.SuitMismatch suit_mismatch + ) + + // Deploy resets the telemetry counters. + CombatRecord deploy( + 1: MobileSuit suit, + 2: string pilot, + ) + + oneway void status(1: string bay) +} +` + runCase(t, src, testOpts(80), want) +} + +func TestFormatTrailingDelim(t *testing.T) { + tests := []struct { + name string + src string + want string + }{ + { + name: "trailing delimiter forces struct body broken", + src: "struct S {\n 1: i32 a;\n}", + want: "struct S {\n 1: i32 a;\n}\n", + }, + { + name: "no trailing delimiter folds struct body", + src: "struct S {\n 1: i32 a\n}", + want: "struct S { 1: i32 a }\n", + }, + { + name: "trailing delimiter forces enum body broken", + src: "enum E {\n A,\n B,\n}", + want: "enum E {\n A,\n B,\n}\n", + }, + { + name: "no trailing delimiter folds enum body", + src: "enum E {\n A,\n B\n}", + want: "enum E { A, B }\n", + }, + { + name: "trailing delimiter forces args broken", + src: "service F {\n void go(\n 1: i32 a,\n )\n}", + want: "service F {\n void go(\n 1: i32 a,\n )\n}\n", + }, + { + name: "no trailing delimiter folds args", + src: "service F {\n void go(\n 1: i32 a\n )\n}", + want: "service F {\n void go(1: i32 a)\n}\n", + }, + { + name: "throws comment does not break args", + src: "service Hangar {\n" + + " void dock(1: i32 a) throws (\n" + + " 1: exceptions.BayFull bay_full, // bay is occupied\n" + + " 2: exceptions.SuitMismatch suit_mismatch\n" + + " )\n" + + "}", + want: "service Hangar {\n" + + " void dock(1: i32 a) throws (\n" + + " 1: exceptions.BayFull bay_full, // bay is occupied\n" + + " 2: exceptions.SuitMismatch suit_mismatch\n" + + " )\n" + + "}\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runCase(t, tt.src, testOpts(80), tt.want) + }) + } +} diff --git a/syntax/lexer.go b/syntax/lexer.go index f8c853f..11eecfc 100644 --- a/syntax/lexer.go +++ b/syntax/lexer.go @@ -184,6 +184,10 @@ type Trivia struct { Offset int // byte offset of the first character Line int // 1-based line of the first character Col int // 1-based rune column of the first character + + // BlankLinesBefore is the number of empty lines between the previous + // token (or trivia) and this one, within the enclosing gap. + BlankLinesBefore int } // Token is a single lexical token with its attached comment trivia. @@ -283,16 +287,16 @@ func (l *lexer) scanTrivia(prevLine int) (leading, trailing []Trivia, blankLines case isWhitespace(c): blankLines += l.scanWhitespace() case c == '/' && l.peekByte(1) == '/': - leading, trailing = l.appendComment(leading, trailing, prevLine, l.scanLineComment()) + leading, trailing = l.appendComment(leading, trailing, prevLine, blankLines, l.scanLineComment()) case c == '/' && l.peekByte(1) == '*': - leading, trailing = l.appendComment(leading, trailing, prevLine, l.scanBlockComment()) + leading, trailing = l.appendComment(leading, trailing, prevLine, blankLines, l.scanBlockComment()) case c == '#': - leading, trailing = l.appendComment(leading, trailing, prevLine, l.scanLineComment()) + leading, trailing = l.appendComment(leading, trailing, prevLine, blankLines, l.scanLineComment()) case c == '@': // Java-style annotations (@name{...}) are preserved as trivia, // like comments, so they round-trip without being part of the // grammar. - leading, trailing = l.appendComment(leading, trailing, prevLine, l.scanLineAnnotation()) + leading, trailing = l.appendComment(leading, trailing, prevLine, blankLines, l.scanLineAnnotation()) default: return leading, trailing, blankLines } @@ -300,7 +304,8 @@ func (l *lexer) scanTrivia(prevLine int) (leading, trailing []Trivia, blankLines return leading, trailing, blankLines } -func (l *lexer) appendComment(leading, trailing []Trivia, prevLine int, t Trivia) ([]Trivia, []Trivia) { +func (l *lexer) appendComment(leading, trailing []Trivia, prevLine, blankLines int, t Trivia) ([]Trivia, []Trivia) { + t.BlankLinesBefore = blankLines if t.Line == prevLine { return leading, append(trailing, t) }