Skip to content

Commit e98f7fc

Browse files
committed
feat!: redesign for-loop syntax with explicit context access
BREAKING CHANGE: The for-loop syntax no longer implicitly treats the first variable as an index when two variables are present. Old syntax (no longer works): for idx, item in items: print(idx, item) New syntax: for item in items with loop: print(loop.idx, item) The `with <identifier>` clause is optional and provides a context object containing: - loop.idx: current iteration index (0-based) - loop.src: the original collection being iterated The recommended convention for for-loops is `loop` (e.g., `with loop`), making the syntax self-documenting: `loop.idx` clearly indicates "the loop's current index". This change unifies the mental model: all variables on the left side of `in` now correspond to values being unpacked from each element. Two variables means unpacking pairs, not index + item. Benefits: - Clearer semantics: variables always map to values - Explicit over implicit: context access is opt-in - Consistent: same pattern for lists, maps, strings, comprehensions - Flexible: context variable name is user-chosen A migration hint appears when the interpreter detects the old pattern (first variable named 'idx', 'index', or 'i' with a non-unpackable collection). This hint triggers for 2 or more left-hand variables, not just exactly 2. Note: The migration hint can be removed from 2027 onwards, once users have had sufficient time to update their scripts. Bug fix: `break` now works correctly in map iteration. Previously, `break` inside a `for key in map:` loop would only exit the switch statement, not the for loop itself. Fixed by adding a labeled loop (`Loop:`) and using `break Loop`. A regression test with switch inside a map loop ensures this won't recur. Implementation notes: - Context creation extracted to `setLoopContext()` helper to avoid duplication between list and map iteration - go.mod contains a temporary replace directive pointing to a local tree-sitter-rad with the grammar changes. This will be removed once tree-sitter-rad publishes a new release.
1 parent 9e4e595 commit e98f7fc

File tree

11 files changed

+591
-78
lines changed

11 files changed

+591
-78
lines changed

core/interpreter.go

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -963,18 +963,32 @@ func (i *Interpreter) doVarPathAssign(varPathNode *ts.Node, rightValue RadValue,
963963
val.ModifyIdx(i, &lastIndex, rightValue)
964964
}
965965

