Skip to content

Commit de866de

Browse files
committed
formatter: preserve readable call args
Parse greedy call arguments with brace awareness so composite literal arguments keep their block shape. This avoids treating map entries as separate call arguments and keeps the closing brace on its own line. Keep the argument following a multiline expression on a fresh line. Also preserve explicit string concatenation when a line-oriented or deeply indented string would otherwise fragment into tiny pieces. Allow the CLI to format multiple files in one -w invocation. Adoption Makefiles can now pass batched xargs input without spawning one process per file.
1 parent ac75428 commit de866de

9 files changed

Lines changed: 352 additions & 36 deletions

cmd/llformat/main.go

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -127,27 +127,47 @@ func run(args []string, stdout, stderr io.Writer) int {
127127
return runPrintLogCallsPatterns(stdout, cfg)
128128
}
129129

130-
if fs.NArg() != 1 {
130+
if fs.NArg() == 0 {
131131
fs.Usage()
132132

133133
return 2
134134
}
135135

136-
path := fs.Arg(0)
137-
data, err := os.ReadFile(path)
138-
if err != nil {
139-
fmt.Fprintf(stderr, "read %s: %v\n", path, err)
136+
paths := fs.Args()
137+
if !f.write && len(paths) != 1 {
138+
fmt.Fprintln(
139+
stderr, "llformat: multiple paths require -w/--write",
140+
)
141+
fs.Usage()
140142

141-
return 1
143+
return 2
142144
}
143145

144-
out := formatter.NewPipeline(cfg).Format(data)
146+
p := formatter.NewPipeline(cfg)
147+
for _, path := range paths {
148+
data, err := os.ReadFile(path)
149+
if err != nil {
150+
fmt.Fprintf(stderr, "read %s: %v\n", path, err)
145151

146-
if f.write {
147-
return writeOutputToFile(path, out, stderr)
152+
return 1
153+
}
154+
155+
out := p.Format(data)
156+
157+
if f.write {
158+
if code := writeOutputToFile(
159+
path, out, stderr,
160+
); code != 0 {
161+
return code
162+
}
163+
164+
continue
165+
}
166+
167+
return writeOutputToWriter(out, stdout)
148168
}
149169

150-
return writeOutputToWriter(out, stdout)
170+
return 0
151171
}
152172

153173
func printUsage(w io.Writer) {
@@ -160,7 +180,7 @@ func printUsage(w io.Writer) {
160180
"FUNCS] [--logcalls-min-tail-len N] "+
161181
"[--logcalls-selector-names NAMES] "+
162182
"[--logcalls-selector-prefixes PREFIXES] "+
163-
"[--fixpoint-iters N] <path>",
183+
"[--fixpoint-iters N] <path> [path ...]",
164184
)
165185
fmt.Fprintln(w, " llformat --print-plan")
166186
fmt.Fprintln(w, " llformat --print-logcalls-patterns")

cmd/llformat/main_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package main
2+
3+
import (
4+
"bytes"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestRunWriteMultiplePaths(t *testing.T) {
13+
dir := t.TempDir()
14+
paths := []string{
15+
filepath.Join(dir, "one.go"),
16+
filepath.Join(dir, "two.go"),
17+
}
18+
src := []byte(`package p
19+
20+
import "fmt"
21+
22+
func f() error {
23+
return fmt.Errorf("this is a deliberately long error message that should split")
24+
}
25+
`)
26+
27+
for _, path := range paths {
28+
require.NoError(t, os.WriteFile(path, src, 0o600))
29+
}
30+
31+
var stdout, stderr bytes.Buffer
32+
code := run(
33+
[]string{"-w", "--col", "50", paths[0], paths[1]}, &stdout,
34+
&stderr,
35+
)
36+
37+
require.Equal(t, 0, code, stderr.String())
38+
require.Empty(t, stdout.String())
39+
for _, path := range paths {
40+
got, err := os.ReadFile(path)
41+
require.NoError(t, err)
42+
require.Contains(t, string(got), "\"this is a \" +")
43+
require.Contains(
44+
t, string(got),
45+
"\"deliberately long error \" +",
46+
)
47+
}
48+
}
49+
50+
func TestRunMultiplePathsRequireWrite(t *testing.T) {
51+
dir := t.TempDir()
52+
one := filepath.Join(dir, "one.go")
53+
two := filepath.Join(dir, "two.go")
54+
require.NoError(t, os.WriteFile(one, []byte("package p\n"), 0o600))
55+
require.NoError(t, os.WriteFile(two, []byte("package p\n"), 0o600))
56+
57+
var stdout, stderr bytes.Buffer
58+
code := run([]string{one, two}, &stdout, &stderr)
59+
60+
require.Equal(t, 2, code)
61+
require.Empty(t, stdout.String())
62+
require.Contains(
63+
t, stderr.String(),
64+
"multiple paths require -w/--write",
65+
)
66+
}

dsl/expr_doc.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,8 @@ func methodChainSegmentDoc(seg segment, ctx *Context,
558558
layout.L(), layout.C(brokenArgs...),
559559
layout.T(","),
560560
),
561-
), layout.L(),
561+
),
562+
layout.L(),
562563
)
563564

564565
// In flat mode, keep args inline. In broken mode, put each arg on its
@@ -690,7 +691,8 @@ func parenExprDoc(p *ast.ParenExpr, ctx *Context) (layout.Doc, bool) {
690691
layout.SL(), inner,
691692
),
692693
),
693-
), layout.T(")"),
694+
),
695+
layout.T(")"),
694696
),
695697
), true
696698
}
@@ -889,7 +891,8 @@ func logicalBinaryExprDoc(bin *ast.BinaryExpr, ctx *Context,
889891
left, layout.T(" "),
890892
layout.T(
891893
bin.Op.String(),
892-
), layout.L(),
894+
),
895+
layout.L(),
893896
right,
894897
),
895898
), true
@@ -941,7 +944,8 @@ func comparisonBinaryExprDoc(bin *ast.BinaryExpr, ctx *Context,
941944
leftDoc, layout.T(" "),
942945
layout.T(
943946
bin.Op.String(),
944-
), layout.SL(),
947+
),
948+
layout.SL(),
945949
rightDoc,
946950
),
947951
), true
@@ -1034,7 +1038,8 @@ func indexExprDoc(idx *ast.IndexExpr, ctx *Context,
10341038
layout.SL(), indexDoc,
10351039
),
10361040
),
1037-
), layout.T("]"),
1041+
),
1042+
layout.T("]"),
10381043
),
10391044
), true
10401045
}
@@ -1189,7 +1194,8 @@ func typeAssertExprDoc(t *ast.TypeAssertExpr, ctx *Context,
11891194
layout.SL(), typeDoc,
11901195
),
11911196
),
1192-
), layout.T(")"),
1197+
),
1198+
layout.T(")"),
11931199
),
11941200
), true
11951201
}
@@ -1271,7 +1277,8 @@ func sliceExprDoc(s *ast.SliceExpr, ctx *Context,
12711277
layout.SL(), inner,
12721278
),
12731279
),
1274-
), layout.T("]"),
1280+
),
1281+
layout.T("]"),
12751282
),
12761283
), true
12771284
}

