From f1fdb78b3946977f5e320b34578e37e545f49d4d Mon Sep 17 00:00:00 2001 From: Knut Wannheden Date: Fri, 29 May 2026 18:56:24 +0200 Subject: [PATCH] Go: model `for ... range` as a rich Go.RangeLoop instead of cramming it into J.ForEachLoop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go's range loop binds a key and an optional value joined by `:=`/`=`, which cannot fit Java's single-variable J.ForEachLoop.Control. The Go side had defined its own fatter `java.ForEachControl` (Key/Value/Operator/Iterable) masquerading under the Java type name and bridged it to the real J.ForEachLoop.Control via a custom RPC codec — but that codec truncated to a single synthesized variable, dropping the value and the `:=`/`=` distinction. Under a recipe (full deserialization) `for k, v := range x` came back as `for_:=range x`. Introduce a dedicated Go.RangeLoop node (key?, value?, operator, iterable, body) on both sides, mirroring the per-language loop-node pattern (JS ForOfLoop/ForInLoop). Remove the lossy ForEachLoop/ForEachControl cheat entirely: the Go-customized java.ForEachLoop/ForEachControl structs, their RPC send/receive, the GolangSender/GolangReceiver bridge overrides, and the J$ForEachLoop wire types are all gone. Adds range round-trip coverage (key+value, key-only, no-variable, `=` assign) to GoRpcPrintRoundTripIntegTest. --- rewrite-go/pkg/parser/go_parser.go | 29 ++-- rewrite-go/pkg/printer/go_printer.go | 36 ++--- rewrite-go/pkg/rpc/go_receiver.go | 37 +++++ rewrite-go/pkg/rpc/go_sender.go | 32 ++++ rewrite-go/pkg/rpc/java_receiver.go | 51 ------- rewrite-go/pkg/rpc/java_sender.go | 39 ----- rewrite-go/pkg/rpc/value_types.go | 6 +- rewrite-go/pkg/template/comparator.go | 13 +- rewrite-go/pkg/template/go_template.go | 2 +- rewrite-go/pkg/template/substitution.go | 2 +- rewrite-go/pkg/tree/golang/go.go | 30 ++++ rewrite-go/pkg/tree/golang/go_methods.go | 12 ++ rewrite-go/pkg/tree/java/j.go | 65 +------- rewrite-go/pkg/tree/java/j_methods.go | 24 --- rewrite-go/pkg/visitor/go_visitor.go | 43 +++--- .../rpc/GoRpcPrintRoundTripIntegTest.java | 42 +++++ .../org/openrewrite/golang/GolangVisitor.java | 16 ++ .../golang/internal/rpc/GolangReceiver.java | 59 ++----- .../golang/internal/rpc/GolangSender.java | 35 ++--- .../java/org/openrewrite/golang/tree/Go.java | 144 ++++++++++++++++++ 20 files changed, 396 insertions(+), 321 deletions(-) diff --git a/rewrite-go/pkg/parser/go_parser.go b/rewrite-go/pkg/parser/go_parser.go index 52b03cdab1..84f7cdb095 100644 --- a/rewrite-go/pkg/parser/go_parser.go +++ b/rewrite-go/pkg/parser/go_parser.go @@ -1546,10 +1546,10 @@ func (ctx *parseContext) mapForStmt(stmt *ast.ForStmt) *java.ForLoop { } // mapRangeStmt maps a for-range statement. -func (ctx *parseContext) mapRangeStmt(stmt *ast.RangeStmt) *java.ForEachLoop { +func (ctx *parseContext) mapRangeStmt(stmt *ast.RangeStmt) *golang.RangeLoop { prefix := ctx.prefixAndSkip(stmt.Pos(), len("for")) - control := java.ForEachControl{ID: uuid.New()} + loop := &golang.RangeLoop{ID: uuid.New(), Prefix: prefix} if stmt.Key != nil { // Has key variable @@ -1565,18 +1565,18 @@ func (ctx *parseContext) mapRangeStmt(stmt *ast.RangeStmt) *java.ForEachLoop { ctx.skip(1) // "," } keyRP := java.RightPadded[java.Expression]{Element: key, After: keyAfter} - control.Key = &keyRP + loop.Key = &keyRP value := ctx.mapExpr(stmt.Value) // Value.After captures space before operator opPrefix := ctx.prefix(stmt.TokPos) valueRP := java.RightPadded[java.Expression]{Element: value, After: opPrefix} - control.Value = &valueRP + loop.Value = &valueRP } else { // for k := range expr {} — no value opPrefix := ctx.prefix(stmt.TokPos) keyRP := java.RightPadded[java.Expression]{Element: key, After: opPrefix} - control.Key = &keyRP + loop.Key = &keyRP } // Parse operator (:= or =) @@ -1590,24 +1590,17 @@ func (ctx *parseContext) mapRangeStmt(stmt *ast.RangeStmt) *java.ForEachLoop { // Space between operator and "range" rangePrefix := ctx.prefix(stmt.Range) - control.Operator = java.LeftPadded[java.AssignOp]{Before: rangePrefix, Element: op} + loop.Operator = java.LeftPadded[java.AssignOp]{Before: rangePrefix, Element: op} } else { - // for range expr {} — no variable - control.Prefix = ctx.prefix(stmt.Range) + // for range expr {} — no variable; the space before `range` lives in Operator.Before + loop.Operator = java.LeftPadded[java.AssignOp]{Before: ctx.prefix(stmt.Range)} } ctx.skip(len("range")) - iterable := ctx.mapExpr(stmt.X) - control.Iterable = iterable + loop.Iterable = ctx.mapExpr(stmt.X) + loop.Body = java.RightPadded[java.Statement]{Element: ctx.mapBlockStmt(stmt.Body), After: java.EmptySpace} - body := ctx.mapBlockStmt(stmt.Body) - - return &java.ForEachLoop{ - ID: uuid.New(), - Prefix: prefix, - Control: control, - Body: body, - } + return loop } // mapIncDecStmt maps an increment/decrement statement (x++ or x--). diff --git a/rewrite-go/pkg/printer/go_printer.go b/rewrite-go/pkg/printer/go_printer.go index 74a87ff652..0dbe2824ea 100644 --- a/rewrite-go/pkg/printer/go_printer.go +++ b/rewrite-go/pkg/printer/go_printer.go @@ -553,38 +553,30 @@ func (p *GoPrinter) VisitForControl(control *java.ForControl, param any) java.J return control } -func (p *GoPrinter) VisitForEachLoop(forEach *java.ForEachLoop, param any) java.J { +func (p *GoPrinter) VisitRangeLoop(loop *golang.RangeLoop, param any) java.J { out := param.(*PrintOutputCapture) - p.beforeSyntax(forEach.Prefix, forEach.Markers, out) + p.beforeSyntax(loop.Prefix, loop.Markers, out) out.Append("for") - p.Visit(&forEach.Control, out) - p.Visit(forEach.Body, out) - p.afterSyntax(forEach.Markers, out) - return forEach -} - -func (p *GoPrinter) VisitForEachControl(control *java.ForEachControl, param any) java.J { - out := param.(*PrintOutputCapture) - p.beforeSyntax(control.Prefix, control.Markers, out) - if control.Key != nil { - p.Visit(control.Key.Element, out) - p.visitSpace(control.Key.After, out) - if control.Value != nil { + if loop.Key != nil { + p.Visit(loop.Key.Element, out) + p.visitSpace(loop.Key.After, out) + if loop.Value != nil { out.Append(",") - p.Visit(control.Value.Element, out) - p.visitSpace(control.Value.After, out) + p.Visit(loop.Value.Element, out) + p.visitSpace(loop.Value.After, out) } - if control.Operator.Element == java.AssignOpDefine { + if loop.Operator.Element == java.AssignOpDefine { out.Append(":=") } else { out.Append("=") } - p.visitSpace(control.Operator.Before, out) } + p.visitSpace(loop.Operator.Before, out) out.Append("range") - p.Visit(control.Iterable, out) - p.afterSyntax(control.Markers, out) - return control + p.Visit(loop.Iterable, out) + p.Visit(loop.Body.Element, out) + p.afterSyntax(loop.Markers, out) + return loop } // VisitAnnotation prints an Annotation in struct-tag form diff --git a/rewrite-go/pkg/rpc/go_receiver.go b/rewrite-go/pkg/rpc/go_receiver.go index 770f293682..4992fc5b29 100644 --- a/rewrite-go/pkg/rpc/go_receiver.go +++ b/rewrite-go/pkg/rpc/go_receiver.go @@ -210,6 +210,43 @@ func (r *GoReceiver) VisitGoVariadic(vr *golang.Variadic, p any) java.J { return vr } +func (r *GoReceiver) VisitRangeLoop(l *golang.RangeLoop, p any) java.J { + q := p.(*ReceiveQueue) + c := *l // shallow copy to avoid mutating remoteObjects baseline + l = &c + // key (right-padded, nullable) + var beforeKey any + if l.Key != nil { + beforeKey = *l.Key + } + if result := q.Receive(beforeKey, func(v any) any { return receiveRightPadded(r, q, v) }); result != nil { + rp := coerceToExpressionRP(result) + l.Key = &rp + } else { + l.Key = nil + } + // value (right-padded, nullable) + var beforeValue any + if l.Value != nil { + beforeValue = *l.Value + } + if result := q.Receive(beforeValue, func(v any) any { return receiveRightPadded(r, q, v) }); result != nil { + rp := coerceToExpressionRP(result) + l.Value = &rp + } else { + l.Value = nil + } + // operator (left-padded AssignOp enum) + l.Operator = receiveLeftPaddedEnum(r, q, l.Operator, parseAssignOpDefaulting) + // iterable + l.Iterable = receiveValue(q, l.Iterable, func(e java.Expression) any { return r.Visit(e, q) }) + // body (right-padded Statement) + if result := q.Receive(l.Body, func(v any) any { return receiveRightPadded(r, q, v) }); result != nil { + l.Body = coerceToStatementRP(result) + } + return l +} + // VisitFallthrough mirrors GolangReceiver.visitFallthrough — the node has no // payload beyond the framework-handled id/prefix/markers, so this override // is intentionally a no-op. Present for sender/receiver symmetry. diff --git a/rewrite-go/pkg/rpc/go_sender.go b/rewrite-go/pkg/rpc/go_sender.go index 6bc02d5197..1302097d0f 100644 --- a/rewrite-go/pkg/rpc/go_sender.go +++ b/rewrite-go/pkg/rpc/go_sender.go @@ -183,6 +183,38 @@ func (s *GoSender) VisitGoVariadic(vr *golang.Variadic, p any) java.J { return vr } +func (s *GoSender) VisitRangeLoop(l *golang.RangeLoop, p any) java.J { + q := p.(*SendQueue) + // key (right-padded, nullable) + q.GetAndSend(l, func(v any) any { + k := v.(*golang.RangeLoop).Key + if k == nil { + return nil + } + return *k + }, func(v any) { sendRightPadded(s, v, q) }) + // value (right-padded, nullable) + q.GetAndSend(l, func(v any) any { + val := v.(*golang.RangeLoop).Value + if val == nil { + return nil + } + return *val + }, func(v any) { sendRightPadded(s, v, q) }) + // operator (left-padded AssignOp, sent as faithful enum-constant name) + q.GetAndSend(l, func(v any) any { + op := v.(*golang.RangeLoop).Operator + return java.LeftPadded[string]{Before: op.Before, Element: op.Element.String(), Markers: op.Markers} + }, func(v any) { sendLeftPadded(s, v, q) }) + // iterable + q.GetAndSend(l, func(v any) any { return v.(*golang.RangeLoop).Iterable }, + func(v any) { s.Visit(v.(java.Tree), q) }) + // body (right-padded Statement) + q.GetAndSend(l, func(v any) any { return v.(*golang.RangeLoop).Body }, + func(v any) { sendRightPadded(s, v, q) }) + return l +} + // VisitFallthrough mirrors GolangSender.visitFallthrough — the node has no // payload beyond the framework-handled id/prefix/markers, so this override // is intentionally a no-op. Present for sender/receiver symmetry. diff --git a/rewrite-go/pkg/rpc/java_receiver.go b/rewrite-go/pkg/rpc/java_receiver.go index f492fdf9e3..dfd6053760 100644 --- a/rewrite-go/pkg/rpc/java_receiver.go +++ b/rewrite-go/pkg/rpc/java_receiver.go @@ -552,57 +552,6 @@ func (r *JavaReceiver) VisitForControl(fc *java.ForControl, p any) java.J { return fc } -func (r *JavaReceiver) VisitForEachLoop(f *java.ForEachLoop, p any) java.J { - q := p.(*ReceiveQueue) - c := *f // shallow copy to avoid mutating remoteObjects baseline - f = &c - ctrl := &f.Control - if result := q.Receive(ctrl, func(v any) any { return r.Visit(v.(java.Tree), q) }); result != nil { - f.Control = *result.(*java.ForEachControl) - } - // body - Java sends RightPadded wrapping the Block - if bodyResult := q.Receive(nil, func(v any) any { return receiveRightPadded(r, q, v) }); bodyResult != nil { - rp := coerceToStatementRP(bodyResult) - if blk, ok := rp.Element.(*java.Block); ok { - f.Body = blk - } - } - return f -} - -func (r *JavaReceiver) VisitForEachControl(fc *java.ForEachControl, p any) java.J { - q := p.(*ReceiveQueue) - c := *fc // shallow copy to avoid mutating remoteObjects baseline - fc = &c - // key (right-padded, nullable) - var beforeKey any - if fc.Key != nil { - beforeKey = *fc.Key - } - if result := q.Receive(beforeKey, func(v any) any { return receiveRightPadded(r, q, v) }); result != nil { - rp := coerceToExpressionRP(result) - fc.Key = &rp - } else { - fc.Key = nil - } - // value (right-padded, nullable) - var beforeValue any - if fc.Value != nil { - beforeValue = *fc.Value - } - if result := q.Receive(beforeValue, func(v any) any { return receiveRightPadded(r, q, v) }); result != nil { - rp := coerceToExpressionRP(result) - fc.Value = &rp - } else { - fc.Value = nil - } - // operator (left-padded AssignOp enum) - fc.Operator = receiveLeftPaddedEnum(r, q, fc.Operator, parseAssignOpDefaulting) - // iterable - fc.Iterable = receiveValue(q, fc.Iterable, func(e java.Expression) any { return r.Visit(e, q) }) - return fc -} - func (r *JavaReceiver) VisitSwitch(sw *java.Switch, p any) java.J { q := p.(*ReceiveQueue) c := *sw // shallow copy to avoid mutating remoteObjects baseline diff --git a/rewrite-go/pkg/rpc/java_sender.go b/rewrite-go/pkg/rpc/java_sender.go index 9a9c5b29ff..5b212909ea 100644 --- a/rewrite-go/pkg/rpc/java_sender.go +++ b/rewrite-go/pkg/rpc/java_sender.go @@ -386,45 +386,6 @@ func (s *JavaSender) VisitForControl(fc *java.ForControl, p any) java.J { return fc } -func (s *JavaSender) VisitForEachLoop(f *java.ForEachLoop, p any) java.J { - q := p.(*SendQueue) - q.GetAndSend(f, func(v any) any { - ctrl := v.(*java.ForEachLoop).Control - return &ctrl - }, func(v any) { s.Visit(v.(java.Tree), q) }) - q.GetAndSend(f, func(v any) any { - return java.RightPadded[java.Statement]{Element: v.(*java.ForEachLoop).Body, After: java.EmptySpace} - }, func(v any) { sendRightPadded(s, v, q) }) - return f -} - -func (s *JavaSender) VisitForEachControl(fc *java.ForEachControl, p any) java.J { - q := p.(*SendQueue) - // Go sends: key (right-padded), value (right-padded), operator (left-padded string), iterable - // Java GolangReceiver override reads this format - q.GetAndSend(fc, func(v any) any { - k := v.(*java.ForEachControl).Key - if k == nil { - return nil - } - return *k - }, func(v any) { sendRightPadded(s, v, q) }) - q.GetAndSend(fc, func(v any) any { - val := v.(*java.ForEachControl).Value - if val == nil { - return nil - } - return *val - }, func(v any) { sendRightPadded(s, v, q) }) - q.GetAndSend(fc, func(v any) any { - op := v.(*java.ForEachControl).Operator - return java.LeftPadded[string]{Before: op.Before, Element: op.Element.String(), Markers: op.Markers} - }, func(v any) { sendLeftPadded(s, v, q) }) - q.GetAndSend(fc, func(v any) any { return v.(*java.ForEachControl).Iterable }, - func(v any) { s.Visit(v.(java.Tree), q) }) - return fc -} - func (s *JavaSender) VisitSwitch(sw *java.Switch, p any) java.J { q := p.(*SendQueue) // selector - wrap tag in ControlParentheses for Java's J.Switch model diff --git a/rewrite-go/pkg/rpc/value_types.go b/rewrite-go/pkg/rpc/value_types.go index 825088bba4..2e06b78e1c 100644 --- a/rewrite-go/pkg/rpc/value_types.go +++ b/rewrite-go/pkg/rpc/value_types.go @@ -68,8 +68,7 @@ func init() { RegisterValueType(reflect.TypeOf((*java.MethodDeclaration)(nil)), "org.openrewrite.java.tree.J$MethodDeclaration") RegisterValueType(reflect.TypeOf((*java.ForLoop)(nil)), "org.openrewrite.java.tree.J$ForLoop") RegisterValueType(reflect.TypeOf((*java.ForControl)(nil)), "org.openrewrite.java.tree.J$ForLoop$Control") - RegisterValueType(reflect.TypeOf((*java.ForEachLoop)(nil)), "org.openrewrite.java.tree.J$ForEachLoop") - RegisterValueType(reflect.TypeOf((*java.ForEachControl)(nil)), "org.openrewrite.java.tree.J$ForEachLoop$Control") + RegisterValueType(reflect.TypeOf((*golang.RangeLoop)(nil)), "org.openrewrite.golang.tree.Go$RangeLoop") RegisterValueType(reflect.TypeOf((*java.Switch)(nil)), "org.openrewrite.java.tree.J$Switch") RegisterValueType(reflect.TypeOf((*java.Case)(nil)), "org.openrewrite.java.tree.J$Case") RegisterValueType(reflect.TypeOf((*java.Break)(nil)), "org.openrewrite.java.tree.J$Break") @@ -186,8 +185,7 @@ func init() { RegisterFactory("org.openrewrite.java.tree.J$MethodDeclaration", func() any { return &java.MethodDeclaration{ID: uuid.New()} }) RegisterFactory("org.openrewrite.java.tree.J$ForLoop", func() any { return &java.ForLoop{ID: uuid.New()} }) RegisterFactory("org.openrewrite.java.tree.J$ForLoop$Control", func() any { return &java.ForControl{ID: uuid.New()} }) - RegisterFactory("org.openrewrite.java.tree.J$ForEachLoop", func() any { return &java.ForEachLoop{ID: uuid.New()} }) - RegisterFactory("org.openrewrite.java.tree.J$ForEachLoop$Control", func() any { return &java.ForEachControl{ID: uuid.New()} }) + RegisterFactory("org.openrewrite.golang.tree.Go$RangeLoop", func() any { return &golang.RangeLoop{ID: uuid.New()} }) RegisterFactory("org.openrewrite.java.tree.J$Switch", func() any { return &java.Switch{ID: uuid.New()} }) RegisterFactory("org.openrewrite.java.tree.J$Case", func() any { return &java.Case{ID: uuid.New()} }) RegisterFactory("org.openrewrite.java.tree.J$Break", func() any { return &java.Break{ID: uuid.New()} }) diff --git a/rewrite-go/pkg/template/comparator.go b/rewrite-go/pkg/template/comparator.go index ca5e8583d8..a015400c7f 100644 --- a/rewrite-go/pkg/template/comparator.go +++ b/rewrite-go/pkg/template/comparator.go @@ -219,19 +219,18 @@ func (c *patternComparator) matchProperties(pattern, candidate java.J) bool { return false } return c.matchOptionalRightPaddedStmt(p.Update, cand.Update) - case *java.ForEachLoop: - cand := candidate.(*java.ForEachLoop) - return c.matchNode(&p.Control, &cand.Control) && - c.matchNode(p.Body, cand.Body) - case *java.ForEachControl: - cand := candidate.(*java.ForEachControl) + case *golang.RangeLoop: + cand := candidate.(*golang.RangeLoop) if !c.matchOptionalRightPaddedExpr2(p.Key, cand.Key) { return false } if !c.matchOptionalRightPaddedExpr2(p.Value, cand.Value) { return false } - return c.matchNode(p.Iterable, cand.Iterable) + if !c.matchNode(p.Iterable, cand.Iterable) { + return false + } + return c.matchNode(p.Body.Element, cand.Body.Element) case *java.Switch: cand := candidate.(*java.Switch) if !c.matchOptionalRightPaddedStmt(p.Init, cand.Init) { diff --git a/rewrite-go/pkg/template/go_template.go b/rewrite-go/pkg/template/go_template.go index 367979912b..d14946926e 100644 --- a/rewrite-go/pkg/template/go_template.go +++ b/rewrite-go/pkg/template/go_template.go @@ -286,7 +286,7 @@ func getLeadingPrefix(j java.J) java.Space { return n.Prefix case *java.ForLoop: return n.Prefix - case *java.ForEachLoop: + case *golang.RangeLoop: return n.Prefix case *java.Switch: return n.Prefix diff --git a/rewrite-go/pkg/template/substitution.go b/rewrite-go/pkg/template/substitution.go index 70cf292923..49d51931b3 100644 --- a/rewrite-go/pkg/template/substitution.go +++ b/rewrite-go/pkg/template/substitution.go @@ -106,7 +106,7 @@ func setPrefix(j java.J, prefix java.Space) java.J { return n.WithPrefix(prefix) case *java.ForLoop: return n.WithPrefix(prefix) - case *java.ForEachLoop: + case *golang.RangeLoop: return n.WithPrefix(prefix) case *java.Switch: return n.WithPrefix(prefix) diff --git a/rewrite-go/pkg/tree/golang/go.go b/rewrite-go/pkg/tree/golang/go.go index bf00cf041c..c7faeef0d4 100644 --- a/rewrite-go/pkg/tree/golang/go.go +++ b/rewrite-go/pkg/tree/golang/go.go @@ -907,6 +907,36 @@ func (n *Variadic) WithMarkers(markers java.Markers) *Variadic { return &c } +// RangeLoop represents Go's `for ... range` loop: `for k, v := range expr { body }`. +// Unlike Java's single-variable J.ForEachLoop.Control, Go binds a Key and an +// optional Value (or none, for `for range expr`), joined by `:=` or `=`. +type RangeLoop struct { + ID uuid.UUID + Prefix java.Space + Markers java.Markers + Key *java.RightPadded[java.Expression] // nil for `for range expr`; After = space after key (incl. comma) + Value *java.RightPadded[java.Expression] // nil when no value; After = space before operator + Operator java.LeftPadded[java.AssignOp] // `:=` or `=`; Before = space before `range`. Unused when Key is nil + Iterable java.Expression + Body java.RightPadded[java.Statement] // the loop body block +} + +func (*RangeLoop) IsTree() {} +func (*RangeLoop) IsJ() {} +func (*RangeLoop) IsStatement() {} + +func (n *RangeLoop) WithPrefix(prefix java.Space) *RangeLoop { + c := *n + c.Prefix = prefix + return &c +} + +func (n *RangeLoop) WithMarkers(markers java.Markers) *RangeLoop { + c := *n + c.Markers = markers + return &c +} + // SelectStmt is a marker on Switch indicating it's a `select` statement instead of `switch`. type SelectStmt struct { Ident uuid.UUID diff --git a/rewrite-go/pkg/tree/golang/go_methods.go b/rewrite-go/pkg/tree/golang/go_methods.go index 0915571013..6b32acd983 100644 --- a/rewrite-go/pkg/tree/golang/go_methods.go +++ b/rewrite-go/pkg/tree/golang/go_methods.go @@ -250,6 +250,18 @@ func (n *Variadic) WithID(id uuid.UUID) java.J { func (n *Variadic) GetPrefix() java.Space { return n.Prefix } func (n *Variadic) GetMarkers() java.Markers { return n.Markers } +func (n *RangeLoop) GetID() uuid.UUID { return n.ID } +func (n *RangeLoop) WithID(id uuid.UUID) java.J { + if n.ID == id { + return n + } + c := *n + c.ID = id + return &c +} +func (n *RangeLoop) GetPrefix() java.Space { return n.Prefix } +func (n *RangeLoop) GetMarkers() java.Markers { return n.Markers } + func (n *Send) GetID() uuid.UUID { return n.ID } func (n *Send) WithID(id uuid.UUID) java.J { if n.ID == id { diff --git a/rewrite-go/pkg/tree/java/j.go b/rewrite-go/pkg/tree/java/j.go index 6eead79426..4846856780 100644 --- a/rewrite-go/pkg/tree/java/j.go +++ b/rewrite-go/pkg/tree/java/j.go @@ -717,54 +717,8 @@ func (n *ForControl) WithMarkers(markers Markers) *ForControl { return &c } -// ForEachLoop represents a for-range loop: `for k, v := range expr { body }` -type ForEachLoop struct { - ID uuid.UUID - Prefix Space - Markers Markers - Control ForEachControl - Body *Block -} - -func (*ForEachLoop) IsTree() {} -func (*ForEachLoop) IsJ() {} -func (*ForEachLoop) IsStatement() {} - -func (n *ForEachLoop) WithPrefix(prefix Space) *ForEachLoop { - c := *n - c.Prefix = prefix - return &c -} - -func (n *ForEachLoop) WithMarkers(markers Markers) *ForEachLoop { - c := *n - c.Markers = markers - return &c -} - -func (n *ForEachLoop) WithBody(body *Block) *ForEachLoop { - c := *n - c.Body = body - return &c -} - -// ForEachControl holds the variable and iterable of a for-range loop. -// The "range" keyword is implicit (always present). -// -// Structure: `for` [key] [`,` value] [`:=`|`=`] `range` iterable -// - Key and Value may be nil (e.g., `for range expr {}`) -// - When Key is non-nil, Operator contains `:=` or `=` -type ForEachControl struct { - ID uuid.UUID - Prefix Space - Markers Markers - Key *RightPadded[Expression] // nil for `for range expr`; After = space after key (including comma) - Value *RightPadded[Expression] // nil when no value; After = space before operator - Operator LeftPadded[AssignOp] // `:=` or `=`; Before = space before operator. Unused when Key is nil - Iterable Expression // expression after "range" keyword -} - -// AssignOp distinguishes := from = in assignment contexts. +// AssignOp distinguishes := from = in assignment contexts (e.g. Go's +// `for k, v := range` vs `for k, v = range`, modeled by golang.RangeLoop). type AssignOp int const ( @@ -798,21 +752,6 @@ func ParseAssignOp(s string) AssignOp { } } -func (*ForEachControl) IsTree() {} -func (*ForEachControl) IsJ() {} - -func (n *ForEachControl) WithPrefix(prefix Space) *ForEachControl { - c := *n - c.Prefix = prefix - return &c -} - -func (n *ForEachControl) WithMarkers(markers Markers) *ForEachControl { - c := *n - c.Markers = markers - return &c -} - // Switch represents a switch statement. type Switch struct { ID uuid.UUID diff --git a/rewrite-go/pkg/tree/java/j_methods.go b/rewrite-go/pkg/tree/java/j_methods.go index cfaf85612b..755bfe5e81 100644 --- a/rewrite-go/pkg/tree/java/j_methods.go +++ b/rewrite-go/pkg/tree/java/j_methods.go @@ -234,30 +234,6 @@ func (n *ForControl) WithID(id uuid.UUID) J { func (n *ForControl) GetPrefix() Space { return n.Prefix } func (n *ForControl) GetMarkers() Markers { return n.Markers } -func (n *ForEachControl) GetID() uuid.UUID { return n.ID } -func (n *ForEachControl) WithID(id uuid.UUID) J { - if n.ID == id { - return n - } - c := *n - c.ID = id - return &c -} -func (n *ForEachControl) GetPrefix() Space { return n.Prefix } -func (n *ForEachControl) GetMarkers() Markers { return n.Markers } - -func (n *ForEachLoop) GetID() uuid.UUID { return n.ID } -func (n *ForEachLoop) WithID(id uuid.UUID) J { - if n.ID == id { - return n - } - c := *n - c.ID = id - return &c -} -func (n *ForEachLoop) GetPrefix() Space { return n.Prefix } -func (n *ForEachLoop) GetMarkers() Markers { return n.Markers } - func (n *ForLoop) GetID() uuid.UUID { return n.ID } func (n *ForLoop) WithID(id uuid.UUID) J { if n.ID == id { diff --git a/rewrite-go/pkg/visitor/go_visitor.go b/rewrite-go/pkg/visitor/go_visitor.go index e6e3e42e57..b20ca9a161 100644 --- a/rewrite-go/pkg/visitor/go_visitor.go +++ b/rewrite-go/pkg/visitor/go_visitor.go @@ -152,10 +152,8 @@ func (v *GoVisitor) Visit(t java.Tree, p any) java.Tree { return v.self().VisitForLoop(n, p) case *java.ForControl: return v.self().VisitForControl(n, p) - case *java.ForEachLoop: - return v.self().VisitForEachLoop(n, p) - case *java.ForEachControl: - return v.self().VisitForEachControl(n, p) + case *golang.RangeLoop: + return v.self().VisitRangeLoop(n, p) case *java.Break: return v.self().VisitBreak(n, p) case *java.Continue: @@ -275,8 +273,7 @@ type VisitorI interface { VisitCase(c *java.Case, p any) java.J VisitForLoop(forLoop *java.ForLoop, p any) java.J VisitForControl(control *java.ForControl, p any) java.J - VisitForEachLoop(forEach *java.ForEachLoop, p any) java.J - VisitForEachControl(control *java.ForEachControl, p any) java.J + VisitRangeLoop(loop *golang.RangeLoop, p any) java.J VisitBreak(b *java.Break, p any) java.J VisitContinue(c *java.Continue, p any) java.J VisitLabel(l *java.Label, p any) java.J @@ -594,17 +591,29 @@ func (v *GoVisitor) VisitForControl(control *java.ForControl, p any) java.J { return control } -func (v *GoVisitor) VisitForEachLoop(forEach *java.ForEachLoop, p any) java.J { - forEach = forEach.WithPrefix(v.self().VisitSpace(forEach.Prefix, p)) - forEach = forEach.WithMarkers(v.visitMarkers(forEach.Markers, p)) - forEach = forEach.WithBody(visitAndCast[*java.Block](v, forEach.Body, p)) - return forEach -} - -func (v *GoVisitor) VisitForEachControl(control *java.ForEachControl, p any) java.J { - control = control.WithPrefix(v.self().VisitSpace(control.Prefix, p)) - control = control.WithMarkers(v.visitMarkers(control.Markers, p)) - return control +func (v *GoVisitor) VisitRangeLoop(loop *golang.RangeLoop, p any) java.J { + loop = loop.WithPrefix(v.self().VisitSpace(loop.Prefix, p)) + loop = loop.WithMarkers(v.visitMarkers(loop.Markers, p)) + if loop.Key != nil { + k := *loop.Key + k.Element = visitAndCast[java.Expression](v, k.Element, p) + k.After = v.self().VisitSpace(k.After, p) + loop.Key = &k + } + if loop.Value != nil { + val := *loop.Value + val.Element = visitAndCast[java.Expression](v, val.Element, p) + val.After = v.self().VisitSpace(val.After, p) + loop.Value = &val + } + op := loop.Operator + op.Before = v.self().VisitSpace(op.Before, p) + loop.Operator = op + loop.Iterable = visitAndCast[java.Expression](v, loop.Iterable, p) + body := loop.Body + body.Element = visitAndCast[java.Statement](v, body.Element, p) + loop.Body = body + return loop } func (v *GoVisitor) VisitBreak(b *java.Break, p any) java.J { diff --git a/rewrite-go/src/integTest/java/org/openrewrite/golang/rpc/GoRpcPrintRoundTripIntegTest.java b/rewrite-go/src/integTest/java/org/openrewrite/golang/rpc/GoRpcPrintRoundTripIntegTest.java index 7ad14ce17b..b1f90842c1 100644 --- a/rewrite-go/src/integTest/java/org/openrewrite/golang/rpc/GoRpcPrintRoundTripIntegTest.java +++ b/rewrite-go/src/integTest/java/org/openrewrite/golang/rpc/GoRpcPrintRoundTripIntegTest.java @@ -150,6 +150,48 @@ void goUnaryOperators() { "}\n"); } + @Test + void typeSwitch() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc f(v any) string {\n\tswitch v.(type) {\n\tcase int:\n\t\treturn \"int\"\n\tcase string:\n\t\treturn \"string\"\n\tdefault:\n\t\treturn \"other\"\n\t}\n}\n"); + } + + @Test + void selectStatement() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc f(c chan int) int {\n\tselect {\n\tcase v := <-c:\n\t\treturn v\n\tdefault:\n\t\treturn 0\n\t}\n}\n"); + } + + @Test + void multipleAssignment() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc f() {\n\tx, y := 1, 2\n\tx, y = y, x\n\t_ = x\n\t_ = y\n}\n"); + } + + @Test + void rangeKeyValue() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc total(items []int) int {\n\ts := 0\n\tfor i, v := range items {\n\t\ts += i + v\n\t}\n\treturn s\n}\n"); + } + + @Test + void rangeKeyOnly() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc count(items []int) int {\n\tn := 0\n\tfor i := range items {\n\t\tn += i\n\t}\n\treturn n\n}\n"); + } + + @Test + void rangeNoVariable() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc count(items []int) int {\n\tn := 0\n\tfor range items {\n\t\tn++\n\t}\n\treturn n\n}\n"); + } + + @Test + void rangeWithAssign() { + assertPrintsUnchangedAfterReset( + "package main\n\nfunc f(items []int) int {\n\tvar i, v int\n\tfor i, v = range items {\n\t\t_ = i\n\t\t_ = v\n\t}\n\treturn v\n}\n"); + } + @Test void goBitClearBinary() { assertPrintsUnchangedAfterReset( diff --git a/rewrite-go/src/main/java/org/openrewrite/golang/GolangVisitor.java b/rewrite-go/src/main/java/org/openrewrite/golang/GolangVisitor.java index fc2f199c65..16fbe5c619 100644 --- a/rewrite-go/src/main/java/org/openrewrite/golang/GolangVisitor.java +++ b/rewrite-go/src/main/java/org/openrewrite/golang/GolangVisitor.java @@ -319,4 +319,20 @@ public J visitGoVariadic(Go.Variadic variadic, P p) { v = v.withDots(visitSpace(v.getDots(), Space.Location.LANGUAGE_EXTENSION, p)); return v; } + + public J visitRangeLoop(Go.RangeLoop rangeLoop, P p) { + Go.RangeLoop l = rangeLoop; + l = l.withPrefix(visitSpace(l.getPrefix(), Space.Location.LANGUAGE_EXTENSION, p)); + l = l.withMarkers(visitMarkers(l.getMarkers(), p)); + if (l.getPadding().getKey() != null) { + l = l.getPadding().withKey(visitRightPadded(l.getPadding().getKey(), JRightPadded.Location.LANGUAGE_EXTENSION, p)); + } + if (l.getPadding().getValue() != null) { + l = l.getPadding().withValue(visitRightPadded(l.getPadding().getValue(), JRightPadded.Location.LANGUAGE_EXTENSION, p)); + } + l = l.getPadding().withOperator(visitLeftPadded(l.getPadding().getOperator(), JLeftPadded.Location.LANGUAGE_EXTENSION, p)); + l = l.withIterable((Expression) visitAndCast(l.getIterable(), p)); + l = l.getPadding().withBody(visitRightPadded(l.getPadding().getBody(), JRightPadded.Location.LANGUAGE_EXTENSION, p)); + return l; + } } diff --git a/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangReceiver.java b/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangReceiver.java index 8b3668aec7..608bf8f353 100644 --- a/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangReceiver.java +++ b/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangReceiver.java @@ -247,6 +247,16 @@ public J visitGoVariadic(Go.Variadic variadic, RpcReceiveQueue q) { .withPostfix(q.receive(variadic.isPostfix())); } + @Override + public J visitRangeLoop(Go.RangeLoop rangeLoop, RpcReceiveQueue q) { + return rangeLoop + .getPadding().withKey(q.receive(rangeLoop.getPadding().getKey(), el -> visitRightPadded(el, q))) + .getPadding().withValue(q.receive(rangeLoop.getPadding().getValue(), el -> visitRightPadded(el, q))) + .getPadding().withOperator(q.receive(rangeLoop.getPadding().getOperator(), o -> visitLeftPadded(o, q, toEnum(Go.RangeLoop.Type.class)))) + .withIterable(q.receive(rangeLoop.getIterable(), expr -> (Expression) visitNonNull(expr, q))) + .getPadding().withBody(q.receive(rangeLoop.getPadding().getBody(), el -> visitRightPadded(el, q))); + } + // Delegation methods to JavaReceiver for RPC-specific visit methods public JLeftPadded visitLeftPadded(JLeftPadded left, RpcReceiveQueue q) { return delegate.visitLeftPadded(left, q); @@ -288,55 +298,6 @@ public GolangReceiverDelegate(GolangReceiver delegate) { return super.visit(tree, p); } - @Override - public J visitForEachControl(J.ForEachLoop.Control control, RpcReceiveQueue q) { - // Go sends: key (right-padded), value (right-padded), operator (left-padded string), iterable - // Read these and construct a valid ForEachLoop.Control - - // key (right-padded Expression, nullable) - @SuppressWarnings({"unchecked", "rawtypes"}) - JRightPadded key = (JRightPadded) ((RpcReceiveQueue) q).receive( - null, (java.util.function.UnaryOperator) el -> visitRightPadded((JRightPadded) el, q)); - // value (right-padded Expression, nullable) - @SuppressWarnings({"unchecked", "rawtypes"}) - JRightPadded value = (JRightPadded) ((RpcReceiveQueue) q).receive( - null, (java.util.function.UnaryOperator) el -> visitRightPadded((JRightPadded) el, q)); - // operator (left-padded string - AssignOp, skip it) - q.receive(null); - // iterable (Expression) - Expression iterable = q.receive(null, el -> (Expression) visitNonNull(el, q)); - if (iterable == null) { - iterable = new J.Empty(Tree.randomId(), Space.EMPTY, Markers.EMPTY); - } - - // Build a synthetic VariableDeclarations to represent key/value - J.Identifier varName; - if (key != null && key.getElement() instanceof J.Identifier) { - varName = (J.Identifier) key.getElement(); - } else { - varName = new J.Identifier(Tree.randomId(), Space.EMPTY, Markers.EMPTY, - Collections.emptyList(), "_", null, null); - } - - J.VariableDeclarations.NamedVariable namedVar = new J.VariableDeclarations.NamedVariable( - Tree.randomId(), varName.getPrefix(), varName.getMarkers(), - varName.withPrefix(Space.EMPTY), Collections.emptyList(), null, null); - - J.VariableDeclarations varDecls = new J.VariableDeclarations( - Tree.randomId(), Space.EMPTY, Markers.EMPTY, - Collections.emptyList(), Collections.emptyList(), null, - null, Collections.emptyList(), - Collections.singletonList(JRightPadded.build(namedVar))); - - Space afterVar = key != null ? key.getAfter() : Space.EMPTY; - - @SuppressWarnings("unchecked") - JRightPadded varPadded = (JRightPadded) (JRightPadded) JRightPadded.build(varDecls).withAfter(afterVar); - return control - .getPadding().withVariable(varPadded) - .getPadding().withIterable(JRightPadded.build(iterable)); - } - @Override public J visitImport(J.Import importStmt, RpcReceiveQueue q) { importStmt = importStmt.getPadding().withStatic( diff --git a/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangSender.java b/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangSender.java index 7047bcb28e..40620735d8 100644 --- a/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangSender.java +++ b/rewrite-go/src/main/java/org/openrewrite/golang/internal/rpc/GolangSender.java @@ -242,6 +242,16 @@ public J visitGoVariadic(Go.Variadic variadic, RpcSendQueue q) { return variadic; } + @Override + public J visitRangeLoop(Go.RangeLoop rangeLoop, RpcSendQueue q) { + q.getAndSend(rangeLoop, l -> l.getPadding().getKey(), el -> visitRightPadded(el, q)); + q.getAndSend(rangeLoop, l -> l.getPadding().getValue(), el -> visitRightPadded(el, q)); + q.getAndSend(rangeLoop, l -> l.getPadding().getOperator(), op -> visitLeftPadded(op, q)); + q.getAndSend(rangeLoop, Go.RangeLoop::getIterable, el -> visit(el, q)); + q.getAndSend(rangeLoop, l -> l.getPadding().getBody(), el -> visitRightPadded(el, q)); + return rangeLoop; + } + // Delegation methods to JavaSender for RPC-specific visit methods public void visitLeftPadded(JLeftPadded left, RpcSendQueue q) { delegate.visitLeftPadded(left, q); @@ -279,31 +289,6 @@ public GolangSenderDelegate(GolangSender delegate) { return super.visit(tree, p); } - @Override - public J visitForEachControl(J.ForEachLoop.Control control, RpcSendQueue q) { - // Send in Go's format: key (right-padded), value (right-padded), operator (left-padded string), iterable - // Extract key identifier from variable declarations - Statement varStmt = control.getVariable(); - JRightPadded key = null; - if (varStmt instanceof J.VariableDeclarations) { - J.VariableDeclarations varDecls = (J.VariableDeclarations) varStmt; - if (!varDecls.getVariables().isEmpty()) { - J.VariableDeclarations.NamedVariable nv = varDecls.getVariables().get(0); - key = JRightPadded.build(nv.getName()).withAfter(control.getPadding().getVariable().getAfter()); - } - } - final JRightPadded finalKey = key; - // key - q.getAndSend(control, c -> finalKey, el -> visitRightPadded(el, q)); - // value (null for Go's single-variable range) - q.getAndSend(control, c -> (JRightPadded) null, el -> visitRightPadded(el, q)); - // operator (left-padded string - ":=") - q.getAndSend(control, c -> JLeftPadded.build(":=").withBefore(Space.EMPTY)); - // iterable - q.getAndSend(control, c -> c.getIterable(), el -> visit(el, q)); - return control; - } - @Override public J visitImport(J.Import importStmt, RpcSendQueue q) { q.getAndSend(importStmt, i -> i.getPadding().getStatic(), s -> visitLeftPadded(s, q)); diff --git a/rewrite-go/src/main/java/org/openrewrite/golang/tree/Go.java b/rewrite-go/src/main/java/org/openrewrite/golang/tree/Go.java index 369e8b8913..c88478c749 100644 --- a/rewrite-go/src/main/java/org/openrewrite/golang/tree/Go.java +++ b/rewrite-go/src/main/java/org/openrewrite/golang/tree/Go.java @@ -2282,4 +2282,148 @@ public CoordinateBuilder.Expression getCoordinates() { return new CoordinateBuilder.Expression(this); } } + + // --------------------------------------------------------------- + // RangeLoop (Go's for-range: for k, v := range expr { body }) + // --------------------------------------------------------------- + + /** + * Go's {@code for ... range} loop. Unlike Java's single-variable + * {@link J.ForEachLoop.Control}, Go ranges bind a {@code key} and an + * optional {@code value} (or none, for {@code for range expr}), joined by + * {@code :=} or {@code =}. {@code operator.before} is the whitespace before + * the {@code range} keyword. + */ + @ToString + @FieldDefaults(makeFinal = true, level = AccessLevel.PRIVATE) + @EqualsAndHashCode(callSuper = false, onlyExplicitlyIncluded = true) + @RequiredArgsConstructor + @AllArgsConstructor(access = AccessLevel.PRIVATE) + final class RangeLoop implements Go, Loop { + @Nullable + @NonFinal + transient WeakReference padding; + + @EqualsAndHashCode.Include + @With + @Getter + UUID id; + + @With + @Getter + Space prefix; + + @With + @Getter + Markers markers; + + @Nullable + JRightPadded key; + + @Nullable + JRightPadded value; + + JLeftPadded operator; + + @With + @Getter + Expression iterable; + + JRightPadded body; + + public @Nullable Expression getKey() { + return key == null ? null : key.getElement(); + } + + public @Nullable Expression getValue() { + return value == null ? null : value.getElement(); + } + + public Type getOperator() { + return operator.getElement(); + } + + public Go.RangeLoop withOperator(Type operator) { + return getPadding().withOperator(this.operator.withElement(operator)); + } + + @Override + public Statement getBody() { + return body.getElement(); + } + + @Override + @SuppressWarnings("unchecked") + public Go.RangeLoop withBody(Statement body) { + return getPadding().withBody(this.body.withElement(body)); + } + + @Override + public

