Skip to content

Commit 40e1468

Browse files
committed
formatter: count trailing call commas
Account for trailing argument commas when preserving printf calls. This prevents fitting fmt.Errorf blocks from overflowing later. The outer comma is now part of the line-width decision. The change stays scoped to printf-style calls only.
1 parent 436cd29 commit 40e1468

2 files changed

Lines changed: 150 additions & 18 deletions

File tree

dsl/action.go

Lines changed: 117 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2211,16 +2211,6 @@ func trailingCommaSuffixWidth(src []byte, start, tabStop int) int {
22112211
return visualLen(suffix, tabStop)
22122212
}
22132213

2214-
func callHasReturnPrefix(src []byte, start int) bool {
2215-
lineStart := start
2216-
for lineStart > 0 && src[lineStart-1] != '\n' {
2217-
lineStart--
2218-
}
2219-
prefix := strings.TrimSpace(string(src[lineStart:start]))
2220-
2221-
return prefix == "return" || strings.HasPrefix(prefix, "return ")
2222-
}
2223-
22242214
// LeftFlowCallAction formats log/printf calls using left-flow packing with
22252215
// string splitting. This action delegates to the legacy formatter to ensure
22262216
// identical output behavior.
@@ -2270,8 +2260,9 @@ func (a *LeftFlowCallAction) Execute(caps Captures, ctx *Context) ([]byte,
22702260
// Find the base length (visual width from line start to call start).
22712261
baseLen := prefixWidthAt(ctx.Source, start, ctx.TabStop)
22722262
effectiveLimit := ctx.ColumnLimit
2273-
if callHasReturnPrefix(ctx.Source, start) {
2274-
suffixWidth := trailingCommaSuffixWidth(
2263+
isPrintfStyle := isLogPrintfFuncName(getFuncName(call))
2264+
if isPrintfStyle {
2265+
suffixWidth := trailingCallCommaSuffixWidth(
22752266
ctx.Source, end, ctx.TabStop,
22762267
)
22772268
if suffixWidth > 0 && effectiveLimit > suffixWidth {
@@ -2291,7 +2282,16 @@ func (a *LeftFlowCallAction) Execute(caps Captures, ctx *Context) ([]byte,
22912282
formatted = formatCallLeftFlowSimple(call, wsIndent, ctx)
22922283
}
22932284

2294-
if formatted == string(original) {
2285+
if formatted == string(original) && isPrintfStyle {
2286+
if fixed, ok := breakCallClosingParenBeforeTrailingSuffix(
2287+
ctx, start, end, string(original), wsIndent,
2288+
); ok {
2289+
2290+
formatted = fixed
2291+
} else {
2292+
return nil, false
2293+
}
2294+
} else if formatted == string(original) {
22952295
return nil, false
22962296
}
22972297

@@ -2302,10 +2302,20 @@ func (a *LeftFlowCallAction) Execute(caps Captures, ctx *Context) ([]byte,
23022302
origNorm := normalizeCallWithGofmt(string(original), wsIndent)
23032303
fmtNorm := normalizeCallWithGofmt(formatted, wsIndent)
23042304

2305-
if origNorm == fmtNorm {
2305+
if origNorm == fmtNorm && isPrintfStyle {
2306+
if fixed, ok := breakCallClosingParenBeforeTrailingSuffix(
2307+
ctx, start, end, string(original), wsIndent,
2308+
); ok {
23062309

2307-
// After gofmt normalization, both produce the same output. This
2308-
// means our change would be undone by gofmt - skip it.
2310+
formatted = fixed
2311+
} else {
2312+
2313+
// After gofmt normalization, both produce the same
2314+
// output. This means our change would be undone by
2315+
// gofmt - skip it.
2316+
return nil, false
2317+
}
2318+
} else if origNorm == fmtNorm {
23092319
return nil, false
23102320
}
23112321

@@ -2331,8 +2341,97 @@ func shouldPreserveFittingMultilineErrorf(ctx *Context, call *ast.CallExpr,
23312341
return false
23322342
}
23332343

2334-
return maxVisualLineLenInSpan(ctx.Source, start, end, ctx.TabStop) <=
2335-
ctx.ColumnLimit
2344+
maxLen := maxVisualLineLenInSpan(ctx.Source, start, end, ctx.TabStop)
2345+
suffixWidth := trailingCallCommaSuffixWidth(
2346+
ctx.Source, end, ctx.TabStop,
2347+
)
2348+
if suffixWidth > 0 {
2349+
lastLen := visualLen(
2350+
string(ctx.Source[lineStart(ctx.Source, end):end]),
2351+
ctx.TabStop,
2352+
)
2353+
if lastLen+suffixWidth > maxLen {
2354+
maxLen = lastLen + suffixWidth
2355+
}
2356+
}
2357+
2358+
return maxLen <= ctx.ColumnLimit
2359+
}
2360+
2361+
func breakCallClosingParenBeforeTrailingSuffix(ctx *Context, start, end int,
2362+
original string, wsIndent string) (string, bool) {
2363+
2364+
if !strings.Contains(original, "\n") {
2365+
return "", false
2366+
}
2367+
suffixWidth := trailingCallCommaSuffixWidth(
2368+
ctx.Source, end, ctx.TabStop,
2369+
)
2370+
if suffixWidth == 0 {
2371+
return "", false
2372+
}
2373+
2374+
lastLen := visualLen(
2375+
string(ctx.Source[lineStart(ctx.Source, end):end]), ctx.TabStop,
2376+
)
2377+
if lastLen+suffixWidth <= ctx.ColumnLimit {
2378+
return "", false
2379+
}
2380+
2381+
closeIdx := strings.LastIndex(original, ")")
2382+
if closeIdx < 0 && end < len(ctx.Source) && ctx.Source[end] == ')' {
2383+
before := strings.TrimRight(original, " \t")
2384+
if before == "" || strings.HasSuffix(before, ",") {
2385+
return "", false
2386+
}
2387+
2388+
return before + ",\n" + wsIndent, true
2389+
}
2390+
if closeIdx < 0 || strings.TrimSpace(original[closeIdx+1:]) != "" {
2391+
return "", false
2392+
}
2393+
lineStartIdx := strings.LastIndex(original[:closeIdx], "\n") + 1
2394+
if strings.TrimSpace(original[lineStartIdx:closeIdx]) == "" {
2395+
return "", false
2396+
}
2397+
before := strings.TrimRight(original[:closeIdx], " \t")
2398+
if strings.HasSuffix(before, ",") {
2399+
return "", false
2400+
}
2401+
2402+
return before + ",\n" + wsIndent + ")", true
2403+
}
2404+
2405+
func trailingCallCommaSuffixWidth(src []byte, start, tabStop int) int {
2406+
width := trailingCommaSuffixWidth(src, start, tabStop)
2407+
if width > 0 {
2408+
return width
2409+
}
2410+
if start >= len(src) || src[start] != ')' {
2411+
return 0
2412+
}
2413+
2414+
lineEnd := start
2415+
for lineEnd < len(src) && src[lineEnd] != '\n' {
2416+
lineEnd++
2417+
}
2418+
suffix := string(src[start:lineEnd])
2419+
if !strings.HasPrefix(strings.TrimSpace(suffix), "),") {
2420+
return 0
2421+
}
2422+
2423+
return visualLen(suffix, tabStop)
2424+
}
2425+
2426+
func isLogPrintfFuncName(name string) bool {
2427+
if isLogPrintfName(name, nil) {
2428+
return true
2429+
}
2430+
if dot := strings.LastIndex(name, "."); dot >= 0 {
2431+
return isLogPrintfName(name[dot+1:], nil)
2432+
}
2433+
2434+
return false
23362435
}
23372436

23382437
func callIsCompositeKeyValue(call *ast.CallExpr, ctx *Context) bool {

formatter/pipeline_corpus_regression_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,39 @@ func f(client ClientAPI) Response {
359359
requireNoLineLongerThan(t, out, 80)
360360
}
361361

362+
func TestPipelineNext_Corpus_ReflowsErrorfWhenOuterCommaOverflows(
363+
t *testing.T) {
364+
365+
const in = `package p
366+
367+
import "fmt"
368+
369+
type ConfResp struct{}
370+
371+
var fn struct {
372+
Err func(error) ConfResp
373+
}
374+
375+
func f(err error) ConfResp {
376+
if err != nil {
377+
return fn.Err[ConfResp](
378+
fmt.Errorf(
379+
"failed to register for confirmations: %w", err),
380+
)
381+
}
382+
383+
return ConfResp{}
384+
}
385+
`
386+
387+
out := formatWithDefaultNext(t, in)
388+
389+
require.NotContains(
390+
t, out, "\"failed to register for confirmations: %w\", err),",
391+
)
392+
requireNoLineLongerThan(t, out, 80)
393+
}
394+
362395
func TestPipelineNext_Corpus_ExpandsCompositeInCallArgFuncBody(t *testing.T) {
363396
const in = `package p
364397

0 commit comments

Comments
 (0)