966+
// setLoopContext creates and sets the loop context variable if the user specified 'with <name>'.
967+
// The context is a map containing 'idx' (current iteration index) and 'src' (original collection).
968+
func (i *Interpreter) setLoopContext(contextNode *ts.Node, idx int64, srcValue RadValue) {
969+
if contextNode == nil {
970+
return
971+
}
972+
ctxName := i.GetSrcForNode(contextNode)
973+
ctx := NewRadMap()
974+
ctx.Set(newRadValue(i, contextNode, "idx"), newRadValue(i, contextNode, idx))
975+
ctx.Set(newRadValue(i, contextNode, "src"), srcValue)
976+
i.env.SetVar(ctxName, newRadValue(i, contextNode, ctx))
977+
}
978+
966979
func (i *Interpreter) executeForLoop(node *ts.Node, doOneLoop func() EvalResult) EvalResult {
967980
leftsNode := rl.GetChild(node, rl.F_LEFTS)
968981
rightNode := rl.GetChild(node, rl.F_RIGHT)
982+
contextNode := rl.GetChild(node, rl.F_CONTEXT)
969983

970984
res := i.eval(rightNode)
971985
switch coercedRight := res.Val.Val.(type) {
972986
case RadString:
973-
return runForLoopList(i, leftsNode, rightNode, coercedRight.ToRuneList(), doOneLoop)
987+
return runForLoopList(i, leftsNode, rightNode, contextNode, coercedRight.ToRuneList(), res.Val, doOneLoop)
974988
case *RadList:
975-
return runForLoopList(i, leftsNode, rightNode, coercedRight, doOneLoop)
989+
return runForLoopList(i, leftsNode, rightNode, contextNode, coercedRight, res.Val, doOneLoop)
976990
case *RadMap:
977-
return runForLoopMap(i, leftsNode, coercedRight, doOneLoop)
991+
return runForLoopMap(i, leftsNode, contextNode, coercedRight, res.Val, doOneLoop)
978992
default:
979993
i.errorf(rightNode, "Cannot iterate through a %s", TypeAsString(res.Val))
980994
panic(UNREACHABLE)
@@ -983,52 +997,50 @@ func (i *Interpreter) executeForLoop(node *ts.Node, doOneLoop func() EvalResult)
983997

984998
func runForLoopList(
985999
i *Interpreter,
986-
leftsNode, rightNode *ts.Node,
1000+
leftsNode, rightNode, contextNode *ts.Node,
9871001
list *RadList,
1002+
srcValue RadValue,
9881003
doOneLoop func() EvalResult,
9891004
) EvalResult {
990-
var idxNode *ts.Node
991-
itemNodes := make([]*ts.Node, 0)
992-
9931005
leftNodes := rl.GetChildren(leftsNode, rl.F_LEFT)
9941006

9951007
if len(leftNodes) == 0 {
9961008
i.errorf(leftsNode, "Expected at least one variable on the left side of for loop")
997-
} else if len(leftNodes) == 1 {
998-
itemNodes = append(itemNodes, &leftNodes[0])
999-
} else {
1000-
idxNode = &leftNodes[0]
1001-
for idx := 1; idx < len(leftNodes); idx++ {
1002-
itemNodes = append(itemNodes, &leftNodes[idx])
1003-
}
10041009
}
10051010

10061011
Loop:
10071012
for idx, val := range list.Values {
1008-
if idxNode != nil {
1009-
idxName := i.GetSrcForNode(idxNode)
1010-
i.env.SetVar(idxName, newRadValue(i, idxNode, int64(idx)))
1011-
}
1013+
i.setLoopContext(contextNode, int64(idx), srcValue)
10121014

1013-
if len(itemNodes) == 1 {
1014-
itemNode := itemNodes[0]
1015+
if len(leftNodes) == 1 {
1016+
itemNode := &leftNodes[0]
10151017
itemName := i.GetSrcForNode(itemNode)
10161018
i.env.SetVar(itemName, val)
1017-
} else if len(itemNodes) > 1 {
1018-
// expecting list of lists, unpacking by idx
1019+
} else {
1020+
// Multiple variables = unpacking (expecting list of lists)
10191021
listInList, ok := val.TryGetList()
10201022
if !ok {
1021-
i.errorf(rightNode, "Expected list of lists, got element type %q", TypeAsString(val))
1023+
// Migration hint for old syntax
1024+
firstName := i.GetSrcForNode(&leftNodes[0])
1025+
if firstName == "idx" || firstName == "index" || firstName == "i" {
1026+
i.errorf(rightNode, "Cannot unpack %q into %d values\n\n"+
1027+
"Note: The for-loop syntax changed. It looks like you may be using the old syntax.\n"+
1028+
"Old: for idx, item in items:\n"+
1029+
"New: for item in items with loop:\n"+
1030+
" print(loop.idx, item)",
1031+
TypeAsString(val), len(leftNodes))
1032+
}
1033+
i.errorf(rightNode, "Cannot unpack %q into %d values", TypeAsString(val), len(leftNodes))
10221034
}
10231035

1024-
if listInList.LenInt() < len(itemNodes) {
1036+
if listInList.LenInt() < len(leftNodes) {
10251037
i.errorf(rightNode, "Expected at least %s in inner list, got %d",
1026-
com.Pluralize(len(itemNodes), "value"), listInList.LenInt())
1038+
com.Pluralize(len(leftNodes), "value"), listInList.LenInt())
10271039
}
10281040

1029-
for idx, itemNode := range itemNodes {
1030-
itemName := i.GetSrcForNode(itemNode)
1031-
i.env.SetVar(itemName, listInList.Values[idx])
1041+
for j, itemNode := range leftNodes {
1042+
itemName := i.GetSrcForNode(&itemNode)
1043+
i.env.SetVar(itemName, listInList.Values[j])
10321044
}
10331045
}
10341046

@@ -1043,7 +1055,13 @@ Loop:
10431055
return VoidNormal
10441056
}
10451057

1046-
func runForLoopMap(i *Interpreter, leftsNode *ts.Node, radMap *RadMap, doOneLoop func() EvalResult) EvalResult {
1058+
func runForLoopMap(
1059+
i *Interpreter,
1060+
leftsNode, contextNode *ts.Node,
1061+
radMap *RadMap,
1062+
srcValue RadValue,
1063+
doOneLoop func() EvalResult,
1064+
) EvalResult {
10471065
var keyNode *ts.Node
10481066
var valueNode *ts.Node
10491067

@@ -1059,7 +1077,11 @@ func runForLoopMap(i *Interpreter, leftsNode *ts.Node, radMap *RadMap, doOneLoop
10591077
valueNode = &leftNodes[1]
10601078
}
10611079

1080+
idx := int64(0)
1081+
Loop:
10621082
for _, key := range radMap.Keys() {
1083+
i.setLoopContext(contextNode, idx, srcValue)
1084+
10631085
keyName := i.GetSrcForNode(keyNode)
10641086
i.env.SetVar(keyName, key)
10651087

@@ -1072,10 +1094,11 @@ func runForLoopMap(i *Interpreter, leftsNode *ts.Node, radMap *RadMap, doOneLoop
10721094
res := doOneLoop()
10731095
switch res.Ctrl {
10741096
case CtrlBreak:
1075-
break
1097+
break Loop
10761098
case CtrlReturn, CtrlYield:
10771099
return res
10781100
}
1101+
idx++
10791102
}
10801103
return VoidNormal
10811104
}

core/testing/for_context_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package testing
2+
3+
import "testing"
4+
5+
func Test_For_WithContext_PrintFullContext(t *testing.T) {
6+
// Print the full context object to verify structure
7+
script := `
8+
items = ["a", "b", "c"]
9+
for item in items with loop:
10+
print(loop)
11+
`
12+
setupAndRunCode(t, script, "--color=never")
13+
expected := `{ "idx": 0, "src": [ "a", "b", "c" ] }
14+
{ "idx": 1, "src": [ "a", "b", "c" ] }
15+
{ "idx": 2, "src": [ "a", "b", "c" ] }
16+
`
17+
assertOnlyOutput(t, stdOutBuffer, expected)
18+
assertNoErrors(t)
19+
}
20+
21+
func Test_For_WithContext_IdxAndItem(t *testing.T) {
22+
script := `
23+
items = ["a", "b", "c"]
24+
for item in items with loop:
25+
print(loop.idx, item)
26+
`
27+
setupAndRunCode(t, script, "--color=never")
28+
assertOnlyOutput(t, stdOutBuffer, "0 a\n1 b\n2 c\n")
29+
assertNoErrors(t)
30+
}
31+
32+
func Test_For_WithContext_AccessSrc(t *testing.T) {
33+
// Use loop.src to access original collection for lookahead
34+
script := `
35+
items = ["a", "b", "c"]
36+
for item in items with loop:
37+
if loop.idx < loop.src.len() - 1:
38+
print(loop.src[loop.idx + 1])
39+
`
40+
setupAndRunCode(t, script, "--color=never")
41+
assertOnlyOutput(t, stdOutBuffer, "b\nc\n")
42+
assertNoErrors(t)
43+
}
44+
45+
func Test_For_WithContext_Map(t *testing.T) {
46+
script := `
47+
m = {"a": 1, "b": 2, "c": 3}
48+
for key in m with loop:
49+
print(loop.idx, key)
50+
`
51+
setupAndRunCode(t, script, "--color=never")
52+
assertOnlyOutput(t, stdOutBuffer, "0 a\n1 b\n2 c\n")
53+
assertNoErrors(t)
54+
}
55+
56+
func Test_For_WithContext_MapKeyValue(t *testing.T) {
57+
script := `
58+
m = {"a": 1, "b": 2}
59+
for key, value in m with loop:
60+
print(loop.idx, key, value)
61+
`
62+
setupAndRunCode(t, script, "--color=never")
63+
assertOnlyOutput(t, stdOutBuffer, "0 a 1\n1 b 2\n")
64+
assertNoErrors(t)
65+
}
66+
67+
func Test_For_WithContext_ListComprehension(t *testing.T) {
68+
script := `
69+
items = ["a", "b", "c"]
70+
result = ["{loop.idx}:{item}" for item in items with loop]
71+
print(result)
72+
`
73+
setupAndRunCode(t, script, "--color=never")
74+
assertOnlyOutput(t, stdOutBuffer, "[ \"0:a\", \"1:b\", \"2:c\" ]\n")
75+
assertNoErrors(t)
76+
}
77+
78+
func Test_For_WithContext_String(t *testing.T) {
79+
script := `
80+
s = "abc"
81+
for char in s with loop:
82+
print(loop.idx, char)
83+
`
84+
setupAndRunCode(t, script, "--color=never")
85+
assertOnlyOutput(t, stdOutBuffer, "0 a\n1 b\n2 c\n")
86+
assertNoErrors(t)
87+
}
88+
89+
func Test_For_WithContext_CustomName(t *testing.T) {
90+
// User can use any name for context variable
91+
script := `
92+
items = [1, 2, 3]
93+
for item in items with meta:
94+
print(meta.idx)
95+
`
96+
setupAndRunCode(t, script, "--color=never")
97+
assertOnlyOutput(t, stdOutBuffer, "0\n1\n2\n")
98+
assertNoErrors(t)
99+
}
100+
101+
func Test_For_Unpacking_NoAutoIndex(t *testing.T) {
102+
script := `
103+
pairs = [["a", 1], ["b", 2]]
104+
for name, val in pairs:
105+
print(name, val)
106+
`
107+
setupAndRunCode(t, script, "--color=never")
108+
assertOnlyOutput(t, stdOutBuffer, "a 1\nb 2\n")
109+
assertNoErrors(t)
110+
}
111+
112+
func Test_For_Unpacking_WithContext(t *testing.T) {
113+
script := `
114+
pairs = [["a", 1], ["b", 2]]
115+
for name, val in pairs with loop:
116+
print(loop.idx, name, val)
117+
`
118+
setupAndRunCode(t, script, "--color=never")
119+
assertOnlyOutput(t, stdOutBuffer, "0 a 1\n1 b 2\n")
120+
assertNoErrors(t)
121+
}

core/testing/for_test.go

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ for item in a:
1616
func Test_For_ILoop(t *testing.T) {
1717
script := `
1818
a = ["a", "b", "c"]
19-
for idx, item in a:
20-
print(idx, item)
19+
for item in a with loop:
20+
print(loop.idx, item)
2121
`
2222
setupAndRunCode(t, script, "--color=never")
2323
assertOnlyOutput(t, stdOutBuffer, "0 a\n1 b\n2 c\n")
@@ -28,8 +28,8 @@ func Test_For_ChangesInsideAreRemembered(t *testing.T) {
2828
script := `
2929
num = 0
3030
a = ["a", "b", "c"]
31-
for idx, item in a:
32-
num += idx
31+
for item in a with loop:
32+
num += loop.idx
3333
print(num)
3434
`
3535
setupAndRunCode(t, script, "--color=never")
@@ -112,3 +112,36 @@ for i in range(5):
112112
assertOnlyOutput(t, stdOutBuffer, expected)
113113
assertNoErrors(t)
114114
}
115+
116+
func Test_For_MapCanBreak(t *testing.T) {
117+
script := `
118+
m = {"a": 1, "b": 2, "c": 3}
119+
for key in m:
120+
print(key)
121+
break
122+
`
123+
setupAndRunCode(t, script, "--color=never")
124+
assertOnlyOutput(t, stdOutBuffer, "a\n")
125+
assertNoErrors(t)
126+
}
127+
128+
func Test_For_MapCanBreakFromSwitch(t *testing.T) {
129+
// Regression test: break inside switch should exit the for loop, not just the switch.
130+
// The original bug was that the switch statement used 'break' internally, so a user's
131+
// 'break' would only exit the switch, causing an infinite loop.
132+
script := `
133+
m = {"a": 1, "b": 2, "c": 3}
134+
for key, val in m:
135+
switch val:
136+
case 1:
137+
print("found one")
138+
break
139+
case 2:
140+
print("found two")
141+
default:
142+
print("other")
143+
`
144+
setupAndRunCode(t, script, "--color=never")
145+
assertOnlyOutput(t, stdOutBuffer, "found one\n")
146+
assertNoErrors(t)
147+
}

0 commit comments

Comments
 (0)