@Nullable J acceptGolang(GolangVisitor

v, P p) { + return v.visitRangeLoop(this, p); + } + + @Override + public CoordinateBuilder.Statement getCoordinates() { + return new CoordinateBuilder.Statement(this); + } + + /** Whether the loop binds variables with {@code :=} (define) or {@code =} (assign). */ + public enum Type { + Equals, + Define + } + + public Padding getPadding() { + Padding p; + if (this.padding == null) { + p = new Padding(this); + this.padding = new WeakReference<>(p); + } else { + p = this.padding.get(); + if (p == null || p.t != this) { + p = new Padding(this); + this.padding = new WeakReference<>(p); + } + } + return p; + } + + @RequiredArgsConstructor + public static class Padding { + private final Go.RangeLoop t; + + public @Nullable JRightPadded getKey() { + return t.key; + } + + public Go.RangeLoop withKey(@Nullable JRightPadded key) { + return t.key == key ? t : new Go.RangeLoop(t.padding, t.id, t.prefix, t.markers, key, t.value, t.operator, t.iterable, t.body); + } + + public @Nullable JRightPadded getValue() { + return t.value; + } + + public Go.RangeLoop withValue(@Nullable JRightPadded value) { + return t.value == value ? t : new Go.RangeLoop(t.padding, t.id, t.prefix, t.markers, t.key, value, t.operator, t.iterable, t.body); + } + + public JLeftPadded getOperator() { + return t.operator; + } + + public Go.RangeLoop withOperator(JLeftPadded operator) { + return t.operator == operator ? t : new Go.RangeLoop(t.padding, t.id, t.prefix, t.markers, t.key, t.value, operator, t.iterable, t.body); + } + + public JRightPadded getBody() { + return t.body; + } + + public Go.RangeLoop withBody(JRightPadded body) { + return t.body == body ? t : new Go.RangeLoop(t.padding, t.id, t.prefix, t.markers, t.key, t.value, t.operator, t.iterable, body); + } + } + } }