dsl/layout/render_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ func TestRenderAlignIndentsToCurrentColumn(t *testing.T) {
4949
C(
5050
T("a,"), L(), T("b,"), L(), T("c"),
5151
),
52-
), T(")"),
52+
),
53+
T(")"),
5354
),
5455
)
5556
out := Render(doc, 7, 8, "")

formatter/compact_call_formatter.go

Lines changed: 76 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -971,7 +971,7 @@ func formatCallPackedMultiLine(call []byte, wsIndent, fullPrefix string,
971971
// []string{" in Configure)
972972
shouldBreakAfterCall := seenMultilineCall
973973
shouldBreakAfterComposite := seenMultilineComposite &&
974-
!seenMultilineCall && !argIsMultiline
974+
!seenMultilineCall
975975
shouldBreak := curLen+need > lineWidth || forcedBreak ||
976976
shouldBreakAfterCall || shouldBreakAfterComposite
977977
if shouldBreak {
@@ -1531,13 +1531,14 @@ func formatCallPackedMultiLineNext(call []byte, wsIndent, fullPrefix string,
15311531

15321532
// Handle call expression arguments with potential nesting.
15331533
callState := &callExprArgNextState{
1534-
b: &b,
1535-
contIndent: contIndent,
1536-
contIndentLen: contIndentLen,
1537-
lineWidth: lineWidth,
1538-
curLen: curLen,
1539-
first: first,
1540-
forcedBreak: forcedBreak,
1534+
b: &b,
1535+
contIndent: contIndent,
1536+
contIndentLen: contIndentLen,
1537+
lineWidth: lineWidth,
1538+
curLen: curLen,
1539+
first: first,
1540+
forcedBreak: forcedBreak || (!first &&
1541+
(seenMultilineCall || seenMultilineComposite)),
15411542
seenMultilineCall: seenMultilineCall,
15421543
seenMultilineComposite: seenMultilineComposite,
15431544
}
@@ -1955,6 +1956,10 @@ func (s *callExprArgNextState) writeArgSimple(a string) {
19551956
s.b.WriteString(s.contIndent)
19561957
s.b.WriteString(a)
19571958
s.curLen = s.contIndentLen + firstLineLen(a)
1959+
if strings.Contains(a, "\n") {
1960+
s.curLen = lastLineLen(a)
1961+
s.seenMultilineCall = true
1962+
}
19581963
s.first = false
19591964

19601965
return
@@ -1970,6 +1975,10 @@ func (s *callExprArgNextState) writeArgSimple(a string) {
19701975
}
19711976
s.b.WriteString(a)
19721977
s.curLen = advanceCols(s.curLen+2, a)
1978+
if strings.Contains(a, "\n") {
1979+
s.curLen = lastLineLen(a)
1980+
s.seenMultilineCall = true
1981+
}
19731982
}
19741983

19751984
func formatSelectorCallArgIfOverflowsNext(arg, contIndent string,
@@ -2421,7 +2430,7 @@ func formatCallGreedyWithOptions(call []byte, wsIndent string, baseLen int,
24212430

24222431
// No pre-scan; we will attach leading comments of the next arg (// or
24232432
// /* */) to the previous argument inline when emitting.
2424-
rawArgs := scanner.SplitTopLevel(argsBody)
2433+
rawArgs := scanner.SplitTopLevelAny(argsBody)
24252434
hasInlineComment := strings.Contains(argsBody, "/*") ||
24262435
strings.Contains(argsBody, "//")
24272436
normArgs := normalizeCallArgs(rawArgs, opts)
@@ -2432,6 +2441,15 @@ func formatCallGreedyWithOptions(call []byte, wsIndent string, baseLen int,
24322441
b.WriteByte('(')
24332442
curLen := baseLen + visualLen(head) + 1
24342443
contIndent := wsIndent + "\t"
2444+
prevArgMultiline := false
2445+
2446+
if len(normArgs) == 1 && normArgs[0].kind == argText {
2447+
q := quoteGoString(normArgs[0].text)
2448+
candidate := head + "(" + q + ")"
2449+
if advanceCols(baseLen, candidate) <= width {
2450+
return candidate
2451+
}
2452+
}
24352453

24362454
writeSplit := func(seg string, hasTrailingArgs bool) {
24372455
q := quoteGoString(seg)
@@ -2474,7 +2492,12 @@ func formatCallGreedyWithOptions(call []byte, wsIndent string, baseLen int,
24742492
a.expr,
24752493
)
24762494
}
2477-
if hasInlineComment {
2495+
if prevArgMultiline {
2496+
curLen = emitBreakWithComments(
2497+
&b, prefixes, contIndent,
2498+
)
2499+
justBroke = true
2500+
} else if hasInlineComment {
24782501
// Separator on same line; attach trailing line
24792502
// comment to previous arg, then place any block
24802503
// comment before next arg.
@@ -2541,10 +2564,19 @@ func formatCallGreedyWithOptions(call []byte, wsIndent string, baseLen int,
25412564
curLen = writeExprArg(
25422565
&b, a.expr, wsIndent, curLen, opts,
25432566
)
2567+
prevArgMultiline = strings.Contains(a.expr, "\n")
25442568
continue
25452569
}
25462570

25472571
// String arg: split greedily
2572+
if shouldPreserveExplicitStringConcat(a, contIndent, width) {
2573+
b.WriteString(a.raw)
2574+
curLen = advanceCols(curLen, a.raw)
2575+
prevArgMultiline = strings.Contains(a.raw, "\n")
2576+
2577+
continue
2578+
}
2579+
25482580
rest := a.text
25492581
hasTrailingArgs := i < len(normArgs)-1
25502582
avoidTinyVerbTail := opts.AvoidTinyFormatVerbTail &&
@@ -2746,6 +2778,7 @@ func formatCallGreedyWithOptions(call []byte, wsIndent string, baseLen int,
27462778
writeSplit(seg, hasTrailingArgs)
27472779
rest = rest[cut+1:]
27482780
}
2781+
prevArgMultiline = false
27492782
}
27502783
b.WriteByte(')')
27512784

@@ -2827,13 +2860,32 @@ func normalizeCallArg(trimmed string, argIndex int, rawCount int,
28272860
}
28282861

28292862
return arg{
2830-
kind: argText,
2831-
text: str,
2832-
raw: trimmed,
2833-
containsFormatVerb: containsFormatVerb(str),
2863+
kind: argText,
2864+
text: str,
2865+
raw: trimmed,
2866+
containsFormatVerb: containsFormatVerb(str),
2867+
explicitStringConcat: !isBasicStringLitExpr(e),
28342868
}
28352869
}
28362870

2871+
func shouldPreserveExplicitStringConcat(a arg, contIndent string,
2872+
width int) bool {
2873+
2874+
if !a.explicitStringConcat {
2875+
return false
2876+
}
2877+
if strings.Contains(a.text, "\n") {
2878+
return true
2879+
}
2880+
2881+
// Deeply indented calls can leave too little room for useful word
2882+
// splits. In that case, keep the author's concatenation boundaries
2883+
// instead of hard-cutting tiny string fragments.
2884+
const minUsefulTextCols = 24
2885+
2886+
return width-visualLen(contIndent)-4 < minUsefulTextCols
2887+
}
2888+
28372889
type argKind int
28382890

28392891
const (
@@ -2847,7 +2899,8 @@ type arg struct {
28472899
text string
28482900
raw string
28492901

2850-
containsFormatVerb bool
2902+
containsFormatVerb bool
2903+
explicitStringConcat bool
28512904
}
28522905

28532906
// commentPrefixes holds extracted comment prefixes from an argument.
@@ -3162,6 +3215,14 @@ func tryMoveStringToCont(b *strings.Builder, q, contIndent string, curLen *int,
31623215
func writeExprArg(b *strings.Builder, expr, wsIndent string, curLen int,
31633216
opts greedyCallOptions) int {
31643217

3218+
if formatted, ok := FormatCompositeLiteralArg(
3219+
expr, wsIndent+"\t",
3220+
); ok && strings.Contains(formatted, "\n") {
3221+
3222+
b.WriteString(formatted)
3223+
3224+
return lastLineLen(formatted)
3225+
}
31653226
if isTargetedCallStart(expr) {
31663227
formatted := formatCallGreedyWithOptions(
31673228
[]byte(expr), wsIndent, curLen, opts,

0 commit comments

Comments
 (0)