diff --git a/dsl/action.go b/dsl/action.go index 13a6280..6f36205 100644 --- a/dsl/action.go +++ b/dsl/action.go @@ -2771,6 +2771,18 @@ type PackedMultiLineCallAction struct { OnlyIfSingleLine bool } +// CollapseSimpleCallArgsAction collapses over-expanded single-argument calls +// when the compact argument list fits at the call's current line position. +type CollapseSimpleCallArgsAction struct { + Target string +} + +// PackReturnStmtAction greedily packs multi-result return statements while +// keeping simple nested calls on one line when they fit. +type PackReturnStmtAction struct { + Target string +} + // BreakSelectorCallArgsAction breaks only the argument list of a selector call // whose callee head should remain flat, such as `make(x).Check(...)`. type BreakSelectorCallArgsAction struct { @@ -3185,6 +3197,10 @@ func (a *PackedMultiLineCallAction) Execute(caps Captures, ctx *Context) ( if hasAnyComment(string(original)) { return nil, false } + if compact, ok := compactSingleArgCallText(ctx, call, start, end); ok && + replacementLinesFitWithinLimit(ctx, start, end, compact) { + return nil, false + } wsIndent := ctx.IndentAt(call) callText := string(original) @@ -3248,6 +3264,215 @@ func (a *PackedMultiLineCallAction) Execute(caps Captures, ctx *Context) ( return out, true } +// Execute implements Action for CollapseSimpleCallArgsAction. +func (a *CollapseSimpleCallArgsAction) Execute(caps Captures, ctx *Context) ( + []byte, bool) { + + node := resolveTarget(caps, a.Target) + call, ok := node.(*ast.CallExpr) + if !ok || call == nil || len(call.Args) != 1 { + return nil, false + } + + start, end, ok := callSpanOffsets(ctx, call) + if !ok { + return nil, false + } + + original := string(ctx.Source[start:end]) + if !strings.Contains(original, "\n") || hasAnyComment(original) { + return nil, false + } + if !isSimpleCallArg(call.Args[0]) { + return nil, false + } + + formatted, ok := compactSingleArgCallText(ctx, call, start, end) + if !ok { + return nil, false + } + if formatted == original || + !replacementLinesFitWithinLimit(ctx, start, end, formatted) { + return nil, false + } + + out, err := ApplySingleEdit( + ctx.Source, start, end, []byte(formatted), + ) + if err != nil || !parseCheckOK(out) { + return nil, false + } + + return out, true +} + +func compactSingleArgCallText(ctx *Context, call *ast.CallExpr, start, + end int) (string, bool) { + + if ctx == nil || call == nil || len(call.Args) != 1 || + !isSimpleCallArg(call.Args[0]) { + return "", false + } + + lparen := ctx.Fset.Position(call.Lparen).Offset + rparen := ctx.Fset.Position(call.Rparen).Offset + if lparen < start || rparen <= lparen || rparen > end { + return "", false + } + + head := string(ctx.Source[start:lparen]) + if strings.Contains(head, "[\n") { + return "", false + } + + arg := strings.TrimSpace(renderNode(call.Args[0], ctx.Fset)) + if arg == "" || strings.Contains(arg, "\n") || hasAnyComment(arg) { + return "", false + } + + return head + "(" + arg + ")", true +} + +// Execute implements Action for PackReturnStmtAction. +func (a *PackReturnStmtAction) Execute(caps Captures, ctx *Context) ([]byte, + bool) { + + node := resolveTarget(caps, a.Target) + ret, ok := node.(*ast.ReturnStmt) + if !ok || ret == nil || len(ret.Results) < 2 { + return nil, false + } + + start := ctx.Fset.Position(ret.Pos()).Offset + end := ctx.Fset.Position(ret.End()).Offset + if start < 0 || end > len(ctx.Source) || start >= end { + return nil, false + } + + original := string(ctx.Source[start:end]) + if hasAnyComment(original) { + return nil, false + } + + exprs := make([]string, 0, len(ret.Results)) + for _, expr := range ret.Results { + text, ok := compactReturnExprText(ctx, expr) + if !ok { + return nil, false + } + exprs = append(exprs, text) + } + + formatted := formatReturnResultsPacked( + exprs, ctx.IndentAt(ret), ctx.ColumnLimit, ctx.TabStop, + ) + if formatted == original || + !replacementLinesFitWithinLimit(ctx, start, end, formatted) { + return nil, false + } + + out, err := ApplySingleEdit( + ctx.Source, start, end, []byte(formatted), + ) + if err != nil || !parseCheckOK(out) { + return nil, false + } + + return out, true +} + +func compactReturnExprText(ctx *Context, expr ast.Expr) (string, bool) { + start := ctx.Fset.Position(expr.Pos()).Offset + end := ctx.Fset.Position(expr.End()).Offset + if start < 0 || end > len(ctx.Source) || start >= end { + return "", false + } + + orig := string(ctx.Source[start:end]) + if hasAnyComment(orig) { + return "", false + } + if call, ok := expr.(*ast.CallExpr); ok { + if compact, ok := compactSingleArgCallText( + ctx, call, start, end, + ); ok { + return compact, true + } + } + if strings.Contains(orig, "\n") { + return "", false + } + + text := strings.TrimSpace(renderNode(expr, ctx.Fset)) + if text == "" || strings.Contains(text, "\n") || hasAnyComment(text) { + return "", false + } + + return text, true +} + +func formatReturnResultsPacked(exprs []string, indent string, colLimit, + tabStop int) string { + + contIndent := indent + "\t" + var b strings.Builder + b.WriteString("return ") + current := indent + "return " + for i, expr := range exprs { + sep := "" + if i > 0 { + sep = ", " + } + candidate := current + sep + expr + if i < len(exprs)-1 { + candidate += "," + } + if i > 0 && visualLen(candidate, tabStop) > colLimit { + b.WriteString(",\n") + b.WriteString(contIndent) + b.WriteString(expr) + current = contIndent + expr + continue + } + + b.WriteString(sep) + b.WriteString(expr) + current += sep + expr + } + + return b.String() +} + +func replacementLinesFitWithinLimit(ctx *Context, start, end int, + replacement string) bool { + + if ctx == nil { + return false + } + prefix := string(ctx.Source[lineStart(ctx.Source, start):start]) + suffix := string(ctx.Source[end:lineEnd(ctx.Source, end)]) + lines := strings.Split(replacement, "\n") + if len(lines) == 0 { + return true + } + if len(lines) == 1 { + return visualLen(prefix+lines[0]+suffix, ctx.TabStop) <= + ctx.ColumnLimit + } + + if visualLen(prefix+lines[0], ctx.TabStop) > ctx.ColumnLimit { + return false + } + for _, line := range lines[1 : len(lines)-1] { + if visualLen(line, ctx.TabStop) > ctx.ColumnLimit { + return false + } + } + + return visualLen(lines[len(lines)-1]+suffix, ctx.TabStop) <= + ctx.ColumnLimit +} + func callAlreadyAcceptableWithMultilineLiteralArg(ctx *Context, call *ast.CallExpr, start, end int) bool { diff --git a/dsl/composite_lit_expand_action.go b/dsl/composite_lit_expand_action.go index 22b5da3..de77477 100644 --- a/dsl/composite_lit_expand_action.go +++ b/dsl/composite_lit_expand_action.go @@ -319,6 +319,50 @@ func (a *ExpandCompositeLitAction) Execute(caps Captures, ctx *Context) ([]byte, } if strings.Contains(orig, "\n") { + if formatted, ok := formatElidedCompositeLitValue( + lit, ctx, start, end, + ); ok { + + out, err := ApplySingleEdit( + ctx.Source, start, end, []byte(formatted), + ) + if err != nil || !parseCheckOK(out) { + return nil, false + } + + return out, true + } + if formatted, ok := packSimpleElidedCompositeLitElements( + lit, ctx, start, end, + ); ok { + + out, err := ApplySingleEdit( + ctx.Source, start, end, []byte(formatted), + ) + if err != nil || !parseCheckOK(out) { + return nil, false + } + + return out, true + } + if out, changed := breakAdjacentCompositeLitElements( + lit, ctx, + ); changed { + return out, true + } + if formatted, ok := collapseSimpleCompositeLit( + lit, ctx, start, end, + ); ok { + + out, err := ApplySingleEdit( + ctx.Source, start, end, []byte(formatted), + ) + if err != nil || !parseCheckOK(out) { + return nil, false + } + + return out, true + } if out, changed := breakCompositeLitTypeArgs( lit, ctx, ); changed { @@ -375,6 +419,10 @@ func shouldExpandCompositeLit(lit *ast.CompositeLit, ctx *Context) bool { return false } + if compositeLitCompactSingleUnkeyedFits(lit, ctx) { + return false + } + if ctx.LineWidth(lit) > ctx.ColumnLimit { return true } @@ -391,6 +439,334 @@ func shouldExpandCompositeLit(lit *ast.CompositeLit, ctx *Context) bool { return false } +func breakAdjacentCompositeLitElements(lit *ast.CompositeLit, + ctx *Context) ([]byte, bool) { + + if lit == nil || ctx == nil || len(lit.Elts) < 2 { + return nil, false + } + + elemIndent := ctx.IndentAt(lit) + "\t" + var b EditBuilder + changed := false + for i := 1; i < len(lit.Elts); i++ { + prev, ok := lit.Elts[i-1].(*ast.CompositeLit) + if !ok || prev == nil { + continue + } + next, ok := lit.Elts[i].(*ast.CompositeLit) + if !ok || next == nil { + continue + } + + prevEnd := ctx.Fset.Position(prev.End()).Offset + nextStart := ctx.Fset.Position(next.Pos()).Offset + if prevEnd < 0 || nextStart <= prevEnd || + nextStart > len(ctx.Source) { + + continue + } + if lineStart(ctx.Source, prevEnd) != + lineStart(ctx.Source, nextStart) { + + continue + } + if strings.TrimSpace( + string(ctx.Source[prevEnd:nextStart]), + ) != "," { + + continue + } + if isCompactElidedCompositeLit(prev, ctx) && + isCompactElidedCompositeLit(next, ctx) { + + continue + } + + b.Replace(prevEnd, nextStart, []byte(",\n"+elemIndent)) + changed = true + } + if !changed { + return nil, false + } + + out, changed, err := b.Apply(ctx.Source) + if err != nil || !changed || !parseCheckOK(out) { + return nil, false + } + + return out, true +} + +func collapseSimpleCompositeLit(lit *ast.CompositeLit, ctx *Context, start, + end int) (string, bool) { + + if lit != nil && ctx != nil && lit.Type == nil { + kv, ok := ctx.Parent(lit).(*ast.KeyValueExpr) + if ok && kv != nil && kv.Value == lit { + return "", false + } + } + + formatted, ok := compactSingleUnkeyedCompositeLit(lit, ctx) + if !ok || formatted == string(ctx.Source[start:end]) { + return "", false + } + if !replacementLinesFitWithinLimit(ctx, start, end, formatted) { + return "", false + } + + return formatted, true +} + +func compositeLitCompactSingleUnkeyedFits(lit *ast.CompositeLit, + ctx *Context) bool { + + formatted, ok := compactSingleUnkeyedCompositeLit(lit, ctx) + if !ok { + return false + } + + start := ctx.Fset.Position(lit.Pos()).Offset + end := ctx.Fset.Position(lit.End()).Offset + if start < 0 || end > len(ctx.Source) || start >= end { + return false + } + + return replacementLinesFitWithinLimit(ctx, start, end, formatted) +} + +func compactSingleUnkeyedCompositeLit(lit *ast.CompositeLit, + ctx *Context) (string, bool) { + + if lit == nil || ctx == nil { + return "", false + } + if lit.Type == nil { + return compactElidedCompositeLit(lit, ctx) + } + if len(lit.Elts) != 1 { + return "", false + } + if _, ok := lit.Elts[0].(*ast.KeyValueExpr); ok { + return "", false + } + + typeText := strings.TrimSpace(renderNode(lit.Type, ctx.Fset)) + eltText := strings.TrimSpace(renderNode(lit.Elts[0], ctx.Fset)) + if typeText == "" || eltText == "" || + strings.Contains(typeText, "\n") || + strings.Contains(eltText, "\n") || + hasAnyComment(typeText) || hasAnyComment(eltText) { + return "", false + } + + return typeText + "{" + eltText + "}", true +} + +func compactElidedCompositeLit(lit *ast.CompositeLit, + ctx *Context) (string, bool) { + + if lit == nil || ctx == nil || lit.Type != nil { + return "", false + } + if len(lit.Elts) == 0 { + return "{}", true + } + if len(lit.Elts) != 1 { + return "", false + } + switch lit.Elts[0].(type) { + case *ast.KeyValueExpr, *ast.CompositeLit: + return "", false + } + + eltText := strings.TrimSpace(renderNode(lit.Elts[0], ctx.Fset)) + if eltText == "" || strings.Contains(eltText, "\n") || + hasAnyComment(eltText) { + return "", false + } + + return "{" + eltText + "}", true +} + +func isCompactElidedCompositeLit(lit *ast.CompositeLit, ctx *Context) bool { + formatted, ok := compactElidedCompositeLit(lit, ctx) + if !ok { + return false + } + + return strings.TrimSpace(string(ctx.NodeSource(lit))) == formatted +} + +func packSimpleElidedCompositeLitElements(lit *ast.CompositeLit, + ctx *Context, start, end int) (string, bool) { + + if lit == nil || ctx == nil || len(lit.Elts) < 2 || + hasPackedElidedCompositeLitElements(lit, ctx) { + return "", false + } + + elems := make([]string, 0, len(lit.Elts)) + for _, elt := range lit.Elts { + eltLit, ok := elt.(*ast.CompositeLit) + if !ok || eltLit == nil { + return "", false + } + formatted, ok := compactElidedCompositeLit(eltLit, ctx) + if !ok { + return "", false + } + elems = append(elems, formatted) + } + + formatted, ok := formatSimpleElidedCompositeLitElements( + lit, ctx, elems, + ) + if !ok || formatted == string(ctx.Source[start:end]) { + return "", false + } + + return formatted, true +} + +func hasPackedElidedCompositeLitElements(lit *ast.CompositeLit, + ctx *Context) bool { + + for i := 1; i < len(lit.Elts); i++ { + prev, ok := lit.Elts[i-1].(*ast.CompositeLit) + if !ok || !isCompactElidedCompositeLit(prev, ctx) { + continue + } + next, ok := lit.Elts[i].(*ast.CompositeLit) + if !ok || !isCompactElidedCompositeLit(next, ctx) { + continue + } + + prevEnd := ctx.Fset.Position(prev.End()).Offset + nextStart := ctx.Fset.Position(next.Pos()).Offset + if prevEnd >= 0 && nextStart > prevEnd && + lineStart(ctx.Source, prevEnd) == + lineStart(ctx.Source, nextStart) { + return true + } + } + + return false +} + +func formatSimpleElidedCompositeLitElements(lit *ast.CompositeLit, + ctx *Context, elems []string) (string, bool) { + + wsIndent := ctx.IndentAt(lit) + elemIndent := wsIndent + "\t" + + typeText := "" + if lit.Type != nil { + typeText = strings.TrimSpace(renderNode(lit.Type, ctx.Fset)) + if typeText == "" || strings.Contains(typeText, "\n") || + hasAnyComment(typeText) { + return "", false + } + } + + open := "{" + if typeText != "" { + open = typeText + "{" + } + + var out strings.Builder + out.WriteString(open) + out.WriteByte('\n') + + line := elemIndent + for _, elem := range elems { + if line == elemIndent { + line += elem + } else { + candidate := line + ", " + elem + if visualLen(candidate+",", ctx.TabStop) <= ctx.ColumnLimit { + line = candidate + } else { + out.WriteString(line) + out.WriteString(",\n") + line = elemIndent + elem + } + } + if visualLen(line+",", ctx.TabStop) > ctx.ColumnLimit { + return "", false + } + } + out.WriteString(line) + out.WriteString(",\n") + out.WriteString(wsIndent) + out.WriteString("}") + + return out.String(), true +} + +func formatElidedCompositeLitValue(lit *ast.CompositeLit, ctx *Context, start, + end int) (string, bool) { + + if lit == nil || ctx == nil || lit.Type != nil || len(lit.Elts) == 0 { + return "", false + } + + kv, ok := ctx.Parent(lit).(*ast.KeyValueExpr) + if !ok || kv == nil || kv.Value != lit { + return "", false + } + + for _, elt := range lit.Elts { + eltLit, ok := elt.(*ast.CompositeLit) + if !ok || eltLit == nil || eltLit.Type != nil || + isCompactElidedCompositeLit(eltLit, ctx) { + return "", false + } + } + + formatted := formatElidedCompositeLitValueMultiline(lit, ctx) + if formatted == "" || formatted == string(ctx.Source[start:end]) { + return "", false + } + + return formatted, true +} + +func formatElidedCompositeLitValueMultiline(lit *ast.CompositeLit, + ctx *Context) string { + + wsIndent := ctx.IndentAt(lit) + elemIndent := wsIndent + "\t" + fieldIndent := elemIndent + "\t" + + var out strings.Builder + out.WriteString("{\n") + for _, elt := range lit.Elts { + eltLit := elt.(*ast.CompositeLit) + out.WriteString(elemIndent) + out.WriteString("{\n") + for _, inner := range eltLit.Elts { + eltText := strings.TrimSpace( + string( + ctx.NodeSource(inner), + ), + ) + if eltText == "" { + return "" + } + writeCompositeElement(&out, fieldIndent, eltText) + out.WriteString(",\n") + } + out.WriteString(elemIndent) + out.WriteString("},\n") + } + out.WriteString(wsIndent) + out.WriteString("}") + + return out.String() +} + func hasVarOrConstParent(lit *ast.CompositeLit, ctx *Context) bool { for cur := ast.Node(lit); cur != nil; cur = ctx.Parent(cur) { if _, ok := cur.(*ast.ValueSpec); ok { diff --git a/dsl/condition.go b/dsl/condition.go index d186e73..7ec89c6 100644 --- a/dsl/condition.go +++ b/dsl/condition.go @@ -45,6 +45,45 @@ func (c *IsCallFuncInListCond) Eval(caps Captures, ctx *Context) bool { return stringInSlice(callExprFuncName(call), c.Names) } +// CallArgCountCond checks the number of arguments in a CallExpr. +type CallArgCountCond struct { + Target string + Op string + Value int +} + +func (c *CallArgCountCond) Eval(caps Captures, ctx *Context) bool { + node := resolveTarget(caps, c.Target) + call, ok := node.(*ast.CallExpr) + if !ok { + return false + } + + got := len(call.Args) + switch c.Op { + case ">": + return got > c.Value + + case ">=": + return got >= c.Value + + case "<": + return got < c.Value + + case "<=": + return got <= c.Value + + case "==": + return got == c.Value + + case "!=": + return got != c.Value + + default: + return false + } +} + // IsCallFuncContainsAnyCond checks whether the target node is a CallExpr whose // callee name contains any of the provided substrings. This matches legacy // multiline-exclude semantics (strings.Contains). diff --git a/dsl/condition_logprintf_selector_prefixes_test.go b/dsl/condition_logprintf_selector_prefixes_test.go index 811b44a..9f8a0c9 100644 --- a/dsl/condition_logprintf_selector_prefixes_test.go +++ b/dsl/condition_logprintf_selector_prefixes_test.go @@ -27,18 +27,14 @@ func TestIsLogOrPrintfCallCond_SelectorPrefixes_RestrictsWhenMatchAnyEnabled( condAllowed := &IsLogOrPrintfCallCond{ Target: "node", MatchAnySelectorPrefix: true, - SelectorPrefixes: []string{ - "rpcSLog", - }, + SelectorPrefixes: []string{"rpcSLog"}, } require.True(t, condAllowed.Eval(Captures{"node": call}, &Context{})) condDenied := &IsLogOrPrintfCallCond{ Target: "node", MatchAnySelectorPrefix: true, - SelectorPrefixes: []string{ - "otherLog", - }, + SelectorPrefixes: []string{"otherLog"}, } require.False(t, condDenied.Eval(Captures{"node": call}, &Context{})) } @@ -61,9 +57,7 @@ func TestIsLogOrPrintfCallCond_SelectorPrefixes_ExpandWhenMatchAnyDisabled( cond := &IsLogOrPrintfCallCond{ Target: "node", MatchAnySelectorPrefix: false, - SelectorPrefixes: []string{ - "rpcSLog", - }, + SelectorPrefixes: []string{"rpcSLog"}, } require.True(t, cond.Eval(Captures{"node": call}, &Context{})) } @@ -86,9 +80,7 @@ func TestIsLogOrPrintfCallCond_SelectorPrefixes_DoNotBlockCanonicalMatches( cond := &IsLogOrPrintfCallCond{ Target: "node", MatchAnySelectorPrefix: true, - SelectorPrefixes: []string{ - "rpcSLog", - }, + SelectorPrefixes: []string{"rpcSLog"}, } require.True(t, cond.Eval(Captures{"node": call}, &Context{})) } @@ -109,9 +101,7 @@ func TestIsLogOrPrintfCallCond_SelectorNamesOverride(t *testing.T) { cond := &IsLogOrPrintfCallCond{ Target: "node", MatchAnySelectorPrefix: true, - SelectorNames: []string{ - "Noticef", - }, + SelectorNames: []string{"Noticef"}, } require.True(t, cond.Eval(Captures{"node": call}, &Context{})) diff --git a/dsl/engine.go b/dsl/engine.go index 585dd2e..7745250 100644 --- a/dsl/engine.go +++ b/dsl/engine.go @@ -286,16 +286,11 @@ func (e *Engine) estimateMaxIterations(src []byte) int { if n == nil { return true } - rt := reflect.TypeOf(n) - if rt == nil { - return true - } - if rt.Kind() == reflect.Ptr { - rt = rt.Elem() - } - if rt == nil { + v := reflect.Indirect(reflect.ValueOf(n)) + if !v.IsValid() { return true } + rt := v.Type() if _, ok := nodeTypes[rt.Name()]; ok { candidates++ } @@ -692,8 +687,8 @@ func ensureParseable(ctx *Context, src []byte, return true, "" } -func collectNodesAndParents(file *ast.File) ([]ast.Node, - map[ast.Node]ast.Node) { +func collectNodesAndParents( + file *ast.File) ([]ast.Node, map[ast.Node]ast.Node) { // We need parent links for conditions like parent()/scope(). Capture // parents while traversing the AST once to avoid repeated reflection- @@ -833,7 +828,8 @@ func (e *Engine) executeEditAction(editAction EditAction, caps Captures, applied, err := ApplyEdits(ctx.Source, edits) if err != nil { - return nil, false, false, "edit_action=apply_edits_error=" + err.Error() + return nil, false, false, + "edit_action=apply_edits_error=" + err.Error() } // Never accept a transformation that produces syntactically invalid Go diff --git a/dsl/rules.go b/dsl/rules.go index 141a2c1..34fb5ac 100644 --- a/dsl/rules.go +++ b/dsl/rules.go @@ -1110,12 +1110,36 @@ func LogPrintfRulesWithOptions(opts LogPrintfOptions, // Only check if this is a log/printf call - the action // will normalize the format and skip if already in // correct format. - When: &IsLogOrPrintfCallCond{ - Target: "node", - MatchAnySelectorPrefix: opts.MatchAnySelectorPrefix, - SelectorNames: opts.SelectorNames, - SelectorPrefixes: opts.SelectorPrefixes, - IncludeNonFStringCalls: opts.IncludeNonFStringCalls, + When: &AndCond{ + Conds: []Condition{ + &IsLogOrPrintfCallCond{ + Target: "node", + MatchAnySelectorPrefix: opts.MatchAnySelectorPrefix, + SelectorNames: opts.SelectorNames, + SelectorPrefixes: opts.SelectorPrefixes, + IncludeNonFStringCalls: opts.IncludeNonFStringCalls, + }, + &NotCond{ + Cond: &AndCond{ + Conds: []Condition{ + &IsCallArgCond{ + Target: "node", + }, + &IsCallFuncInListCond{ + Target: "node", + Names: []string{ + "fmt.Errorf", + }, + }, + &CallArgCountCond{ + Target: "node", + Op: ">=", + Value: 3, + }, + }, + }, + }, + }, }, Priority: 75, Action: action, @@ -1309,6 +1333,9 @@ func MultiLineCallRulesWithOptions(opts MultiLineCallOptions, } var rules []Rule + rules = append( + rules, collapseSimpleCallArgsRule(), packReturnStmtRule(), + ) // Method chain rule - higher priority, handles chains specially. // @@ -1388,6 +1415,34 @@ func MultiLineCallRulesWithOptions(opts MultiLineCallOptions, return rules } +func collapseSimpleCallArgsRule() Rule { + return Rule{ + Name: "collapse_simple_call_args", + Pattern: &NodePattern{ + Type: "CallExpr", + }, + When: TrueCond{}, + Priority: 70, + Action: &CollapseSimpleCallArgsAction{ + Target: "node", + }, + } +} + +func packReturnStmtRule() Rule { + return Rule{ + Name: "pack_return_stmt", + Pattern: &NodePattern{ + Type: "ReturnStmt", + }, + When: TrueCond{}, + Priority: 69, + Action: &PackReturnStmtAction{ + Target: "node", + }, + } +} + // PackedMultiLineOnlyRules returns multiline call rules that format only // non-method-chain call expressions using packed multiline formatting. // @@ -1431,48 +1486,51 @@ func PackedMultiLineOnlyRulesWithOptions(opts MultiLineCallOptions, ) } - return []Rule{ - { - Name: "long_call_expr_packed", - Pattern: &NodePattern{ - Type: "CallExpr", - }, - When: &AndCond{ - Conds: []Condition{ - &OrCond{ - Conds: spanWidthConds, - }, - &NotCond{ - Cond: &IsLogOrPrintfCallCond{ - Target: "node", - MatchAnySelectorPrefix: true, - SelectorNames: opts.LogCallSelectorNames, - SelectorPrefixes: opts.LogCallSelectorPrefixes, - }, + rules := []Rule{ + collapseSimpleCallArgsRule(), + packReturnStmtRule(), + } + + return append(rules, Rule{ + Name: "long_call_expr_packed", + Pattern: &NodePattern{ + Type: "CallExpr", + }, + When: &AndCond{ + Conds: []Condition{ + &OrCond{ + Conds: spanWidthConds, + }, + &NotCond{ + Cond: &IsLogOrPrintfCallCond{ + Target: "node", + MatchAnySelectorPrefix: true, + SelectorNames: opts.LogCallSelectorNames, + SelectorPrefixes: opts.LogCallSelectorPrefixes, }, - &NotCond{ - Cond: &IsMethodChainCond{ - Target: "node", - MinCalls: 2, - }, + }, + &NotCond{ + Cond: &IsMethodChainCond{ + Target: "node", + MinCalls: 2, }, - &NotCond{ - Cond: &IsCallFuncContainsAnyCond{ - Target: "node", - Names: opts.Excludes, - }, + }, + &NotCond{ + Cond: &IsCallFuncContainsAnyCond{ + Target: "node", + Names: opts.Excludes, }, - &NotCond{ - Cond: &HasAnyCommentCond{ - Target: "node", - }, + }, + &NotCond{ + Cond: &HasAnyCommentCond{ + Target: "node", }, }, }, - Priority: 50, - Action: action, }, - } + Priority: 50, + Action: action, + }) } // LegacyMultiLineCallRules returns a rule set intended to match the legacy diff --git a/formatter/compact_call_formatter.go b/formatter/compact_call_formatter.go index ad42f0d..c7a7606 100644 --- a/formatter/compact_call_formatter.go +++ b/formatter/compact_call_formatter.go @@ -1387,6 +1387,15 @@ func stringLitArgNextWidth(cfg stringLitArgNextConfig, hasMore bool) int { func formatCallPackedMultiLineNext(call []byte, wsIndent, fullPrefix string, trailingComma bool) string { + return formatCallPackedMultiLineNextInternal( + call, wsIndent, fullPrefix, trailingComma, false, + ) +} + +func formatCallPackedMultiLineNextInternal(call []byte, wsIndent, + fullPrefix string, trailingComma bool, + firstStringArgInline bool) string { + s := string(call) open := strings.IndexByte(s, '(') if open == -1 || !strings.HasSuffix(s, ")") { @@ -1438,6 +1447,13 @@ func formatCallPackedMultiLineNext(call []byte, wsIndent, fullPrefix string, head = formattedHead } + if firstStringArgInline { + if formatted, ok := formatCallFirstStringArgInlineNext( + head, args, wsIndent, contIndent, lineWidth, + ); ok { + return formatted + } + } var b strings.Builder b.WriteString(head) @@ -1603,6 +1619,129 @@ func formatCallPackedMultiLineNext(call []byte, wsIndent, fullPrefix string, return b.String() } +func formatCallFirstStringArgInlineNext(head string, args []string, wsIndent, + contIndent string, lineWidth int) (string, bool) { + + if strings.Contains(head, "\n") || len(args) < 2 { + return "", false + } + + first := strings.TrimSpace(args[0]) + firstText, ok := flattenedStringExprText(first) + if first == "" || !ok { + return "", false + } + + firstLiteral := quoteGoString(firstText) + firstLine := head + "(" + firstLiteral + "," + if visualLen(wsIndent)+firstLineLen(firstLine) > lineWidth { + firstLine, ok = formatCallSplitFirstStringArgInlineNext( + head, firstText, wsIndent, contIndent, lineWidth, + ) + if !ok { + return "", false + } + } + + var b strings.Builder + b.WriteString(firstLine) + + firstRestAfterInlineString := strings.Contains(firstLine, "\n") + curLen := lastLineLen(firstLine) + if !firstRestAfterInlineString { + b.WriteByte('\n') + b.WriteString(contIndent) + curLen = visualLen(contIndent) + } + + wroteRest := false + for _, raw := range args[1:] { + arg := strings.TrimSpace(raw) + if arg == "" || strings.Contains(arg, "\n") { + return "", false + } + sep := "" + if wroteRest { + sep = ", " + } else if firstRestAfterInlineString { + sep = " " + } + need := firstLineLen(sep) + firstLineLen(arg) + if (wroteRest || firstRestAfterInlineString) && + curLen+need > lineWidth { + + if wroteRest { + b.WriteByte(',') + } + b.WriteByte('\n') + b.WriteString(contIndent) + b.WriteString(arg) + curLen = visualLen(contIndent) + firstLineLen(arg) + } else { + b.WriteString(sep) + b.WriteString(arg) + curLen += need + } + firstRestAfterInlineString = false + wroteRest = true + } + if !wroteRest { + return "", false + } + if curLen+1 > lineWidth { + b.WriteByte('\n') + b.WriteString(wsIndent) + b.WriteByte(')') + } else { + b.WriteByte(')') + } + + return b.String(), true +} + +func formatCallSplitFirstStringArgInlineNext(head, firstText, wsIndent, + contIndent string, lineWidth int) (string, bool) { + + startCol := visualLen(wsIndent) + firstLineLen(head+"(") + split := buildSplitQuotedForCallArg( + firstText, startCol, contIndent, lineWidth, true, + ) + if !strings.Contains(split, "\n") { + return "", false + } + + firstLine := head + "(" + split + "," + if !replacementTextLinesFitWidth(wsIndent, firstLine, lineWidth) { + return "", false + } + + return firstLine, true +} + +func replacementTextLinesFitWidth(prefix, text string, lineWidth int) bool { + lines := strings.Split(text, "\n") + for i, line := range lines { + candidate := line + if i == 0 { + candidate = prefix + line + } + if visualLen(candidate) > lineWidth { + return false + } + } + + return true +} + +func flattenedStringExprText(s string) (string, bool) { + expr, err := parser.ParseExpr(s) + if err != nil { + return "", false + } + + return llast.FlattenStringExprAST(expr) +} + func formatGenericCallHeadNext(head, wsIndent, fullPrefix string) (string, bool) { @@ -1954,14 +2093,34 @@ func (s *callExprArgNextState) handleCallExprArgNext(a string) bool { } // Need to recursively format the nested call. - nested := formatCallPackedMultiLineNext( + nested := formatCallPackedMultiLineNextInternal( []byte(a), s.contIndent, s.contIndent, true, + isFmtErrorfCallText(a), ) s.writeNestedCall(nested) return true } +func isFmtErrorfCallText(s string) bool { + expr, err := parser.ParseExpr(s) + if err != nil { + return false + } + + call, ok := expr.(*ast.CallExpr) + if !ok { + return false + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel == nil || sel.Sel.Name != "Errorf" { + return false + } + ident, ok := sel.X.(*ast.Ident) + + return ok && ident.Name == "fmt" +} + // writeArgSimple writes an argument that fits inline. func (s *callExprArgNextState) writeArgSimple(a string) { if s.first { diff --git a/formatter/dsl_bundles_next.go b/formatter/dsl_bundles_next.go index 906a59f..d0f20cc 100644 --- a/formatter/dsl_bundles_next.go +++ b/formatter/dsl_bundles_next.go @@ -111,8 +111,8 @@ func dslRulesForExpr(opts StageOptions) []dsl.Rule { return exprRules } -func dslRulesForMultiLineCalls(opts StageOptions) (rules []dsl.Rule, - nodeOrder dsl.NodeOrder) { +func dslRulesForMultiLineCalls( + opts StageOptions) (rules []dsl.Rule, nodeOrder dsl.NodeOrder) { style := opts.Style.DSLMultiLineStyle if style == "" { diff --git a/formatter/expression_formatter.go b/formatter/expression_formatter.go index 1f5db2c..9891f86 100644 --- a/formatter/expression_formatter.go +++ b/formatter/expression_formatter.go @@ -44,7 +44,8 @@ func FormatCompositeLiteralArg(arg, contIndent string, isKV := isKeyedCompositeLiteral(elems) if isKV { - return formatKeyedCompositeLiteral(before, contIndent, elems), true + return formatKeyedCompositeLiteral(before, contIndent, elems), + true } return formatSliceCompositeLiteral( @@ -168,7 +169,8 @@ func formatCompositeLiteralValue(value, elemIndent string) (string, bool) { elems := scanner.SplitTopLevelAny(inside) if isKeyedCompositeLiteral(elems) { - return formatKeyedCompositeLiteral(before, elemIndent, elems), true + return formatKeyedCompositeLiteral(before, elemIndent, elems), + true } return formatSliceCompositeLiteral( diff --git a/formatter/func_sig_formatter.go b/formatter/func_sig_formatter.go index d9ee812..15044c5 100644 --- a/formatter/func_sig_formatter.go +++ b/formatter/func_sig_formatter.go @@ -33,6 +33,12 @@ type FuncSigConfig struct { // return lists (e.g. `([]T, error)`) on one line by breaking parameters // earlier. PreferInlineSmallReturnList bool + // PreferInlineReturnListByBreakingParams extends the inline-return + // preference to methods, single-parameter interface methods, and + // selected function literals. For function literals, it still avoids + // breaking a single named parameter because that creates poorly + // balanced signatures; in that case splitting returns is clearer. + PreferInlineReturnListByBreakingParams bool // BreakLongFuncTypeParams enables breaking of function-typed parameters // when their inner parameter list exceeds the column limit (even when @@ -258,9 +264,10 @@ func FormatFuncSignatureNext(signature, indent string, colLimit, ReserveTrailingComma: true, // Prefer keeping very small return lists inline by breaking // parameters earlier (when feasible). - PreferInlineSmallReturnList: true, - BreakLongFuncTypeParams: true, - FormatInlineStructParams: true, + PreferInlineSmallReturnList: true, + PreferInlineReturnListByBreakingParams: true, + BreakLongFuncTypeParams: true, + FormatInlineStructParams: true, }) // When the signature already contains newlines, prefer to collapse it @@ -315,6 +322,21 @@ func FormatFuncSignatureNext(signature, indent string, colLimit, if width.VisualLenWithTab(indent+collapsed, tabStop) <= colLimit { return indent + collapsed, false } + if formatted, ok := formatPartialSplitReturnsByBreakingParam( + f, signature, indent, colLimit, tabStop, + ); ok { + return formatted, true + } + if signatureLinesFit( + signature, indent, colLimit, tabStop, + ) && !isFuncLitSignature(signature) && + !shouldRebreakFailedMultilineSigCollapse( + f, signature, collapsed, indent, + colLimit, tabStop, + ) { + return indent + signature, + hasNewlineOutsideBraces(signature) + } // Even if it doesn't fit, collapsing whitespace makes // subsequent breaking decisions more consistent. signature = collapsed @@ -351,6 +373,172 @@ func signatureLinesFit(signature, indent string, colLimit, tabStop int) bool { return true } +func formatPartialSplitReturnsByBreakingParam(f *FuncSigFormatter, signature, + indent string, colLimit, tabStop int) (string, bool) { + + if !hasPartialSplitReturnList(signature) || + isFuncLitSignature(signature) { + return "", false + } + + sigAtFunc, hasFuncKeyword := signatureAtFunc( + strings.TrimSpace(signature), + ) + if !hasFuncKeyword || !strings.HasPrefix(sigAtFunc, "func ") || + strings.HasPrefix(sigAtFunc, "func (") { + return "", false + } + + funcPart, rest, _, ok := f.parseSigParts(signature) + if !ok { + return "", false + } + paramEnd := f.findMatchingParen(rest, 0) + if paramEnd == -1 { + return "", false + } + + params := rest[1:paramEnd] + paramList := filterNonEmptyTrimmed(f.splitFuncParamList(params)) + if len(paramList) != 1 || strings.Contains(paramList[0], "\n") || + !hasSingleNamedParam(paramList) || + hasSharedNameParamGroup(paramList) { + return "", false + } + + returns, afterReturns := f.parseSigReturns(rest[paramEnd+1:]) + collapsedReturns, ok := collapseSmallParenReturns(returns) + if !ok { + return "", false + } + + tail := "" + if sigHasBrace(afterReturns) { + tail = " {" + } else if strings.TrimSpace(afterReturns) != "" { + return "", false + } + + contIndent := indent + "\t" + firstLine := indent + funcPart + "(" + secondLine := contIndent + strings.TrimSpace(paramList[0]) + ") " + + collapsedReturns + tail + + if width.VisualLenWithTab(firstLine, tabStop) > colLimit || + width.VisualLenWithTab(secondLine, tabStop) > colLimit { + return "", false + } + + return firstLine + "\n" + secondLine, true +} + +func hasPartialSplitReturnList(signature string) bool { + lines := strings.Split(signature, "\n") + for i, line := range lines { + trimmed := strings.TrimRight(line, " \t") + openIdx := strings.LastIndex(trimmed, ") (") + if openIdx < 0 { + continue + } + for _, restLine := range lines[i+1:] { + if strings.Contains(restLine, ")") { + return true + } + } + } + + return false +} + +func collapseSmallParenReturns(returns string) (string, bool) { + trimmed := strings.TrimSpace(returns) + if len(trimmed) < 2 || trimmed[0] != '(' || + trimmed[len(trimmed)-1] != ')' { + return "", false + } + + content := strings.TrimSpace(trimmed[1 : len(trimmed)-1]) + if content == "" { + return "()", true + } + if strings.ContainsAny(content, "{}") || + strings.Contains(content, "func(") { + return "", false + } + + parts := filterNonEmptyTrimmed(scanner.SplitTopLevel(content)) + if len(parts) == 0 || len(parts) > 2 { + return "", false + } + + return "(" + strings.Join(parts, ", ") + ")", true +} + +func shouldRebreakFailedMultilineSigCollapse(f *FuncSigFormatter, original, + collapsed, indent string, colLimit, tabStop int) bool { + + if !hasSplitReturnOpenAfterMultilineParams(f, original) { + return false + } + + funcPart, rest, _, ok := f.parseSigParts(collapsed) + if !ok { + return false + } + paramEnd := f.findMatchingParen(rest, 0) + if paramEnd == -1 { + return false + } + + params := rest[1:paramEnd] + paramList := filterNonEmptyTrimmed(f.splitFuncParamList(params)) + if len(paramList) == 0 || strings.Contains(paramList[0], "\n") { + return false + } + + returns, afterReturns := f.parseSigReturns(rest[paramEnd+1:]) + if !strings.HasPrefix(returns, "(") { + return false + } + + hasBrace := sigHasBrace(afterReturns) + hasParenReturns := strings.HasPrefix(returns, "(") + trailingMinimal, _ := computeSigTrailing( + returns, hasBrace, hasParenReturns, + ) + + testLine := indent + funcPart + "(" + paramList[0] + if len(paramList) == 1 { + testLine += trailingMinimal + } else { + testLine += "," + } + + return width.VisualLenWithTab(testLine, tabStop) <= colLimit +} + +func hasSplitReturnOpenAfterMultilineParams(f *FuncSigFormatter, + signature string) bool { + + _, rest, _, ok := f.parseSigParts(signature) + if !ok { + return false + } + paramEnd := f.findMatchingParen(rest, 0) + if paramEnd == -1 { + return false + } + + params := rest[1:paramEnd] + if !strings.Contains(params, "\n") { + return false + } + + afterParams := strings.TrimLeft(rest[paramEnd+1:], " \t") + + return strings.HasPrefix(afterParams, "(\n") +} + func collapseMultilineParenReturnListIfFits(signature string, colLimit, tabStop int) string { @@ -1520,7 +1708,7 @@ func (f *FuncSigFormatter) shouldUseInlineReturns(sig, returns string, hasParenReturns bool, paramList []string) bool { if !f.cfg.PreferInlineSmallReturnList || !hasParenReturns || - len(paramList) <= 1 || !isSmallParenReturnList(returns) || + !isSmallParenReturnList(returns) || hasSharedNameParamGroup(paramList) { return false } @@ -1528,13 +1716,43 @@ func (f *FuncSigFormatter) shouldUseInlineReturns(sig, returns string, sigAtFunc, hasFuncKeyword := signatureAtFunc(trimmedSig) isFuncDeclNoRecv := strings.HasPrefix(sigAtFunc, "func ") && !strings.HasPrefix(sigAtFunc, "func (") + isMethodDecl := strings.HasPrefix(sigAtFunc, "func (") isInterfaceMethod := !hasFuncKeyword isFuncLit := isFuncLitSignature(sigAtFunc) - // Function literals are common in call-arg position; for readability we - // prefer keeping their parameter list intact and breaking the return - // list instead (matching the "next" golden spec). - return (isFuncDeclNoRecv || isInterfaceMethod) && !isFuncLit + if !f.cfg.PreferInlineReturnListByBreakingParams { + return len(paramList) > 1 && + (isFuncDeclNoRecv || isInterfaceMethod) && + !isFuncLit + } + + if isFuncLit { + return funcLitCanBreakParamsForInlineReturns(paramList) + } + if isFuncDeclNoRecv && hasSingleNamedParam(paramList) { + return false + } + + return isFuncDeclNoRecv || isMethodDecl || isInterfaceMethod +} + +func hasSingleNamedParam(paramList []string) bool { + if len(paramList) != 1 { + return false + } + + return strings.ContainsAny(strings.TrimSpace(paramList[0]), " \t") +} + +func funcLitCanBreakParamsForInlineReturns(paramList []string) bool { + if len(paramList) != 1 { + return false + } + + // A single named closure parameter usually reads better kept compact, + // with the return list split if needed. An unnamed single parameter + // such as `context.Context` can move to a continuation line cleanly. + return !strings.ContainsAny(strings.TrimSpace(paramList[0]), " \t") } func hasSharedNameParamGroup(paramList []string) bool { @@ -1603,7 +1821,8 @@ func (f *FuncSigFormatter) parseSigReturns(afterParams string) (returns, // Simple return type (no parens) - find where it ends braceIdx := strings.Index(afterParams, "{") if braceIdx != -1 { - return strings.TrimSpace(afterParams[:braceIdx]), afterParams[braceIdx:] + return strings.TrimSpace(afterParams[:braceIdx]), + afterParams[braceIdx:] } return strings.TrimSpace(afterParams), "" @@ -1732,11 +1951,89 @@ func (ctx *returnFormatContext) formatLegacyReturns(currentLine string) { ctx.leftFlowPackReturns(ctx.contIndent) } else { // First return fits inline - use left-flow packing. + if ctx.tryFormatBalancedTwoLineReturns(currentLine) { + return + } ctx.result.WriteString(" (") ctx.leftFlowPackReturns(currentLine + ") (") } } +func (ctx *returnFormatContext) tryFormatBalancedTwoLineReturns( + currentLine string) bool { + + if len(ctx.retList) < 3 { + return false + } + + type candidate struct { + text string + diff int + max int + } + + best := candidate{diff: int(^uint(0) >> 1)} + for split := 1; split < len(ctx.retList); split++ { + firstParts := trimmedReturnParts(ctx.retList[:split]) + secondParts := trimmedReturnParts(ctx.retList[split:]) + if len(firstParts) == 0 || len(secondParts) == 0 { + continue + } + + firstLine := currentLine + ") (" + + strings.Join(firstParts, ", ") + "," + secondLine := ctx.contIndent + + strings.Join(secondParts, ", ") + ")" + if ctx.hasBrace { + secondLine += " {" + } + + firstLen := width.VisualLenWithTab(firstLine, ctx.tabStop) + secondLen := width.VisualLenWithTab(secondLine, ctx.tabStop) + if firstLen > ctx.colLimit || secondLen > ctx.colLimit { + continue + } + + diff := firstLen - secondLen + if diff < 0 { + diff = -diff + } + maxLen := firstLen + if secondLen > maxLen { + maxLen = secondLen + } + if diff < best.diff || diff == best.diff && maxLen < best.max { + best = candidate{ + text: " (" + strings.Join(firstParts, ", ") + + ",\n" + ctx.contIndent + + strings.Join(secondParts, ", ") + ")", + diff: diff, + max: maxLen, + } + } + } + + if best.text == "" { + return false + } + + ctx.result.WriteString(best.text) + + return true +} + +func trimmedReturnParts(parts []string) []string { + out := make([]string, 0, len(parts)) + for _, part := range parts { + part = strings.TrimSpace(part) + if part != "" { + out = append(out, part) + } + } + + return out +} + func isSmallParenReturnList(returns string) bool { trimmed := strings.TrimSpace(returns) if len(trimmed) < 2 || trimmed[0] != '(' || diff --git a/formatter/pipeline_regression_snippets_more_test.go b/formatter/pipeline_regression_snippets_more_test.go index 19bde61..7e0712f 100644 --- a/formatter/pipeline_regression_snippets_more_test.go +++ b/formatter/pipeline_regression_snippets_more_test.go @@ -196,9 +196,7 @@ func ExportedName(x C.int) C.int { //line generated.go:123 func f() int { return 1 } `, - wantContains: []string{ - "//line generated.go:123\n", - }, + wantContains: []string{"//line generated.go:123\n"}, }, { name: "parenthesized_composite_and_func_lits", diff --git a/formatter/pipeline_sigs_interfaces_next_test.go b/formatter/pipeline_sigs_interfaces_next_test.go index 73b7225..0d8d7c1 100644 --- a/formatter/pipeline_sigs_interfaces_next_test.go +++ b/formatter/pipeline_sigs_interfaces_next_test.go @@ -160,9 +160,11 @@ type I interface { ) } -func TestPipelineNext_Signatures_InterfaceMethod_UsesCanonicalMultilineReturnListForLongReturns( +func TestPipelineNext_Signatures_InterfaceMethod_PrefersInlineMapReturnListByBreakingSingleParam( t *testing.T) { + // This uses lntypes.Hash as a representative real-world map key type. + // The implementation does not special-case either name. const in = `package p import ( @@ -202,15 +204,13 @@ type I interface { "parenthesized return list", ) - // For long returns, prefer gofmt-like multiline results. + // Even when the first return type is long, prefer keeping a two-item + // return list inline by breaking the single parameter first. require.Contains( - t, out, ") "+ - "("+ - "\n"+ - " map[lntypes.Hash]Invoice,"+ - "\n error,\n )", "next profile "+ - "should use canonical multiline return lists for "+ - "long return types", + t, out, "FetchPendingInvoices(\n ctx "+ + "context.Context) (map[lntypes.Hash]Invoice, error)", + "expected params to break before forcing a multiline "+ + "two-item return list", ) // No blank lines inside the return list. diff --git a/formatter/pipeline_sigs_next_test.go b/formatter/pipeline_sigs_next_test.go index da74b71..73cbcdd 100644 --- a/formatter/pipeline_sigs_next_test.go +++ b/formatter/pipeline_sigs_next_test.go @@ -91,6 +91,45 @@ func getChainSyncInfo() ( ) } +func TestPipelineNext_Signatures_DoesNotMoveParamWithInlineReturns( + t *testing.T) { + + const in = `package p + +func unmarshalFixedPoint( + fp *looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, error) { + + return nil, nil +} +` + + p := NewPipeline(PipelineConfig{ + ColumnLimit: 80, + TabStop: 8, + UseDSLFuncSigs: true, + UseDSLFuncSigsNative: true, + DSLSigsStyle: "legacy", + // Keep other DSL stages off so this test stays focused. + UseDSLLogCalls: false, + UseDSLMultiLineCalls: false, + UseDSLExpr: false, + UseDSLComments: false, + UseDSLBlankLines: false, + UseDSLBlankLinesNative: false, + }) + + out := string(p.Format([]byte(in))) + + require.Contains( + t, out, "func unmarshalFixedPoint(\n fp "+ + "*looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, "+ + "error) {", + ) + require.NotContains( + t, out, "func unmarshalFixedPoint(fp *looprpc.FixedPoint) (", + ) +} + func TestPipelineNext_Signatures_InsertsBlankLineAfterAlreadyMultilineSignature( t *testing.T) { diff --git a/formatter/rules_test.go b/formatter/rules_test.go index a734e3e..65a95f1 100644 --- a/formatter/rules_test.go +++ b/formatter/rules_test.go @@ -56,10 +56,8 @@ func TestCallRuleMatch(t *testing.T) { func TestCallRuleName(t *testing.T) { rule := &CallRule{ - Patterns: []string{ - "log.Infof(", - }, - Breaker: NewLeftFlowBreaker(), + Patterns: []string{"log.Infof("}, + Breaker: NewLeftFlowBreaker(), } if rule.Name() != "call:log.Infof(" { t.Errorf("Name() = %q, want %q", rule.Name(), "call:log.Infof(") @@ -74,10 +72,8 @@ func TestCallRuleName(t *testing.T) { func TestCallRuleApply(t *testing.T) { rule := &CallRule{ - Patterns: []string{ - "log.Infof(", - }, - Breaker: NewLeftFlowBreaker(), + Patterns: []string{"log.Infof("}, + Breaker: NewLeftFlowBreaker(), } cfg := NewBaseConfig(80, 8) @@ -95,10 +91,8 @@ func TestCallRuleApply(t *testing.T) { func TestCallRuleApplyNoMatch(t *testing.T) { rule := &CallRule{ - Patterns: []string{ - "log.Infof(", - }, - Breaker: NewLeftFlowBreaker(), + Patterns: []string{"log.Infof("}, + Breaker: NewLeftFlowBreaker(), } cfg := NewBaseConfig(80, 8) @@ -183,16 +177,12 @@ func TestDefaultPatterns(t *testing.T) { func TestRuleMatcherMatchAt(t *testing.T) { rule1 := &CallRule{ - Patterns: []string{ - "log.Infof(", - }, - Breaker: NewLeftFlowBreaker(), + Patterns: []string{"log.Infof("}, + Breaker: NewLeftFlowBreaker(), } rule2 := &CallRule{ - Patterns: []string{ - "fmt.Printf(", - }, - Breaker: NewLeftFlowBreaker(), + Patterns: []string{"fmt.Printf("}, + Breaker: NewLeftFlowBreaker(), } matcher := NewRuleMatcher([]Rule{rule1, rule2}, NewBaseConfig(80, 8)) @@ -225,10 +215,8 @@ func TestRuleMatcherMatchAt(t *testing.T) { func TestRuleMatcherApplyAt(t *testing.T) { rule := &CallRule{ - Patterns: []string{ - "log.Infof(", - }, - Breaker: NewLeftFlowBreaker(), + Patterns: []string{"log.Infof("}, + Breaker: NewLeftFlowBreaker(), } matcher := NewRuleMatcher([]Rule{rule}, NewBaseConfig(80, 8)) diff --git a/formatter/stage_test.go b/formatter/stage_test.go index f860f9c..2ecb390 100644 --- a/formatter/stage_test.go +++ b/formatter/stage_test.go @@ -67,9 +67,7 @@ func TestStageOrder(t *testing.T) { func TestStageOrderMissingDep(t *testing.T) { formatter := NoopFormatter{} - stages := []Stage{ - NewStage("stage1", formatter).WithRequires("missing"), - } + stages := []Stage{NewStage("stage1", formatter).WithRequires("missing")} _, err := StageOrder(stages) if err == nil { t.Fatalf("expected error") diff --git a/internal/compat/comment_formatter.go b/internal/compat/comment_formatter.go index 620b9ad..31c9dad 100644 --- a/internal/compat/comment_formatter.go +++ b/internal/compat/comment_formatter.go @@ -419,11 +419,9 @@ func reflowLineCommentBlock(block []string, indent string) []string { flushList() itemText := strings.TrimSpace(afterLead[2:]) curList = ¶{ - kind: paraListItem, - lead: lead, - lines: []string{ - itemText, - }, + kind: paraListItem, + lead: lead, + lines: []string{itemText}, } continue } @@ -534,11 +532,9 @@ func reflowBlockComment(block []string, indent string) []string { flushList() itemText := strings.TrimSpace(afterLead[2:]) curList = ¶{ - kind: paraListItem, - lead: lead, - lines: []string{ - itemText, - }, + kind: paraListItem, + lead: lead, + lines: []string{itemText}, } continue } diff --git a/testdata/expressions/input.go b/testdata/expressions/input.go index 596fcd7..21a989c 100644 --- a/testdata/expressions/input.go +++ b/testdata/expressions/input.go @@ -130,6 +130,74 @@ func example18(x, y, z, w int, enabled, active bool) bool { return x > 0 && y > 0 && z > 0 && w > 0 && x < 100 && y < 100 && z < 100 && w < 100 && enabled && active } +// Example 19: Single-element composite literals should stay compact +func example19SingleElementCompositeLiteral(swapRequest SwapRequest) { + expectedOut := []requestResponse{ + { + request: swapRequest, + response: &SwapInfo{ + SwapHash: lntypes.Hash{ + 1, + }, + Marker: sampleMarker{ + "ready", + }, + }, + }, + } + _ = expectedOut +} + +// Example 20: Keyed composite elements should not share a line +func example20KeyedElements(resId1, resId2 ReservationID) { + requests := []ReservationRequest{ + { + ID: resId1, + }, { + ID: resId2, + }, + } + _ = requests +} + +// Example 21: Simple elided composite elements should stay packed +func example21PackedElidedElements() { + hops := []*RouteHop{ + {}, + {}, + {}, + } + preimages := []SamplePreimage{ + {1}, {2}, {3}, + {4}, {5}, {6}, + {7}, {8}, {9}, + } + _, _ = hops, preimages +} + +// Example 22: Elided composite keyed values should expand outer braces +func example22ElidedCompositeKeyedValues() { + requiredOps := map[string][]PermissionOp{ + "/service/Action": {{ + Entity: "swap", + Action: "execute", + }, { + Entity: "service", + Action: "read", + }}, + "/service/Read": {{ + Entity: "swap", + Action: "read", + }}, + } + simpleIDs := map[SampleHash][]DepositID{ + sampleHashA: { + depositID1, + }, + } + _, _ = requiredOps, simpleIDs +} + // Stubs to make the file compile type formatter struct{} @@ -144,6 +212,29 @@ func anotherLongFunction(int, int) int { return 0 } func someCheck(int) bool { return false } func anotherCheck(int) bool { return false } +type SwapRequest struct{} +type SwapInfo struct { + SwapHash interface{} + Marker interface{} +} +type requestResponse struct { + request SwapRequest + response *SwapInfo +} +type sampleMarker []string +type RouteHop struct{} +type SamplePreimage [32]byte +type SampleHash [32]byte +type DepositID int +type PermissionOp struct { + Entity string + Action string +} +type ReservationID struct{} +type ReservationRequest struct { + ID ReservationID +} + type clientType struct{} func (clientType) WithTimeout(time.Duration) clientType { return clientType{} } diff --git a/testdata/expressions/output_next.go b/testdata/expressions/output_next.go index 8281682..e4c0362 100644 --- a/testdata/expressions/output_next.go +++ b/testdata/expressions/output_next.go @@ -176,6 +176,74 @@ func example18(x, y, z, w int, enabled, active bool) bool { z < 100 && w < 100 && enabled && active } +// Example 19: Single-element composite literals should stay compact +func example19SingleElementCompositeLiteral(swapRequest SwapRequest) { + expectedOut := []requestResponse{ + { + request: swapRequest, + response: &SwapInfo{ + SwapHash: lntypes.Hash{1}, + Marker: sampleMarker{"ready"}, + }, + }, + } + _ = expectedOut +} + +// Example 20: Keyed composite elements should not share a line +func example20KeyedElements(resId1, resId2 ReservationID) { + requests := []ReservationRequest{ + { + ID: resId1, + }, + { + ID: resId2, + }, + } + _ = requests +} + +// Example 21: Simple elided composite elements should stay packed +func example21PackedElidedElements() { + hops := []*RouteHop{ + {}, {}, {}, + } + preimages := []SamplePreimage{ + {1}, {2}, {3}, + {4}, {5}, {6}, + {7}, {8}, {9}, + } + _, _ = hops, preimages +} + +// Example 22: Elided composite keyed values should expand outer braces +func example22ElidedCompositeKeyedValues() { + requiredOps := map[string][]PermissionOp{ + "/service/Action": { + { + Entity: "swap", + Action: "execute", + }, + { + Entity: "service", + Action: "read", + }, + }, + "/service/Read": { + { + Entity: "swap", + Action: "read", + }, + }, + } + simpleIDs := map[SampleHash][]DepositID{ + sampleHashA: { + depositID1, + }, + } + _, _ = requiredOps, simpleIDs +} + // Stubs to make the file compile type formatter struct{} @@ -190,6 +258,29 @@ func anotherLongFunction(int, int) int { return 0 } func someCheck(int) bool { return false } func anotherCheck(int) bool { return false } +type SwapRequest struct{} +type SwapInfo struct { + SwapHash interface{} + Marker interface{} +} +type requestResponse struct { + request SwapRequest + response *SwapInfo +} +type sampleMarker []string +type RouteHop struct{} +type SamplePreimage [32]byte +type SampleHash [32]byte +type DepositID int +type PermissionOp struct { + Entity string + Action string +} +type ReservationID struct{} +type ReservationRequest struct { + ID ReservationID +} + type clientType struct{} func (clientType) WithTimeout(time.Duration) clientType { return clientType{} } diff --git a/testdata/logs/input.go b/testdata/logs/input.go index 9c573e8..1c1e72f 100644 --- a/testdata/logs/input.go +++ b/testdata/logs/input.go @@ -447,3 +447,42 @@ func example44NestedFirst() { func example45SpacesTabs() { log.Infof("one two three four five with a long sentence that ensures wrapping") } + +// example46: fmt.Errorf nested in an outer error handler. +// - Exercises keeping the format string attached to fmt.Errorf when possible. +func example46HandleError(preimage Preimage, payReq PayReq) error { + return f.HandleError( + fmt.Errorf( + "invalid swap invoice hash: expected %x got %x", + preimage.Hash(), payReq.Hash, + ), + ) +} + +type Preimage struct{} + +func (Preimage) Hash() string { return "" } + +type PayReq struct { + Hash string +} + +var f sampleErrorHandler + +type sampleErrorHandler struct{} + +func (sampleErrorHandler) HandleError(error) error { return nil } + +// example47: Nested fmt.Errorf with a split format string. +func example47HandleSplitError(preimage Preimage, payReq PayReq) error { + if preimage.Hash() != payReq.Hash { + return f.HandleError( + fmt.Errorf( + "invalid swap invoice hash: expected %x got %x", + preimage.Hash(), payReq.Hash, + ), + ) + } + + return nil +} diff --git a/testdata/logs/output_next.go b/testdata/logs/output_next.go index fbbb65d..f5dabf5 100644 --- a/testdata/logs/output_next.go +++ b/testdata/logs/output_next.go @@ -419,3 +419,38 @@ func example45SpacesTabs() { log.Infof("one two three four five with a long " + "sentence that ensures wrapping") } + +// example46: fmt.Errorf nested in an outer error handler. +// - Exercises keeping the format string attached to fmt.Errorf when possible. +func example46HandleError(preimage Preimage, payReq PayReq) error { + return f.HandleError( + fmt.Errorf("invalid swap invoice hash: expected %x got %x", + preimage.Hash(), payReq.Hash), + ) +} + +type Preimage struct{} + +func (Preimage) Hash() string { return "" } + +type PayReq struct { + Hash string +} + +var f sampleErrorHandler + +type sampleErrorHandler struct{} + +func (sampleErrorHandler) HandleError(error) error { return nil } + +// example47: Nested fmt.Errorf with a split format string. +func example47HandleSplitError(preimage Preimage, payReq PayReq) error { + if preimage.Hash() != payReq.Hash { + return f.HandleError( + fmt.Errorf("invalid swap invoice hash: expected %x "+ + "got %x", preimage.Hash(), payReq.Hash), + ) + } + + return nil +} diff --git a/testdata/multiline/input.go b/testdata/multiline/input.go index 80dace4..69bd97b 100644 --- a/testdata/multiline/input.go +++ b/testdata/multiline/input.go @@ -122,6 +122,42 @@ func main() { chain2().Then("ok").Limit(100) } +// Simple nested calls should not be over-expanded. +func simpleNestedCalls(row Row, quote Quote) error { + maxPrepayRoutingFee := getMaxRoutingFee( + btcutil.Amount( + quote.PrepayAmtSat, + ), + ) + _ = maxPrepayRoutingFee + + err := finalizedHtlcTx.Deserialize( + bytes.NewReader( + row.FinalizedHtlcTx, + ), + ) + return err +} + +// A simple notification argument should stay with the call. +func simpleNotifier(notifCtx context.Context) error { + blockHeightChan, errEpochChan, err := f.cfg.ChainNotifier. + RegisterBlockEpochNtfn( + notifCtx, + ) + _, _ = blockHeightChan, errEpochChan + return err +} + +// Mock returns should pack args.Error(2) when it fits. +func mockReturn(mock Args) (chan *chainntnfs.TxConfirmation, chan error, error) { + args := m.Called(ctx, txid, pkScript, numConfs, heightHint) + + return args.Get(0).(chan *chainntnfs.TxConfirmation), args.Get(1).(chan error), args.Error( + 2, + ) +} + func someFunction(arg1, arg2, arg3, arg4 string) string { return "" } diff --git a/testdata/multiline/output_next.go b/testdata/multiline/output_next.go index 889ee15..0218fad 100644 --- a/testdata/multiline/output_next.go +++ b/testdata/multiline/output_next.go @@ -184,6 +184,39 @@ func main() { chain2().Then("ok").Limit(100) } +// Simple nested calls should not be over-expanded. +func simpleNestedCalls(row Row, quote Quote) error { + maxPrepayRoutingFee := getMaxRoutingFee( + btcutil.Amount(quote.PrepayAmtSat), + ) + _ = maxPrepayRoutingFee + + err := finalizedHtlcTx.Deserialize( + bytes.NewReader(row.FinalizedHtlcTx), + ) + + return err +} + +// A simple notification argument should stay with the call. +func simpleNotifier(notifCtx context.Context) error { + blockHeightChan, errEpochChan, err := f.cfg.ChainNotifier. + RegisterBlockEpochNtfn(notifCtx) + _, _ = blockHeightChan, errEpochChan + + return err +} + +// Mock returns should pack args.Error(2) when it fits. +func mockReturn(mock Args) (chan *chainntnfs.TxConfirmation, + chan error, error) { + + args := m.Called(ctx, txid, pkScript, numConfs, heightHint) + + return args.Get(0).(chan *chainntnfs.TxConfirmation), + args.Get(1).(chan error), args.Error(2) +} + func someFunction(arg1, arg2, arg3, arg4 string) string { return "" } diff --git a/testdata/signatures/input.go b/testdata/signatures/input.go index f0f5656..2fcf3e1 100644 --- a/testdata/signatures/input.go +++ b/testdata/signatures/input.go @@ -116,7 +116,64 @@ func getComprehensiveSystemStatus() (systemHealth SystemHealthStatus, resourceUt return } +// Example 21: Method with one short parameter and short returns +func (c *TapdClient) GetAssetName(ctx context.Context, assetId []byte) (string, error) { + return "", nil +} + +// Example 22: Interface method with a long row slice return +type InstantOutStore interface { + GetInstantOutSwaps(ctx context.Context) ([]sqlc.GetInstantOutSwapsRow, error) +} + +// Example 23: Function literal assigned through a long selector prefix +func configureManager(manager *ServiceManager) { + manager.reallyLongNestedConfig.FetchLiquidityParams = func(context.Context) ([]byte, error) { + return nil, nil + } +} + +// Example 24: Reservation subscription helper with channel parameters +func (f *ReservationFSM) handleSubcriptions(ctx context.Context, blockHeightChan <-chan int32, spendChan <-chan *chainntnfs.SpendDetail, errEpochChan <-chan error, errSpendChan <-chan error) (fsm.EventType, error) { + return fsm.NoOp, nil +} + +// Example 25: Function with one named parameter and long returns +func rpcRouteCancel( + details *outCancelDetails) ( + *veryLongRouteCancelResponseWithAdditionalSuffix, error) { + + return nil, nil +} + +// Example 26: Single-line input version of rpcRouteCancel should produce the same result. +func rpcRouteCancel2(details *outCancelDetails) (*veryLongRouteCancelResponseWithAdditionalSuffix, error) { + return nil, nil +} + +// Example 27: Multiline params with inline returns should remain unchanged +func unmarshalFixedPoint( + fp *looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, error) { + + return nil, nil +} + +// Example 28: Partial return split should move the parameter down +func unmarshalFixedPoint2(fp *looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, + error) { + + return nil, nil +} + +// Example 29: Partial return split should move the parameter down +func unmarshalFixedPoint3(fp *looprpc.FixedPoint) ( + *rfqmath.BigIntFixedPoint, error) { + + return nil, nil +} + // Stub types to make the file compile +type TapdClient struct{} type Server struct{} type Request struct{} type Options struct{} @@ -175,5 +232,10 @@ type ResourceUtilizationMetrics struct{} type ConnectionPoolStats struct{} type OperationQueueStats struct{} type ErrorRateMetrics struct{} +type ServiceManager struct{} +type ReservationFSM struct{} +type outCancelDetails struct{} +type routeCancelResp struct{} +type veryLongRouteCancelResponseWithAdditionalSuffix struct{} func main() {} diff --git a/testdata/signatures/output_next.go b/testdata/signatures/output_next.go index 47cfa2b..0802f5b 100644 --- a/testdata/signatures/output_next.go +++ b/testdata/signatures/output_next.go @@ -23,8 +23,8 @@ func getUserDetails(ctx context.Context, userID string, includeProfile bool, // Example 3: Long method signature func (s *Server) HandleComplexRequest(ctx context.Context, req *Request, - opts *Options, callback func(error), middleware []Middleware) ( - *Response, error) { + opts *Options, callback func(error), + middleware []Middleware) (*Response, error) { return nil, nil } @@ -62,8 +62,8 @@ type ComplexInterface interface { compressionLevel int) ([]byte, error) ValidateAndTransform(input InputType, rules []ValidationRule, - transformers []Transformer) (OutputType, []ValidationError, - error) + transformers []Transformer) (OutputType, + []ValidationError, error) } // Example 9: Deeply nested function types in parameters @@ -189,7 +189,75 @@ func getComprehensiveSystemStatus() (systemHealth SystemHealthStatus, return } +// Example 21: Method with one short parameter and short returns +func (c *TapdClient) GetAssetName(ctx context.Context, + assetId []byte) (string, error) { + + return "", nil +} + +// Example 22: Interface method with a long row slice return +type InstantOutStore interface { + GetInstantOutSwaps( + ctx context.Context) ([]sqlc.GetInstantOutSwapsRow, error) +} + +// Example 23: Function literal assigned through a long selector prefix +func configureManager(manager *ServiceManager) { + manager.reallyLongNestedConfig.FetchLiquidityParams = func( + context.Context) ([]byte, error) { + + return nil, nil + } +} + +// Example 24: Reservation subscription helper with channel parameters +func (f *ReservationFSM) handleSubcriptions(ctx context.Context, + blockHeightChan <-chan int32, spendChan <-chan *chainntnfs.SpendDetail, + errEpochChan <-chan error, + errSpendChan <-chan error) (fsm.EventType, error) { + + return fsm.NoOp, nil +} + +// Example 25: Function with one named parameter and long returns +func rpcRouteCancel(details *outCancelDetails) ( + *veryLongRouteCancelResponseWithAdditionalSuffix, error) { + + return nil, nil +} + +// Example 26: Single-line input version of rpcRouteCancel should produce the +// same result. +func rpcRouteCancel2(details *outCancelDetails) ( + *veryLongRouteCancelResponseWithAdditionalSuffix, error) { + + return nil, nil +} + +// Example 27: Multiline params with inline returns should remain unchanged +func unmarshalFixedPoint( + fp *looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, error) { + + return nil, nil +} + +// Example 28: Partial return split should move the parameter down +func unmarshalFixedPoint2( + fp *looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, error) { + + return nil, nil +} + +// Example 29: Partial return split should move the parameter down +func unmarshalFixedPoint3( + fp *looprpc.FixedPoint) (*rfqmath.BigIntFixedPoint, error) { + + return nil, nil +} + // Stub types to make the file compile +type TapdClient struct{} type Server struct{} type Request struct{} type Options struct{} @@ -248,5 +316,10 @@ type ResourceUtilizationMetrics struct{} type ConnectionPoolStats struct{} type OperationQueueStats struct{} type ErrorRateMetrics struct{} +type ServiceManager struct{} +type ReservationFSM struct{} +type outCancelDetails struct{} +type routeCancelResp struct{} +type veryLongRouteCancelResponseWithAdditionalSuffix struct{} func main() {} diff --git a/tools/corpus_check/main_test.go b/tools/corpus_check/main_test.go index 4906cd6..9530b4c 100644 --- a/tools/corpus_check/main_test.go +++ b/tools/corpus_check/main_test.go @@ -111,11 +111,9 @@ func TestApplyProfileAdoptionAddsGeneratedExcludes(t *testing.T) { t.Parallel() cfg := reportConfig{ - Profile: "adoption", - ExcludeDirs: defaultExcludeDirs([]string{"custom"}), - ExcludeSuffix: []string{ - ".custom.go", - }, + Profile: "adoption", + ExcludeDirs: defaultExcludeDirs([]string{"custom"}), + ExcludeSuffix: []string{".custom.go"}, } if err := applyProfile(&cfg); err != nil { t.Fatalf("applyProfile returned error: %v", err) @@ -139,11 +137,9 @@ func TestApplyProfileAllKeepsExplicitExcludesOnly(t *testing.T) { t.Parallel() cfg := reportConfig{ - Profile: "all", - ExcludeDirs: defaultExcludeDirs(nil), - ExcludeSuffix: []string{ - ".custom.go", - }, + Profile: "all", + ExcludeDirs: defaultExcludeDirs(nil), + ExcludeSuffix: []string{".custom.go"}, } if err := applyProfile(&cfg); err != nil { t.Fatalf("applyProfile returned error: %v", err) diff --git a/tools/fmt_repo/main.go b/tools/fmt_repo/main.go index f4012dd..7e947ba 100644 --- a/tools/fmt_repo/main.go +++ b/tools/fmt_repo/main.go @@ -268,9 +268,7 @@ func buildArgs(cfg runConfig, path string) []string { "--col", fmt.Sprint(cfg.colLimit), "--tab", fmt.Sprint(cfg.tabStop), "--comments", cfg.commentMode, - "--logcalls-min-tail-len", fmt.Sprint( - cfg.logCallsMinTailLen, - ), + "--logcalls-min-tail-len", fmt.Sprint(cfg.logCallsMinTailLen), "--fixpoint-iters", fmt.Sprint(cfg.fixpointIters), } diff --git a/tools/overflow_report/main_test.go b/tools/overflow_report/main_test.go index 767f763..eb4a5e5 100644 --- a/tools/overflow_report/main_test.go +++ b/tools/overflow_report/main_test.go @@ -21,9 +21,7 @@ func TestGoFilesUnder_ExcludeSuffix(t *testing.T) { t.Fatalf("goFilesUnder returned error: %v", err) } - want := []string{ - filepath.Join(root, "a.go"), - } + want := []string{filepath.Join(root, "a.go")} if !equalStrings(got, want) { t.Fatalf("unexpected files:\n got: %#v\nwant: %#v", got, want) } @@ -44,9 +42,7 @@ func TestGoFilesUnder_ExcludeDir(t *testing.T) { if err != nil { t.Fatalf("goFilesUnder returned error: %v", err) } - want := []string{ - filepath.Join(root, "keep.go"), - } + want := []string{filepath.Join(root, "keep.go")} if !equalStrings(got, want) { t.Fatalf("unexpected files:\n got: %#v\nwant: %#v", got, want) }