From dfa3fb69b2546a51996af517a74b57441c821dd7 Mon Sep 17 00:00:00 2001 From: Monroe Walker Date: Fri, 12 Dec 2025 11:08:34 -0800 Subject: [PATCH 1/3] Fix line splitting for statements with init clauses and nested struct literals This change fixes golines not properly splitting long lines in several cases: 1. If/for/switch statements with initialization clauses - Previously, 'if err := longFunctionCall(...); err != nil' would not split the function call arguments even when the line exceeded max length - Now properly handles st.Init for IfStmt, ForStmt, and SwitchStmt 2. Struct literal elements after parent has been split - When a struct literal was already on its own line but its field initializers were too long, they weren't being split - Added CompositeLit support to HasAnnotationRecursive to detect when elements have annotations - Updated CompositeLit handling to check HasAnnotationRecursive Added test fixtures for all new cases: - if_init.go: if statements with long init clauses - for_init.go: for statements with long init clauses - switch_init.go: switch statements with long init clauses - composite_lit.go: struct literals with long field lines Co-authored-by: Cursor (claude-4.5-opus-high) --- _fixtures/composite_lit.go | 24 ++++++++++++++++ _fixtures/composite_lit__exp.go | 33 ++++++++++++++++++++++ _fixtures/for_init.go | 18 ++++++++++++ _fixtures/for_init__exp.go | 23 +++++++++++++++ _fixtures/if_init.go | 20 +++++++++++++ _fixtures/if_init__exp.go | 25 +++++++++++++++++ _fixtures/switch_init.go | 18 ++++++++++++ _fixtures/switch_init__exp.go | 24 ++++++++++++++++ annotation.go | 6 ++++ shortener.go | 50 ++++++++++++++++++++++++++++++++- 10 files changed, 240 insertions(+), 1 deletion(-) create mode 100644 _fixtures/composite_lit.go create mode 100644 _fixtures/composite_lit__exp.go create mode 100644 _fixtures/for_init.go create mode 100644 _fixtures/for_init__exp.go create mode 100644 _fixtures/if_init.go create mode 100644 _fixtures/if_init__exp.go create mode 100644 _fixtures/switch_init.go create mode 100644 _fixtures/switch_init__exp.go diff --git a/_fixtures/composite_lit.go b/_fixtures/composite_lit.go new file mode 100644 index 0000000..7e6e430 --- /dev/null +++ b/_fixtures/composite_lit.go @@ -0,0 +1,24 @@ +package fixtures + +type Request struct { + Entity string + Fields []string + VersionCheck bool +} + +func update(ctx string, req Request) error { + return nil +} + +func testCompositeLit() { + // Struct literal with long field line that should be split + update( + "ctx", + Request{ + Entity: "enrollment", Fields: []string{"field1", "field2", "field3", "field4"}, VersionCheck: true, + }, + ) + + // Struct literal with nested long slice + update("ctx", Request{Entity: "test", Fields: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}, VersionCheck: false}) +} diff --git a/_fixtures/composite_lit__exp.go b/_fixtures/composite_lit__exp.go new file mode 100644 index 0000000..7f5b3d9 --- /dev/null +++ b/_fixtures/composite_lit__exp.go @@ -0,0 +1,33 @@ +package fixtures + +type Request struct { + Entity string + Fields []string + VersionCheck bool +} + +func update(ctx string, req Request) error { + return nil +} + +func testCompositeLit() { + // Struct literal with long field line that should be split + update( + "ctx", + Request{ + Entity: "enrollment", + Fields: []string{"field1", "field2", "field3", "field4"}, + VersionCheck: true, + }, + ) + + // Struct literal with nested long slice + update( + "ctx", + Request{ + Entity: "test", + Fields: []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}, + VersionCheck: false, + }, + ) +} diff --git a/_fixtures/for_init.go b/_fixtures/for_init.go new file mode 100644 index 0000000..79f946b --- /dev/null +++ b/_fixtures/for_init.go @@ -0,0 +1,18 @@ +package fixtures + +func getValue(a, b, c, d, e string) int { + return 0 +} + +func testForInit() { + // For statement with long init clause + for i := getValue("argument1", "argument2", "argument3", "argument4", "argument5"); i < 10; i++ { + println(i) + } + + // For with long condition + for j := 0; j < getValue("value1", "value2", "value3", "value4", "value5"); j++ { + println(j) + } +} + diff --git a/_fixtures/for_init__exp.go b/_fixtures/for_init__exp.go new file mode 100644 index 0000000..e6e73ce --- /dev/null +++ b/_fixtures/for_init__exp.go @@ -0,0 +1,23 @@ +package fixtures + +func getValue(a, b, c, d, e string) int { + return 0 +} + +func testForInit() { + // For statement with long init clause + for i := getValue( + "argument1", + "argument2", + "argument3", + "argument4", + "argument5", + ); i < 10; i++ { + println(i) + } + + // For with long condition + for j := 0; j < getValue("value1", "value2", "value3", "value4", "value5"); j++ { + println(j) + } +} diff --git a/_fixtures/if_init.go b/_fixtures/if_init.go new file mode 100644 index 0000000..498117d --- /dev/null +++ b/_fixtures/if_init.go @@ -0,0 +1,20 @@ +package fixtures + +func doSomething(a, b, c, d, e string) error { + return nil +} + +func testIfInit() { + // If statement with long init clause + if err := doSomething("argument1", "argument2", "argument3", "argument4", "argument5"); err != nil { + println(err) + } + + // Nested if with long init + if x := doSomething("value1", "value2", "value3", "value4", "value5"); x == nil { + if y := doSomething("nested1", "nested2", "nested3", "nested4", "nested5"); y == nil { + println("both nil") + } + } +} + diff --git a/_fixtures/if_init__exp.go b/_fixtures/if_init__exp.go new file mode 100644 index 0000000..afece17 --- /dev/null +++ b/_fixtures/if_init__exp.go @@ -0,0 +1,25 @@ +package fixtures + +func doSomething(a, b, c, d, e string) error { + return nil +} + +func testIfInit() { + // If statement with long init clause + if err := doSomething( + "argument1", + "argument2", + "argument3", + "argument4", + "argument5", + ); err != nil { + println(err) + } + + // Nested if with long init + if x := doSomething("value1", "value2", "value3", "value4", "value5"); x == nil { + if y := doSomething("nested1", "nested2", "nested3", "nested4", "nested5"); y == nil { + println("both nil") + } + } +} diff --git a/_fixtures/switch_init.go b/_fixtures/switch_init.go new file mode 100644 index 0000000..baca312 --- /dev/null +++ b/_fixtures/switch_init.go @@ -0,0 +1,18 @@ +package fixtures + +func getType(a, b, c, d, e, f string) int { + return 0 +} + +func testSwitchInit() { + // Switch statement with long init clause + switch v := getType("argument1", "argument2", "argument3", "argument4", "argument5", "argument6"); v { + case 1: + println("one") + case 2: + println("two") + default: + println("other") + } +} + diff --git a/_fixtures/switch_init__exp.go b/_fixtures/switch_init__exp.go new file mode 100644 index 0000000..6a9b278 --- /dev/null +++ b/_fixtures/switch_init__exp.go @@ -0,0 +1,24 @@ +package fixtures + +func getType(a, b, c, d, e, f string) int { + return 0 +} + +func testSwitchInit() { + // Switch statement with long init clause + switch v := getType( + "argument1", + "argument2", + "argument3", + "argument4", + "argument5", + "argument6", + ); v { + case 1: + println("one") + case 2: + println("two") + default: + println("other") + } +} diff --git a/annotation.go b/annotation.go index 0790a30..96e2ad2 100644 --- a/annotation.go +++ b/annotation.go @@ -79,6 +79,12 @@ func HasAnnotationRecursive(node dst.Node) bool { return true } } + case *dst.CompositeLit: + for _, elt := range n.Elts { + if HasAnnotation(elt) { + return true + } + } } return false diff --git a/shortener.go b/shortener.go index 4e8d9f3..6628e45 100644 --- a/shortener.go +++ b/shortener.go @@ -454,12 +454,43 @@ func (s *Shortener) formatStmt(stmt dst.Stmt) { case *dst.ExprStmt: s.formatExpr(st.X, shouldShorten, false) case *dst.ForStmt: + if st.Init != nil { + s.formatStmt(st.Init) + if shouldShorten { + if assignStmt, ok := st.Init.(*dst.AssignStmt); ok { + for _, expr := range assignStmt.Rhs { + s.formatExpr(expr, true, false) + } + } + } + } + if st.Cond != nil { + s.formatExpr(st.Cond, shouldShorten, false) + } + if st.Post != nil { + s.formatStmt(st.Post) + } s.formatStmt(st.Body) case *dst.GoStmt: s.formatExpr(st.Call, shouldShorten, false) case *dst.IfStmt: + if st.Init != nil { + s.formatStmt(st.Init) + // If the init statement is an assignment containing a call expression that + // should be shortened, mark it for shortening + if shouldShorten { + if assignStmt, ok := st.Init.(*dst.AssignStmt); ok { + for _, expr := range assignStmt.Rhs { + s.formatExpr(expr, true, false) + } + } + } + } s.formatExpr(st.Cond, shouldShorten, false) s.formatStmt(st.Body) + if st.Else != nil { + s.formatStmt(st.Else) + } case *dst.RangeStmt: s.formatStmt(st.Body) case *dst.ReturnStmt: @@ -469,6 +500,19 @@ func (s *Shortener) formatStmt(stmt dst.Stmt) { case *dst.SelectStmt: s.formatStmt(st.Body) case *dst.SwitchStmt: + if st.Init != nil { + s.formatStmt(st.Init) + if shouldShorten { + if assignStmt, ok := st.Init.(*dst.AssignStmt); ok { + for _, expr := range assignStmt.Rhs { + s.formatExpr(expr, true, false) + } + } + } + } + if st.Tag != nil { + s.formatExpr(st.Tag, shouldShorten, false) + } s.formatStmt(st.Body) default: if shouldShorten { @@ -528,7 +572,11 @@ func (s *Shortener) formatExpr(expr dst.Expr, force bool, isChain bool) { s.formatExpr(e.Fun, shouldShorten, isChain) } case *dst.CompositeLit: - if shouldShorten { + // Check if we should shorten - either because the parent told us to, + // or because one of the elements has an annotation (meaning the line + // with the elements is too long) + shortenElements := shouldShorten || HasAnnotationRecursive(e) + if shortenElements { for i, element := range e.Elts { if i == 0 { element.Decorations().Before = dst.NewLine From 628576e12eac0da6fce334631811c0d6487613fa Mon Sep 17 00:00:00 2001 From: Monroe Walker Date: Fri, 12 Dec 2025 17:15:39 -0800 Subject: [PATCH 2/3] Fix line splitting for inline function literals with long bodies Single-line function literals like 'func(a, b T) int { return longExpr }' were not being expanded when the line exceeded max length. Now expands the body statements onto separate lines when needed: func(a, b T) int { return longExpr } Added test fixture: func_lit_body.go Co-authored-by: Cursor (claude-4.5-opus-high) --- _fixtures/func_lit_body.go | 24 ++++++++++++++++++++++++ _fixtures/func_lit_body__exp.go | 25 +++++++++++++++++++++++++ shortener.go | 10 ++++++++++ 3 files changed, 59 insertions(+) create mode 100644 _fixtures/func_lit_body.go create mode 100644 _fixtures/func_lit_body__exp.go diff --git a/_fixtures/func_lit_body.go b/_fixtures/func_lit_body.go new file mode 100644 index 0000000..aa940e5 --- /dev/null +++ b/_fixtures/func_lit_body.go @@ -0,0 +1,24 @@ +package fixtures + +type LongTypeName struct { + Value int +} + +func (l LongTypeName) Compare(other LongTypeName) int { + return l.Value - other.Value +} + +func SortFunc[T any](collection []T, fn func(T, T) int) {} + +func testFuncLitBody() { + // Function literal with long body that should be expanded + items := []LongTypeName{} + SortFunc( + items, + func(a, b LongTypeName) int { return a.Compare(b) + a.Compare(b) + a.Compare(b) + a.Compare(b) }, + ) + + // Single line function literal that fits - should stay as is + SortFunc(items, func(a, b LongTypeName) int { return a.Compare(b) }) +} + diff --git a/_fixtures/func_lit_body__exp.go b/_fixtures/func_lit_body__exp.go new file mode 100644 index 0000000..d3be176 --- /dev/null +++ b/_fixtures/func_lit_body__exp.go @@ -0,0 +1,25 @@ +package fixtures + +type LongTypeName struct { + Value int +} + +func (l LongTypeName) Compare(other LongTypeName) int { + return l.Value - other.Value +} + +func SortFunc[T any](collection []T, fn func(T, T) int) {} + +func testFuncLitBody() { + // Function literal with long body that should be expanded + items := []LongTypeName{} + SortFunc( + items, + func(a, b LongTypeName) int { + return a.Compare(b) + a.Compare(b) + a.Compare(b) + a.Compare(b) + }, + ) + + // Single line function literal that fits - should stay as is + SortFunc(items, func(a, b LongTypeName) int { return a.Compare(b) }) +} diff --git a/shortener.go b/shortener.go index 6628e45..6017eee 100644 --- a/shortener.go +++ b/shortener.go @@ -589,6 +589,16 @@ func (s *Shortener) formatExpr(expr dst.Expr, force bool, isChain bool) { s.formatExpr(element, false, isChain) } case *dst.FuncLit: + // If the function literal line is too long, expand the body + if shouldShorten && e.Body != nil && len(e.Body.List) > 0 { + // Put each statement in the body on its own line + for i, stmt := range e.Body.List { + if i == 0 { + stmt.Decorations().Before = dst.NewLine + } + stmt.Decorations().After = dst.NewLine + } + } s.formatStmt(e.Body) case *dst.FuncType: if shouldShorten { From 86dc58eb8292b8aa81297ca0f9d1b0581695b7b0 Mon Sep 17 00:00:00 2001 From: Monroe Walker Date: Fri, 12 Dec 2025 17:43:57 -0800 Subject: [PATCH 3/3] Fix line splitting for arguments in chained method calls When a chained method call line is still too long after splitting at dots, the arguments within that call are now also split onto separate lines. Previously, arguments in chained calls were always processed with shouldShorten=false, meaning they would never be split even when the line exceeded max length. The fix: - Continue processing in annotateLongLines even when line length hasn't decreased (was causing premature stop) - Check for annotations on the SelectorExpr and method Ident, not just the CallExpr (DST attaches mid-chain comments to the Ident) - Split arguments when the specific method call is annotated as too long Added test fixture: chained_args.go Updated: chained_calls__exp.go (now correctly splits more long lines) Co-authored-by: Cursor (claude-4.5-opus-high) --- _fixtures/chained_args.go | 22 ++++++++++++++++++++++ _fixtures/chained_args__exp.go | 21 +++++++++++++++++++++ _fixtures/chained_calls__exp.go | 11 +++++++++-- shortener.go | 21 ++++++++++++++++++--- 4 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 _fixtures/chained_args.go create mode 100644 _fixtures/chained_args__exp.go diff --git a/_fixtures/chained_args.go b/_fixtures/chained_args.go new file mode 100644 index 0000000..4975291 --- /dev/null +++ b/_fixtures/chained_args.go @@ -0,0 +1,22 @@ +package fixtures + +import "strings" + +type Logger struct{} + +func (l Logger) Str(k, v string) Logger { return l } +func (l Logger) Msgf(format string, args ...interface{}) {} + +func Map[T, U any](collection []T, fn func(T, int) U) []U { + return nil +} + +func testChainedArgs() { + messages := []string{} + l := Logger{} + + // Long argument inside a chained method call - should split the arguments + l.Str("key", strings.Join(Map(messages, func(message string, _ int) string { return message + message + message }), ",")). + Msgf("Message with format %s", "arg") +} + diff --git a/_fixtures/chained_args__exp.go b/_fixtures/chained_args__exp.go new file mode 100644 index 0000000..91e120f --- /dev/null +++ b/_fixtures/chained_args__exp.go @@ -0,0 +1,21 @@ +package fixtures + +import "strings" + +type Logger struct{} + +func (l Logger) Str(k, v string) Logger { return l } +func (l Logger) Msgf(format string, args ...interface{}) {} + +func Map[T, U any](collection []T, fn func(T, int) U) []U { + return nil +} + +func testChainedArgs() { + messages := []string{} + l := Logger{} + + // Long argument inside a chained method call - should split the arguments + l.Str("key", strings.Join(Map(messages, func(message string, _ int) string { return message + message + message }), ",")). + Msgf("Message with format %s", "arg") +} diff --git a/_fixtures/chained_calls__exp.go b/_fixtures/chained_calls__exp.go index 7fa5d04..84026e5 100644 --- a/_fixtures/chained_calls__exp.go +++ b/_fixtures/chained_calls__exp.go @@ -20,9 +20,16 @@ func ChainedCalls() { NewChain().ChainCall( "a really really really really really long argument4", "another really really really really really long argument4", - fmt.Sprintf("%v", "this is a long method"), + fmt.Sprintf( + "%v", + "this is a long method", + ), ). - ChainCall("a really really really really really long argument5", "another really really really really really long argument5", "a third really really really really really long argument5"). + ChainCall( + "a really really really really really long argument5", + "another really really really really really long argument5", + "a third really really really really really long argument5", + ). ChainCall("a", "b", fmt.Sprintf("%v", "this is a long method")) NewChain().ChainCall("a", "b", "c").ChainCall("d", "e", "f") NewChain().ChainCall("a", "b", "c"). diff --git a/shortener.go b/shortener.go index 6017eee..9bca03d 100644 --- a/shortener.go +++ b/shortener.go @@ -241,6 +241,10 @@ func (s *Shortener) annotateLongLines(lines []string) ([]string, int) { // Replace annotation with new length annotatedLines[len(annotatedLines)-1] = CreateAnnotation(length) linesToShorten++ + } else { + // Line is still too long with no progress, but keep trying + // (maxRounds will prevent infinite loops) + linesToShorten++ } } else if !s.isComment(line) && length > s.config.MaxLen { annotatedLines = append( @@ -542,7 +546,7 @@ func (s *Shortener) formatExpr(expr dst.Expr, force bool, isChain bool) { s.formatExpr(e.Y, shouldShorten, isChain) } case *dst.CallExpr: - _, ok := e.Fun.(*dst.SelectorExpr) + selectorExpr, ok := e.Fun.(*dst.SelectorExpr) if ok && s.config.ChainSplitDots && @@ -550,8 +554,19 @@ func (s *Shortener) formatExpr(expr dst.Expr, force bool, isChain bool) { (isChain || s.chainLength(e) > 1) { e.Decorations().After = dst.NewLine - for _, arg := range e.Args { - s.formatExpr(arg, false, true) + // If this specific call is annotated (line still too long after chain split), + // also split the arguments. The annotation may be on the CallExpr, the + // SelectorExpr, or the method name Ident. + shortenArgs := HasAnnotation(e) || HasAnnotation(selectorExpr) || + HasAnnotation(selectorExpr.Sel) + for a, arg := range e.Args { + if shortenArgs { + if a == 0 { + arg.Decorations().Before = dst.NewLine + } + arg.Decorations().After = dst.NewLine + } + s.formatExpr(arg, shortenArgs, true) } s.formatExpr(e.Fun, shouldShorten, true)