Skip to content

Go: model for ... range as a rich Go.RangeLoop instead of cramming it into J.ForEachLoop#7843

Open
knutwannheden wants to merge 1 commit into
mainfrom
go-rpc-range-loop
Open

Go: model for ... range as a rich Go.RangeLoop instead of cramming it into J.ForEachLoop#7843
knutwannheden wants to merge 1 commit into
mainfrom
go-rpc-range-loop

Conversation

@knutwannheden
Copy link
Copy Markdown
Contributor

@knutwannheden knutwannheden commented May 29, 2026

Motivation

Running a recipe over Go source mangled every for ... range loop:

-	for _, v := range items {
+	for_:=range items {

Go's range loop binds a key and an optional value joined by := or = (for k, v := range x, for k := range x, for range x). That doesn't fit Java's single-variable J.ForEachLoop.Control (one variable + iterable). The Go side worked around it by defining its own fatter java.ForEachControl (Key/Value/Operator/Iterable) masquerading under the Java type name, then bridging it to the real J.ForEachLoop.Control over RPC with a hand-written codec.

  • That bridge is the bug: the real J.ForEachLoop.Control has nowhere to put a second variable or the :=/=, so the codec synthesized a single variable from the key and dropped the value and the operator. Like the operator fixes in Go: map Go-specific operators to rich Go.* LST nodes so they survive RPC round-trips #7842, it round-trips fine Go↔Go (both sides use the fat struct) but truncates on the Java leg — invisible until a recipe forces full deserialization.

Examples

New node (mirrors the per-language loop-node pattern — JS has ForOfLoop/ForInLoop):

Go.RangeLoop   for k, v := range x { … }
  key       JRightPadded<Expression>   // nullable (for range x)
  value     JRightPadded<Expression>   // nullable (for k := range x)
  operator  JLeftPadded<Type>          // Type {Equals, Define}; before = space before `range`
  iterable  Expression
  body      JRightPadded<Statement>

Summary

  • Added Go.RangeLoop end-to-end (Java Go.java/GolangVisitor/GolangSender/GolangReceiver; Go tree + parser + printer + visitor + RPC sender/receiver + factory & RegisterValueType).
  • Removed the cheat entirely: the Go-customized java.ForEachLoop/java.ForEachControl structs and methods, their RPC send/receive, the lossy visitForEachControl/visitForEachLoop bridge overrides, and the J$ForEachLoop/J$ForEachLoop$Control wire registrations. Template matching (comparator/substitution/go_template) now matches Go.RangeLoop.
  • AssignOp (Equals/Define) is kept and reused as the operator enum, round-tripping as the Go.RangeLoop.Type constant name.

Note for reviewers: a Go LST node needs both RegisterFactory (receive) and RegisterValueType (send) — registering only one silently desyncs the wire into a ParseError.

Test plan

  • New range cases in GoRpcPrintRoundTripIntegTest (key+value, key-only, no-variable, = assign) — all green
  • ./gradlew :rewrite-go:test :rewrite-go:integTest passes (no regressions)
  • go test ./... passes

…it into J.ForEachLoop

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant