Skip to content

Commit 437e389

Browse files
committed
formatter: avoid hanging errorf calls
Remove the fmt.Errorf-only continuation-line shortcut. Let error formatting use the same greedy path as other printf calls. Update regressions to require packed heads without hanging parens.
1 parent db85867 commit 437e389

8 files changed

Lines changed: 75 additions & 86 deletions

File tree

cmd/llformat/main.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,8 @@ func buildPipelineConfig(f cliFlags) (formatter.PipelineConfig, error) {
254254
fixpointIters := f.fixpointIters
255255
if fixpointIters < 0 {
256256
return formatter.PipelineConfig{},
257-
fmt.Errorf(
258-
"invalid flags: --fixpoint-iters must be >= 0",
259-
)
257+
fmt.Errorf("invalid flags: --fixpoint-iters must be " +
258+
">= 0")
260259
}
261260
if fixpointIters == 0 {
262261
fixpointIters = 3

dsl/replace.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,8 @@ import "fmt"
88
func replaceSpan(src []byte, start, end int, replace []byte) ([]byte, error) {
99
if start < 0 || end < 0 || start > len(src) || end > len(src) ||
1010
start >= end {
11-
return nil, fmt.Errorf(
12-
"invalid span [%d:%d] (len=%d)", start, end, len(src),
13-
)
11+
return nil, fmt.Errorf("invalid span [%d:%d] (len=%d)", start,
12+
end, len(src))
1413
}
1514

1615
return ApplySingleEdit(src, start, end, replace)

formatter/compact_call_formatter.go

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -2498,15 +2498,6 @@ func formatCallGreedyWithOptions(call []byte, wsIndent string, baseLen int,
24982498
advanceCols(baseLen, candidate) <= width {
24992499
return candidate
25002500
}
2501-
if head == "fmt.Errorf" {
2502-
if formatted, ok := formatCallArgsOnContinuationLine(
2503-
head, normArgs, wsIndent, contIndent, width,
2504-
hasInlineComment,
2505-
); ok {
2506-
return formatted
2507-
}
2508-
}
2509-
25102501
writeSplit := func(seg string, hasTrailingArgs bool) {
25112502
q := quoteGoString(seg)
25122503
b.WriteString(q)
@@ -2881,39 +2872,6 @@ func formatCallSingleLineCandidate(head string, args []arg,
28812872
return head + "(" + strings.Join(parts, ", ") + ")", true
28822873
}
28832874

2884-
func formatCallArgsOnContinuationLine(head string, args []arg, wsIndent,
2885-
contIndent string, width int, hasInlineComment bool) (string, bool) {
2886-
2887-
if hasInlineComment || len(args) == 0 {
2888-
return "", false
2889-
}
2890-
2891-
parts := make([]string, 0, len(args))
2892-
for _, a := range args {
2893-
switch a.kind {
2894-
case argText:
2895-
parts = append(parts, quoteGoString(a.text))
2896-
2897-
case argExpr:
2898-
expr := strings.TrimSpace(a.expr)
2899-
if expr == "" || strings.Contains(expr, "\n") {
2900-
return "", false
2901-
}
2902-
parts = append(parts, expr)
2903-
2904-
default:
2905-
return "", false
2906-
}
2907-
}
2908-
2909-
argLine := strings.Join(parts, ", ") + ","
2910-
if advanceCols(visualLen(contIndent), argLine) > width {
2911-
return "", false
2912-
}
2913-
2914-
return head + "(\n" + contIndent + argLine + "\n" + wsIndent + ")", true
2915-
}
2916-
29172875
func normalizeCallArgs(rawArgs []string, opts greedyCallOptions) []arg {
29182876
normArgs := make([]arg, 0, len(rawArgs))
29192877
rawCount := len(rawArgs)

formatter/compact_call_formatter_test.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,13 +331,14 @@ func TestFormatCallPackedMultiLine_DoesNotReflowNestedSlogAttrWhenItFits(
331331
requireNoLineLongerThan(t, out, 80)
332332
}
333333

334-
func TestFormatCallGreedy_DoesNotSplitShortFormatStringToFitCommaSpace(
334+
func TestFormatCallGreedy_BreaksBeforeTrailingArgForShortFormatString(
335335
t *testing.T) {
336336

337337
// Regression test for "early break" in deeply indented return
338-
// statements: when the whole call cannot fit on the current line, but
339-
// the short argument list fits on a continuation line, prefer a
340-
// multiline call over splitting the string into `"..." +\n"...", err`.
338+
// statements: when the whole call cannot fit on the current line, keep
339+
// the call head packed and break before the trailing arg instead of
340+
// using a hanging-paren layout or splitting a short format string into
341+
// `"..." +\n"...", err`.
341342
call := []byte(`fmt.Errorf("error parsing psbt: %w", err)`)
342343

343344
// Choose a baseLen such that the quoted format string ends at column
@@ -350,9 +351,10 @@ func TestFormatCallGreedy_DoesNotSplitShortFormatStringToFitCommaSpace(
350351
// 24 == 79 => curLen == 55 => baseLen == 44.
351352
out := FormatCallGreedy(call, "\t\t\t\t\t", 44, 80, 8)
352353

353-
require.Contains(t, out, "fmt.Errorf(\n")
354+
require.Contains(t, out, "fmt.Errorf(\"error parsing psbt: %w\",\n")
354355
require.Contains(t, out, "\n "+
355-
" \"error parsing psbt: %w\", err,")
356+
" err)")
357+
require.NotContains(t, out, "fmt.Errorf(\n")
356358
require.NotContains(
357359
t, out, "\" +\n", "must not split a short format string "+
358360
"just to make room for a following space",

formatter/pipeline_logcalls_next_test.go

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ func f() error {
438438
require.NotContains(t, out, `"nil Message in " +`)
439439
}
440440

441-
func TestPipelineNext_LogCalls_ShortConcatUsesMultilineCallWhenIndented(
441+
func TestPipelineNext_LogCalls_ShortConcatSplitsStringWhenIndented(
442442
t *testing.T) {
443443

444444
const in = `package p
@@ -472,15 +472,16 @@ func f(msg any) error {
472472
out := string(p.Format([]byte(in)))
473473

474474
require.Contains(
475-
t, out, "return "+
476-
"fmt.Errorf(\n \"nil "+
477-
"Message in "+
478-
"SendServerEventRequest\",\n )",
475+
t, out, "return fmt.Errorf(\"nil Message in \" "+
476+
"+"+
477+
"\n"+
478+
" \"SendServerEventReq"+
479+
"uest\")",
479480
)
480-
require.NotContains(t, out, `"nil Message in " +`)
481+
require.NotContains(t, out, "return fmt.Errorf(\n")
481482
}
482483

483-
func TestPipelineNext_LogCalls_ShortErrorfArgsUseMultilineCallWhenIndented(
484+
func TestPipelineNext_LogCalls_ShortErrorfArgsBreakBeforeArgWhenIndented(
484485
t *testing.T) {
485486

486487
const in = `package p
@@ -512,11 +513,10 @@ func f(err error) (any, error) {
512513
out := string(p.Format([]byte(in)))
513514

514515
require.Contains(
515-
t, out, "return nil, "+
516-
"fmt.Errorf(\n \"control "+
517-
"block to bytes: %w\", err,\n )",
516+
t, out, "return nil, fmt.Errorf(\"control block to bytes: "+
517+
"%w\",\n err)",
518518
)
519-
require.NotContains(t, out, `"control block to bytes: %w",`+"\n")
519+
require.NotContains(t, out, "return nil, fmt.Errorf(\n")
520520
}
521521

522522
func TestPipelineNext_LogCalls_ShortErrorfArgsStaySingleLineWhenTheyFit(
@@ -597,6 +597,44 @@ func nodeMaxDepth(depth int) (int, error) {
597597
)
598598
}
599599

600+
func TestPipelineNext_LogCalls_SplitsReturnErrorfWithoutHangingParen(
601+
t *testing.T) {
602+
603+
const in = `package p
604+
605+
import "fmt"
606+
607+
func sum(inputs []Input) (int, error) {
608+
for i := range inputs {
609+
input := inputs[i]
610+
if input.Amount <= 0 {
611+
return 0, fmt.Errorf("input %d amount must be "+
612+
"positive", i)
613+
}
614+
}
615+
616+
return 0, nil
617+
}
618+
`
619+
620+
p := NewPipeline(PipelineConfig{
621+
ColumnLimit: 80,
622+
TabStop: 8,
623+
UseDSLLogCalls: true,
624+
// Keep other DSL stages off to make this test focused.
625+
UseDSLMultiLineCalls: false,
626+
UseDSLExpr: false,
627+
UseDSLComments: false,
628+
UseDSLFuncSigs: false,
629+
UseDSLBlankLines: false,
630+
})
631+
632+
out := string(p.Format([]byte(in)))
633+
634+
require.NotContains(t, out, "fmt.Errorf(\n")
635+
requireNoLineLongerThan(t, out, 80)
636+
}
637+
600638
func TestPipelineNext_LogCalls_FormatCompositeArgAsBlock(t *testing.T) {
601639
const in = `package p
602640

formatter/stage.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,8 @@ func buildStageMap(stages []Stage) (map[string]Stage, []string, error) {
7979
return nil, nil, fmt.Errorf("stage with empty name")
8080
}
8181
if _, exists := stageMap[s.Name]; exists {
82-
return nil, nil, fmt.Errorf(
83-
"duplicate stage name: %q", s.Name,
84-
)
82+
return nil, nil, fmt.Errorf("duplicate stage name: %q",
83+
s.Name)
8584
}
8685
stageMap[s.Name] = s
8786
order = append(order, s.Name)

tools/corpus_check/main.go

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -247,20 +247,17 @@ func buildReport(repos []string, cfg reportConfig) (report, error) {
247247
for _, repoArg := range repos {
248248
root, err := filepath.Abs(repoArg)
249249
if err != nil {
250-
return report{}, fmt.Errorf(
251-
"resolve repo %q: %w", repoArg, err,
252-
)
250+
return report{}, fmt.Errorf("resolve repo %q: %w",
251+
repoArg, err)
253252
}
254253
info, err := os.Stat(root)
255254
if err != nil {
256-
return report{}, fmt.Errorf(
257-
"stat repo %q: %w", root, err,
258-
)
255+
return report{}, fmt.Errorf("stat repo %q: %w", root,
256+
err)
259257
}
260258
if !info.IsDir() {
261-
return report{}, fmt.Errorf(
262-
"repo %q is not a directory", root,
263-
)
259+
return report{}, fmt.Errorf("repo %q is not a "+
260+
"directory", root)
264261
}
265262

266263
summary, cases, err := analyzeRepo(root, cfg)
@@ -782,9 +779,8 @@ func runLLFormat(llformatBin string, col, tabStop int, commentMode string,
782779
}
783780
var ee *exec.ExitError
784781
if errors.As(err, &ee) {
785-
return nil, fmt.Errorf(
786-
"%w: %s", err, strings.TrimSpace(string(ee.Stderr)),
787-
)
782+
return nil, fmt.Errorf("%w: %s", err,
783+
strings.TrimSpace(string(ee.Stderr)))
788784
}
789785

790786
return nil, err

tools/overflow_report/main.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -686,9 +686,8 @@ func runLLFormat(llformatBin string, col, tabStop int,
686686
}
687687
var ee *exec.ExitError
688688
if errors.As(err, &ee) {
689-
return nil, fmt.Errorf(
690-
"%w: %s", err, strings.TrimSpace(string(ee.Stderr)),
691-
)
689+
return nil, fmt.Errorf("%w: %s", err,
690+
strings.TrimSpace(string(ee.Stderr)))
692691
}
693692

694693
return nil, err
@@ -813,9 +812,8 @@ func extractSnippets(originalPath string, formatted []byte,
813812
}
814813
tf := fset.File(f.Pos())
815814
if tf == nil {
816-
return make([]snippet, len(overflows)), fmt.Errorf(
817-
"missing token.File for %s", originalPath,
818-
)
815+
return make([]snippet, len(overflows)), fmt.Errorf("missing "+
816+
"token.File for %s", originalPath)
819817
}
820818

821819
out := make([]snippet, 0, len(overflows))

0 commit comments

Comments
 (0)