From 92c7bd20f3a324c61ad9b2ee3046e7015868dae4 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:38:56 +0200 Subject: [PATCH 01/14] =?UTF-8?q?fix(lexer):=20%-literal=20delimiters=20?= =?UTF-8?q?=E2=80=94=20interpolation,=20`=3D`=20and=20`\`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related faults in the %-literal scan, all found by `language/string_spec.rb`, which now parses (it stopped at line 71 before). 1. The terminator scan looked inside `#{…}`, so `%@hey #{@ip}@` closed on the `@` of `@ip`. MRI's `tokadd_string` (parse.y v3_4_0) leaves the literal scan at a `#` that introduces an interpolation and hands the embedded expression to the main lexer, so the terminator is never looked for inside one. The brace-balanced interpolation is now copied verbatim, in %Q/%() strings and in the interpolating %W/%I word lists (%r/%x/%s already did this). 2. `%=hey=` was lexed as the `%=` op-assign. MRI's `parse_percent` tests `IS_BEG()` first and only falls through to `if ((c = nextc(p)) == '=') … return tOP_ASGN;` away from expression-begin, so `=` delimits a literal where a value is expected and stays op-assign after one. `percentDelimOK` now makes that distinction; `isPercentDelim` keeps only the state-free rules. 3. `%\hey\` treated the closing `\` as an escape. In `tokadd_string` the terminator test precedes the backslash test, so a backslash delimiter closes the literal; an escaped delimiter inside a differently-delimited literal (`%(a\)b)`) still escapes, because the escape consumes its byte before the loop top is reached again. Verified against MRI 4.0.5: `%@hey #{@ip}@`, `%=hey=`, `%\hey\`, `%'hey'`, `%$hey$`, `%w=a b=`, `%q(a\)b)`, `a %= 2`. Co-Authored-By: Claude Opus 5 --- lexer/lexer.go | 71 +++++++++++++---- lexer/lexer_round3_test.go | 25 +++++- percent_delim_test.go | 155 +++++++++++++++++++++++++++++++++++++ 3 files changed, 232 insertions(+), 19 deletions(-) create mode 100644 percent_delim_test.go diff --git a/lexer/lexer.go b/lexer/lexer.go index 3b3cec3..3e7d650 100644 --- a/lexer/lexer.go +++ b/lexer/lexer.go @@ -1414,6 +1414,20 @@ func (l *Lexer) percentBeginsLiteral(spaceBefore bool) bool { return spaceBefore } +// percentDelimOK reports whether b may delimit a %-literal starting at the +// cursor. MRI accepts every non-alphanumeric ASCII character as a delimiter +// (parse.y v3_4_0, parse_percent: `if (!ISALNUM(c)) { term = c; … c = 'Q'; }`), +// but only reaches that code at expression-begin: everywhere else the op-assign +// test runs first (`if ((c = nextc(p)) == '=') … return tOP_ASGN;`). So `%=` +// opens a literal where a value is expected (`%=hey=.length`) and stays the +// modulo op-assign after one (`a %= 2`). +func (l *Lexer) percentDelimOK(b byte) bool { + if b == '=' { + return l.state == exprBegin + } + return isPercentDelim(b) +} + // atPercentArray reports whether the cursor (positioned at '%') begins a // %w/%i array literal: the kind letter must be followed by a delimiter. func (l *Lexer) atPercentArray() bool { @@ -1422,7 +1436,7 @@ func (l *Lexer) atPercentArray() bool { default: return false } - return l.pos+2 < len(l.src) && isPercentDelim(l.src[l.pos+2]) + return l.pos+2 < len(l.src) && l.percentDelimOK(l.src[l.pos+2]) } // lexPercentArray lexes a %w/%i/%W/%I array literal. The non-interpolating @@ -1458,6 +1472,13 @@ func (l *Lexer) lexPercentArray(spaceBefore bool, line, col int) token.Token { content = append(content, l.advance()) continue } + if (kind == 'W' || kind == 'I') && c == '#' && l.peek2() == '{' { + // As in lexPercentString: an interpolation is copied verbatim so a + // delimiter character inside it cannot close the list. + content = append(content, l.advance(), l.advance()) // '#' '{' + content = l.copyInterpolation(content) + continue + } if open != closing && c == open { depth++ } else if c == closing { @@ -1552,14 +1573,15 @@ func splitWordList(body string, open, closing byte) []string { return words } -// isPercentDelim reports whether b can open a %-literal. MRI accepts any -// non-alphanumeric character as the delimiter (`%r"..."`, `%q[...]`, `%w'a b'`, -// `%i`, `%(...)`, `%!...!`, `%|...|`, `%@...@`, …); a bracket pair nests. -// Excluded: alphanumerics (they would be the literal's body), whitespace, and -// `=` so a `%=` compound assignment is never mistaken for a literal opener. +// isPercentDelim reports whether b can open a %-literal irrespective of the +// lexer state. MRI accepts any non-alphanumeric character as the delimiter +// (`%r"..."`, `%q[...]`, `%w'a b'`, `%i`, `%(...)`, `%!...!`, `%|...|`, +// `%@...@`, …); a bracket pair nests. Excluded: alphanumerics (they would be +// the literal's body) and whitespace. `=` is state-dependent and is decided by +// percentDelimOK, which is what the call sites use. func isPercentDelim(b byte) bool { switch { - case b == 0 || b == '=': + case b == 0: return false case b == ' ' || b == '\t' || b == '\n' || b == '\r' || b == '\f' || b == '\v': return false @@ -1582,7 +1604,7 @@ func (l *Lexer) atPercentRXS() bool { default: return false } - return l.pos+2 < len(l.src) && isPercentDelim(l.src[l.pos+2]) + return l.pos+2 < len(l.src) && l.percentDelimOK(l.src[l.pos+2]) } // lexPercentRXS lexes a %r{…}flags regexp, %x{…} backtick command, or %s{…} @@ -1672,9 +1694,9 @@ func (l *Lexer) lexPercentRXS(spaceBefore bool, line, col int) token.Token { // followed by a delimiter (== %Q). func (l *Lexer) atPercentString() bool { if c := l.peek2(); c == 'q' || c == 'Q' { - return l.pos+2 < len(l.src) && isPercentDelim(l.src[l.pos+2]) + return l.pos+2 < len(l.src) && l.percentDelimOK(l.src[l.pos+2]) } - return isPercentDelim(l.peek2()) + return l.percentDelimOK(l.peek2()) } // lexPercentString lexes %q(…) (non-interpolating; only \ and \\ escape), @@ -1696,21 +1718,36 @@ func (l *Lexer) lexPercentString(spaceBefore bool, line, col int) token.Token { if c == 0 { return token.Token{Type: token.ILLEGAL, Lit: "unterminated %-string literal", Line: line, Col: col, SpaceBefore: spaceBefore} } - if c == '\\' { // keep escape pairs verbatim; an escaped delimiter never nests + if open != closing && c == open { + depth++ body = append(body, l.advance()) - if l.peek() != 0 { - body = append(body, l.advance()) - } continue } - if open != closing && c == open { - depth++ - } else if c == closing { + if c == closing { depth-- if depth == 0 { l.advance() // closing delimiter break } + body = append(body, l.advance()) + continue + } + if c == '\\' { // keep escape pairs verbatim; an escaped delimiter never nests + body = append(body, l.advance()) + if l.peek() != 0 { + body = append(body, l.advance()) + } + continue + } + if interp && c == '#' && l.peek2() == '{' { + // MRI's tokadd_string leaves the literal scan at `#{` and hands the + // embedded expression to the main lexer, so the terminator is never + // looked for inside an interpolation: `%@a #{@b}@` closes on the final + // '@', not on the one starting @b. Copy the whole brace-balanced + // interpolation verbatim instead of scanning it for the delimiter. + body = append(body, l.advance(), l.advance()) // '#' '{' + body = l.copyInterpolation(body) + continue } body = append(body, l.advance()) } diff --git a/lexer/lexer_round3_test.go b/lexer/lexer_round3_test.go index 43a6d42..beccf58 100644 --- a/lexer/lexer_round3_test.go +++ b/lexer/lexer_round3_test.go @@ -12,7 +12,7 @@ func firstType(src string) token.Type { } // TestPercentNonDelimiters checks that a character that cannot delimit a -// percent-literal (an alphanumeric, whitespace, '=', or a multi-byte UTF-8 lead) +// percent-literal (an alphanumeric, whitespace, or a multi-byte UTF-8 lead) // leaves the '%' as the modulo/percent operator rather than opening a literal. // This exercises every false branch of isPercentDelim. func TestPercentNonDelimiters(t *testing.T) { @@ -29,7 +29,7 @@ func TestPercentNonDelimiters(t *testing.T) { } // A bare `%` followed by a non-delimiter is the percent operator. for _, src := range []string{ - "%=", // compound-assignment, never a literal + "%", // end of input: no delimiter byte at all "%9", // digit "%a", // alpha (not q/Q/w/i/r/x/s) "% ", // space @@ -67,3 +67,24 @@ func TestPercentDelimiterAccepted(t *testing.T) { } } } + +// TestPercentEqualsIsOpAssignAfterAValue pins the state-dependent half of +// percentDelimOK: after a value MRI's parse_percent reaches its `'='` test +// before the quotation branch, so `%=` is the modulo compound assignment. +func TestPercentEqualsIsOpAssignAfterAValue(t *testing.T) { + toks := New("a %= 2").Tokenize() + if toks[1].Type != token.OPASSIGN || toks[1].Lit != "%" { + t.Errorf("a %%= 2: second token = %s %q, want OPASSIGN %q", toks[1].Type, toks[1].Lit, "%") + } +} + +// TestPercentEqualsDelimitsAtExpressionBegin is the other half: where a value is +// expected, `=` delimits the literal. +func TestPercentEqualsDelimitsAtExpressionBegin(t *testing.T) { + for _, src := range []string{"%=hey=", "%q=hey=", "%w=a b="} { + toks := New(src).Tokenize() + if toks[0].Type == token.OPASSIGN || toks[0].Type == token.ILLEGAL { + t.Errorf("%q: first token = %s, want a literal", src, toks[0].Type) + } + } +} diff --git a/percent_delim_test.go b/percent_delim_test.go new file mode 100644 index 0000000..91a8f56 --- /dev/null +++ b/percent_delim_test.go @@ -0,0 +1,155 @@ +package parser_test + +import ( + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// TestPercentLiteralInterpolationHidesDelimiter covers the %-literal terminator +// scan: MRI's tokadd_string (parse.y v3_4_0) leaves the scan at `#{` and hands +// the embedded expression to the main lexer, so a delimiter character occurring +// inside an interpolation cannot close the literal. +func TestPercentLiteralInterpolationHidesDelimiter(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + src string + want []string // the string-valued parts, in order + }{ + {"ivar in at-delimited string", `%@hey #{@ip}@`, []string{"hey ", ""}}, + {"delimiter inside interpolation", `%@a #{"@"} b@`, []string{"a ", "@", " b"}}, + {"Q form", `%Q!a #{"!"} b!`, []string{"a ", "!", " b"}}, + {"regexp form keeps working", `%r@a #{"@"} b@`, nil}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if _, err := parser.Parse(tc.src); err != nil { + t.Fatalf("Parse(%q) returned error: %v", tc.src, err) + } + if tc.want == nil { + return + } + n := parseOne(t, tc.src) + si, ok := n.(*ast.StrInterp) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.StrInterp", tc.src, n) + } + var got []string + for _, part := range si.Parts { + if s, ok := part.(*ast.StringLit); ok { + got = append(got, s.Value) + } + } + if len(got) != len(tc.want) { + t.Fatalf("Parse(%q): %d literal parts %q, want %d", tc.src, len(got), got, len(tc.want)) + } + for i, w := range tc.want { + if got[i] != w { + t.Errorf("Parse(%q): part %d = %q, want %q", tc.src, i, got[i], w) + } + } + }) + } +} + +// TestPercentWordListInterpolationHidesDelimiter is the %W/%I half of the same +// rule: `%W@a#{"@"}b@` is the one word `a@b`, not a list cut at the inner `@`. +func TestPercentWordListInterpolationHidesDelimiter(t *testing.T) { + t.Parallel() + for _, src := range []string{`p %W@a#{"@"}b@`, `p %I@a#{"@"}b@`} { + if _, err := parser.Parse(src); err != nil { + t.Fatalf("Parse(%q) returned error: %v", src, err) + } + arr, ok := commandArg(t, src, "p", 0).(*ast.ArrayLit) + if !ok { + t.Fatalf("Parse(%q): argument is not an *ast.ArrayLit", src) + } + if len(arr.Elems) != 1 { + t.Errorf("Parse(%q): %d elements, want 1", src, len(arr.Elems)) + } + } +} + +// TestPercentEqualsDelimiter covers MRI's parse_percent ordering: at +// expression-begin every non-alphanumeric byte delimits a %-literal, so `%=…=` +// is a string; away from expression-begin the op-assign test runs first, so +// `a %= 2` stays the modulo op-assign. +func TestPercentEqualsDelimiter(t *testing.T) { + t.Parallel() + t.Run("literal at expression begin", func(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ src, want string }{ + {`x = %=hey=`, "hey"}, + {`x = %q=hey=`, "hey"}, + } { + n := parseOne(t, tc.src) + asg, ok := n.(*ast.Assign) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.Assign", tc.src, n) + } + s, ok := asg.Value.(*ast.StringLit) + if !ok { + t.Fatalf("Parse(%q): value = %T, want *ast.StringLit", tc.src, asg.Value) + } + if s.Value != tc.want { + t.Errorf("Parse(%q) = %q, want %q", tc.src, s.Value, tc.want) + } + } + }) + t.Run("word list at expression begin", func(t *testing.T) { + t.Parallel() + src := `x = %w=a b=` + asg, ok := parseOne(t, src).(*ast.Assign) + if !ok { + t.Fatalf("Parse(%q): want *ast.Assign", src) + } + wantStrings(t, src, asg.Value, []string{"a", "b"}) + }) + t.Run("regexp at expression begin", func(t *testing.T) { + t.Parallel() + src := `x = %r=a=` + asg, ok := parseOne(t, src).(*ast.Assign) + if !ok { + t.Fatalf("Parse(%q): want *ast.Assign", src) + } + if _, ok := asg.Value.(*ast.RegexpLit); !ok { + t.Fatalf("Parse(%q): value = %T, want *ast.RegexpLit", src, asg.Value) + } + }) + t.Run("op-assign after a value", func(t *testing.T) { + t.Parallel() + src := `a %= 2` + n := parseOne(t, src) + if _, ok := n.(*ast.OpAssign); !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.OpAssign", src, n) + } + }) +} + +// TestPercentBackslashDelimiter covers MRI's tokadd_string loop order: the +// terminator test precedes the backslash-escape test, so a backslash used as +// the delimiter closes the literal instead of escaping the next byte. An +// escaped delimiter inside a literal delimited by something else still escapes. +func TestPercentBackslashDelimiter(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ src, want string }{ + {`x = %\hey\`, "hey"}, + {`x = %(a\)b)`, "a)b"}, + {`x = %q(a\)b)`, "a)b"}, + {`x = %(a(b)c)`, "a(b)c"}, + } { + asg, ok := parseOne(t, tc.src).(*ast.Assign) + if !ok { + t.Fatalf("Parse(%q): want *ast.Assign", tc.src) + } + s, ok := asg.Value.(*ast.StringLit) + if !ok { + t.Fatalf("Parse(%q): value = %T, want *ast.StringLit", tc.src, asg.Value) + } + if s.Value != tc.want { + t.Errorf("Parse(%q) = %q, want %q", tc.src, s.Value, tc.want) + } + } +} From 9ce5aec7a06628c6d8dd299175280e29846655c0 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:41:52 +0200 Subject: [PATCH 02/14] fix(parser): a class/module/def definition is a primary, not a statement form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `class C; end.should == nil` and `class << true; self; end.should == TrueClass` failed with `unexpected "=="`: parseStatement intercepted CLASS/MODULE/DEF and ran only the postfix and modifier layers over the result, so the definition could take a `.method` but could not be an operand. In MRI these are alternatives of `primary` (parse.y v3_4_0, lines 4599-4657: `k_class cpath superclass bodystmt k_end`, `k_class tLSHFT expr_value term bodystmt k_end`, `k_module cpath bodystmt k_end`, `defn_head f_arglist bodystmt k_end`), so they compose with everything a primary composes with. parsePrimary already handled all three; the statement-level interception was what kept them from being reached through the expression path. Removing it routes them through parseKeywordLogical → parseExprOrAssign → … → parsePrimary, which still applies the postfix tail and the modifiers, and additionally the operator layers. Unblocks language/class_spec.rb (32 passing examples) and language/metaclass_spec.rb (18), both of which parse for the first time. Verified against MRI 4.0.5. Co-Authored-By: Claude Opus 5 --- class_expr_test.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++ parser.go | 9 ------- 2 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 class_expr_test.go diff --git a/class_expr_test.go b/class_expr_test.go new file mode 100644 index 0000000..d100315 --- /dev/null +++ b/class_expr_test.go @@ -0,0 +1,64 @@ +package parser_test + +import ( + "fmt" + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// TestDefinitionIsAPrimary pins MRI's grammar: `k_class cpath superclass +// bodystmt k_end`, `k_class tLSHFT expr_value term bodystmt k_end`, +// `k_module cpath bodystmt k_end` and `defn_head f_arglist bodystmt k_end` are +// all alternatives of `primary` (parse.y v3_4_0), so a definition composes with +// everything a primary composes with — not only a trailing `.method` and a +// statement modifier, but binary operators too. `class C; end.should == nil` is +// the shape ruby/spec uses to pin a class body's value. +func TestDefinitionIsAPrimary(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + src string + }{ + {"class compared", `class C; end.should == nil`}, + {"class with a body compared", `class C; 20; end.value == 20`}, + {"singleton class compared", `class << self; :s; end.value == :s`}, + {"module compared", `module M; end.to_s == "M"`}, + {"def compared", `def f; end.to_s == "f"`}, + {"class in a boolean chain", `class C; end.nil? && true`}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + n := parseOne(t, tc.src) + if _, ok := n.(*ast.BinaryExpr); !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.BinaryExpr", tc.src, n) + } + }) + } +} + +// TestDefinitionKeepsStatementForms checks the plain statement shapes still +// parse the same way once definitions go through the expression path. +func TestDefinitionKeepsStatementForms(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + src string + want any + }{ + {`class C; end`, (*ast.ClassDef)(nil)}, + {`module M; end`, (*ast.ModuleDef)(nil)}, + {`def f; end`, (*ast.MethodDef)(nil)}, + {`class C; end if true`, (*ast.If)(nil)}, + {`def f; end if true`, (*ast.If)(nil)}, + {`def f; end.tap { |x| x }`, (*ast.Call)(nil)}, + } { + n := parseOne(t, tc.src) + if got, want := fmt.Sprintf("%T", n), fmt.Sprintf("%T", tc.want); got != want { + t.Errorf("Parse(%q): node = %s, want %s", tc.src, got, want) + } + } + if _, err := parser.Parse("class C; def m; end; end"); err != nil { + t.Errorf("nested definition: %v", err) + } +} diff --git a/parser.go b/parser.go index f3170f0..4caeeae 100644 --- a/parser.go +++ b/parser.go @@ -301,15 +301,6 @@ func (p *Parser) parseStatements(stop map[token.Type]bool) []ast.Node { func (p *Parser) parseStatement() ast.Node { switch p.cur().Type { - case token.DEF: - // A method definition may carry a trailing modifier (`def f; …; end if c`) - // and is occasionally chained (`def f; end.tap { … }`), so feed it through - // the postfix/modifier machinery rather than returning it raw. - return p.applyModifiers(p.parsePostfixTail(p.parseDef())) - case token.CLASS: - return p.applyModifiers(p.parsePostfixTail(p.parseClass())) - case token.MODULE: - return p.applyModifiers(p.parsePostfixTail(p.parseModule())) case token.RETURN: return p.applyModifiers(p.parseReturn()) case token.BREAK: From e30e55b200faa27b71e2a01fbb65cf144b206e5d Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:42:40 +0200 Subject: [PATCH 03/14] test: cover applyModifiers' rescue arm, now reached only by alias/undef Routing class/module/def through the expression path moved their `rescue` modifier to withRescueModifier. `alias`/`undef` are the remaining statements that never enter the expression layers (MRI parse.y v3_4_0: `stmt: keyword_alias fitem fitem | keyword_undef undef_list`), so they are what still reaches applyModifiers' RESCUE arm. Pin it; the 100% coverage gate had slipped to 99.9%. Co-Authored-By: Claude Opus 5 --- modifier_rescue_command_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/modifier_rescue_command_test.go b/modifier_rescue_command_test.go index 61e5821..f47265f 100644 --- a/modifier_rescue_command_test.go +++ b/modifier_rescue_command_test.go @@ -217,3 +217,23 @@ func TestModifierRescueBindsTighterThanIf(t *testing.T) { t.Fatalf("%q: fallback = %T, want *ast.IntLit", src, fallback) } } + +// TestRescueModifierOnAliasAndUndef covers applyModifiers' RESCUE arm. Most +// statements have their `rescue` modifier consumed by withRescueModifier on the +// way out of the expression layers; `alias`/`undef` never enter those layers +// (MRI: `stmt: keyword_alias fitem fitem | keyword_undef undef_list`, both +// reached through `stmt_or_begin`, with `stmt modifier_rescue stmt` wrapping +// them), so the modifier is applied by the statement layer instead. +func TestRescueModifierOnAliasAndUndef(t *testing.T) { + t.Parallel() + for _, src := range []string{`alias a b rescue nil`, `undef a rescue nil`} { + n := parseOne(t, src) + b, ok := n.(*ast.Begin) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.Begin", src, n) + } + if len(b.Rescues) != 1 { + t.Errorf("Parse(%q): %d rescue clauses, want 1", src, len(b.Rescues)) + } + } +} From 4fa1e904faeeb778eeb332577f9ed8a38e1ee562 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:46:02 +0200 Subject: [PATCH 04/14] feat(parser): a dynamic symbol is a valid alias/undef name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `alias :"#{'a'}" value` and `undef :"#{'meth'}"` failed with `expected a method name`: parseFitem accepted only a SYMBOL, a GVAR or a bare fname. MRI's `fitem: fname | symbol` with `symbol: ssym | dsym` and `dsym: tSYMBEG string_contents tSTRING_END` (parse.y v3_4_0) makes an interpolated symbol an ordinary alias/undef name, whose value is only known at run time. The lexer already desugars `:"…#{…}…"` into `"…".to_sym`, so parseFitem now takes exactly that — the string concatenation plus the `.to_sym` call, and no further token, since the name after an alias's first item (`alias :"#{x}" v`) must not be read as a paren-less argument of to_sym. ast.Alias gains NewNameExpr/OldNameExpr and ast.Undef gains Exprs, all nil for the static forms, so the existing string fields keep their meaning and existing consumers keep building. Unblocks language/alias_spec.rb and language/undef_spec.rb, which now parse. Verified against MRI 4.0.5: `class C; def v; 5; end; alias :"#{:a}" v; end` answers 5 to `C.new.a`, and the undef form removes the method. Co-Authored-By: Claude Opus 5 --- alias_undef_test.go | 121 ++++++++++++++++++++++++++++++++++++++++++++ ast/ast.go | 21 ++++++-- parser.go | 54 ++++++++++++++++---- 3 files changed, 182 insertions(+), 14 deletions(-) diff --git a/alias_undef_test.go b/alias_undef_test.go index 9225f04..a9c4b20 100644 --- a/alias_undef_test.go +++ b/alias_undef_test.go @@ -81,3 +81,124 @@ func TestChainedMasgnRhs(t *testing.T) { } } } + +// TestDynamicSymbolFitem covers a dynamic symbol as an alias/undef name. MRI's +// `fitem: fname | symbol` and `symbol: ssym | dsym` (parse.y v3_4_0) admit +// `dsym: tSYMBEG string_contents tSTRING_END`, so `alias :"#{x}" y` names the +// new method by an expression evaluated at run time. The lexer desugars an +// interpolated symbol to `"…".to_sym`, which is the node that lands in +// Alias.NewNameExpr / Undef.Exprs; the matching string is left empty. +func TestDynamicSymbolFitem(t *testing.T) { + t.Parallel() + + t.Run("alias new name", func(t *testing.T) { + t.Parallel() + a := parseAlias(t, `alias :"#{'a'}" value`) + if a.NewName != "" || a.NewNameExpr == nil { + t.Errorf("NewName = %q, NewNameExpr = %v; want \"\" and a node", a.NewName, a.NewNameExpr) + } + if a.OldName != "value" || a.OldNameExpr != nil { + t.Errorf("OldName = %q, OldNameExpr = %v; want \"value\" and nil", a.OldName, a.OldNameExpr) + } + wantToSym(t, a.NewNameExpr) + }) + + t.Run("alias old name", func(t *testing.T) { + t.Parallel() + a := parseAlias(t, `alias a :"#{'value'}"`) + if a.NewName != "a" || a.NewNameExpr != nil { + t.Errorf("NewName = %q, NewNameExpr = %v; want \"a\" and nil", a.NewName, a.NewNameExpr) + } + if a.OldName != "" || a.OldNameExpr == nil { + t.Errorf("OldName = %q, OldNameExpr = %v; want \"\" and a node", a.OldName, a.OldNameExpr) + } + }) + + t.Run("alias both names", func(t *testing.T) { + t.Parallel() + a := parseAlias(t, `alias :"#{x}" :"#{y}"`) + if a.NewNameExpr == nil || a.OldNameExpr == nil { + t.Errorf("want both names dynamic, got %v / %v", a.NewNameExpr, a.OldNameExpr) + } + }) + + t.Run("static alias keeps Expr nil", func(t *testing.T) { + t.Parallel() + a := parseAlias(t, `alias foo bar`) + if a.NewNameExpr != nil || a.OldNameExpr != nil { + t.Errorf("static alias carries expressions: %v / %v", a.NewNameExpr, a.OldNameExpr) + } + }) + + t.Run("undef mixed list", func(t *testing.T) { + t.Parallel() + u := parseUndef(t, `undef :"#{a}", :b`) + if got := []string{"", "b"}; len(u.Names) != 2 || u.Names[0] != got[0] || u.Names[1] != got[1] { + t.Fatalf("Names = %q, want %q", u.Names, got) + } + if len(u.Exprs) != 2 || u.Exprs[0] == nil || u.Exprs[1] != nil { + t.Fatalf("Exprs = %v, want [node nil]", u.Exprs) + } + wantToSym(t, u.Exprs[0]) + }) + + t.Run("static undef leaves Exprs nil", func(t *testing.T) { + t.Parallel() + u := parseUndef(t, `undef a, b`) + if u.Exprs != nil { + t.Errorf("Exprs = %v, want nil", u.Exprs) + } + }) + + t.Run("a plain interpolated string is not a name", func(t *testing.T) { + t.Parallel() + if _, err := Parse(`alias "#{a}" b`); err == nil { + t.Error(`alias "#{a}" b: want a parse error, got none`) + } + }) +} + +func parseAlias(t *testing.T, src string) *ast.Alias { + t.Helper() + prog, err := Parse(src) + if err != nil { + t.Fatalf("Parse(%q) error: %v", src, err) + } + a, ok := prog.Body[0].(*ast.Alias) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.Alias", src, prog.Body[0]) + } + return a +} + +func parseUndef(t *testing.T, src string) *ast.Undef { + t.Helper() + prog, err := Parse(src) + if err != nil { + t.Fatalf("Parse(%q) error: %v", src, err) + } + u, ok := prog.Body[0].(*ast.Undef) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.Undef", src, prog.Body[0]) + } + return u +} + +// wantToSym asserts n is the `"…".to_sym` call the lexer desugars an +// interpolated symbol into. +func wantToSym(t *testing.T, n ast.Node) { + t.Helper() + call, ok := n.(*ast.Call) + if !ok { + t.Fatalf("dynamic name = %T, want *ast.Call", n) + } + if call.Name != "to_sym" { + t.Errorf("dynamic name calls %q, want %q", call.Name, "to_sym") + } + if call.Recv == nil { + t.Error("dynamic name has no receiver") + } + if len(call.Args) != 0 { + t.Errorf("dynamic name got %d arguments, want 0", len(call.Args)) + } +} diff --git a/ast/ast.go b/ast/ast.go index abcb267..2f9d287 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -414,14 +414,29 @@ type FindPattern struct { // Alias is `alias NewName OldName` — it makes NewName an alias of an existing // method (or global variable). Each name is the bare method/symbol/global text // without a leading colon. +// +// MRI's `fitem` admits a dynamic symbol (`alias :"#{x}" y`), whose name is only +// known at run time. Such a name arrives in NewNameExpr/OldNameExpr, with the +// matching string field empty; the expression evaluates to the symbol. Both Expr +// fields are nil for the ordinary all-static form. type Alias struct { - NewName string - OldName string + NewName string + OldName string + NewNameExpr Node + OldNameExpr Node } // Undef is `undef name [, name…]` — it removes the named method(s) from the // current class/module. Names hold the bare method names. -type Undef struct{ Names []string } +// +// A dynamic symbol (`undef :"#{x}"`) has an empty string at its position in +// Names and its expression at the same index of Exprs. Exprs is nil when every +// name is static, and otherwise has exactly len(Names) entries, nil at each +// static position. +type Undef struct { + Names []string + Exprs []Node +} // Retry restarts the enclosing begin body from inside a rescue clause. type Retry struct{} diff --git a/parser.go b/parser.go index 4caeeae..7bce1d9 100644 --- a/parser.go +++ b/parser.go @@ -965,32 +965,64 @@ func (p *Parser) parseReturn() ast.Node { // a global variable. The two names are separated by whitespace, not a comma. func (p *Parser) parseAlias() ast.Node { p.expect(token.ALIAS) - return &ast.Alias{NewName: p.parseFitem(), OldName: p.parseFitem()} + newName, newExpr := p.parseFitem() + oldName, oldExpr := p.parseFitem() + return &ast.Alias{NewName: newName, OldName: oldName, NewNameExpr: newExpr, OldNameExpr: oldExpr} } // parseUndef parses `undef name [, name…]`, removing the named methods. func (p *Parser) parseUndef() ast.Node { p.expect(token.UNDEF) - names := []string{p.parseFitem()} - for p.accept(token.COMMA) { - names = append(names, p.parseFitem()) + var names []string + var exprs []ast.Node + dynamic := false + for { + name, expr := p.parseFitem() + names = append(names, name) + exprs = append(exprs, expr) + dynamic = dynamic || expr != nil + if !p.accept(token.COMMA) { + break + } + } + if !dynamic { + return &ast.Undef{Names: names} } - return &ast.Undef{Names: names} + return &ast.Undef{Names: names, Exprs: exprs} } // parseFitem reads one method-name item for alias/undef: a symbol (`:foo`, -// `:==`), a global variable (`$x`, alias only), or a bare method name — an -// identifier, a constant, a reserved word, or an operator (`==`, `<=>`, `[]`). -func (p *Parser) parseFitem() string { +// `:==`), a global variable (`$x`, alias only), a bare method name — an +// identifier, a constant, a reserved word, or an operator (`==`, `<=>`, `[]`) — +// or a dynamic symbol (`:"#{x}"`), which is returned as an expression instead +// of a name. +// +// MRI: `fitem: fname | symbol` with `symbol: ssym | dsym` and +// `dsym: tSYMBEG string_contents tSTRING_END` (parse.y v3_4_0), so an +// interpolated symbol is an ordinary alias/undef name. The lexer has already +// desugared `:"…#{…}…"` into the equivalent `"…".to_sym`, which is why the item +// begins with a STRBEG here; a symbol with no interpolation is a plain SYMBOL. +func (p *Parser) parseFitem() (string, ast.Node) { switch p.cur().Type { case token.SYMBOL, token.GVAR: - return p.advance().Lit + return p.advance().Lit, nil + case token.STRBEG: + // Take exactly the desugared `"…".to_sym` and no more: the name that + // follows an alias's first item (`alias :"#{x}" v`) must not be read as a + // paren-less argument of to_sym. + str := p.parseStringConcat() + if !p.is(token.DOT) || p.peekTok().Lit != "to_sym" { + p.fail("expected a method name") + } + p.advance() // . + p.advance() // to_sym + return "", &ast.Call{Recv: str, Name: "to_sym"} } if name, ok := p.parseDefName(); ok { - return name + return name, nil } p.fail("expected a method name") - return "" + return "", nil } func (p *Parser) parseBreak() ast.Node { From 05f6a4c6628b6762def9360c41eccdf070300528 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:47:32 +0200 Subject: [PATCH 05/14] fix(lexer): `$=` is a global variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `p $=` failed with `unexpected token "$"`. MRI's parse_gvar (parse.y v3_4_0) lists `=` in the switch arm that lexes a single punctuation character as tGVAR — `_ ~ * $ ? ! @ / \ ; , . = : < > "` — with a second arm for the back-references `& ` ' +`. Every one of those was already accepted here except `=`, the obsolete ignore-case flag, which `language/predefined_spec.rb` reads. Unblocks language/predefined_spec.rb, which now parses. Verified against MRI 4.0.5: `p $=` prints false. Co-Authored-By: Claude Opus 5 --- lexer/lexer.go | 8 ++++++-- lexer/lexer_round3_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lexer/lexer.go b/lexer/lexer.go index 3e7d650..2ff9296 100644 --- a/lexer/lexer.go +++ b/lexer/lexer.go @@ -984,7 +984,11 @@ func (l *Lexer) lexGvar(spaceBefore bool, line, col int) token.Token { } case isSpecialGvar(c): // Single-character special globals: $~ $& $` $' $! $@ $/ $\ $; $, $. - // $< $> $? $* $$ $: $" $0 $+ (and the like). Each is exactly one byte. + // $< $> $? $* $$ $: $" $= $+ (and the like). Each is exactly one byte. + // This is MRI's parse_gvar switch (parse.y v3_4_0), whose tGVAR arm lists + // `_ ~ * $ ? ! @ / \ ; , . = : < > "` and whose tBACK_REF arm adds + // `& ` ' +`; `$=` (the obsolete ignore-case flag) belongs to it and was + // the one missing here, which made `p $=` a lexer error. l.advance() case c >= '1' && c <= '9': for l.peek() >= '0' && l.peek() <= '9' { @@ -2322,7 +2326,7 @@ func isIdentPart(c byte) bool { return isIdentStart(c) || isDigit(c) } func isSpecialGvar(c byte) bool { switch c { case '~', '&', '`', '\'', '!', '@', '/', '\\', ';', ',', '.', - '<', '>', '?', '*', '$', ':', '"', '+': + '<', '>', '?', '*', '$', ':', '"', '+', '=': return true } return false diff --git a/lexer/lexer_round3_test.go b/lexer/lexer_round3_test.go index beccf58..1a653fa 100644 --- a/lexer/lexer_round3_test.go +++ b/lexer/lexer_round3_test.go @@ -88,3 +88,19 @@ func TestPercentEqualsDelimitsAtExpressionBegin(t *testing.T) { } } } + +// TestIgnoreCaseGlobal covers `$=`, the obsolete ignore-case flag. MRI's +// parse_gvar (parse.y v3_4_0) lists `=` among the single-character globals that +// lex as tGVAR, alongside `_ ~ * $ ? ! @ / \ ; , . : < > "`. +func TestIgnoreCaseGlobal(t *testing.T) { + toks := New("p $=").Tokenize() + if toks[1].Type != token.GVAR || toks[1].Lit != "$=" { + t.Errorf("p $=: second token = %s %q, want GVAR %q", toks[1].Type, toks[1].Lit, "$=") + } + // An assignment to it still sees the following `=` as the operator. + toks = New("$= = false").Tokenize() + if toks[0].Type != token.GVAR || toks[0].Lit != "$=" || toks[1].Type != token.ASSIGN { + t.Errorf("$= = false: got %s %q then %s, want GVAR %q then ASSIGN", + toks[0].Type, toks[0].Lit, toks[1].Type, "$=") + } +} From c1df3373b29022c216a7b954206348505eedaab1 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:51:09 +0200 Subject: [PATCH 06/14] =?UTF-8?q?fix(parser):=20`defined=3F(=E2=80=A6)`=20?= =?UTF-8?q?takes=20an=20expr,=20and=20the=20jump=20keywords=20are=20primar?= =?UTF-8?q?ies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `defined?($zz and true)` failed with `expected ), got "and"`: `defined?` was parsed as an ordinary method call, so its argument was a call argument. MRI gives `defined?` two dedicated productions (parse.y v3_4_0): `keyword_defined '\n'? '(' begin_defined expr rparen` at 4458, an alternative of `primary`, and `keyword_defined '\n'? begin_defined arg` at 4039, an alternative of `arg`. The parenthesised form takes a full `expr` — which includes `expr keyword_and expr`, `expr keyword_or expr` and `keyword_not '\n'? expr` — but one expr only, not a statement sequence (`defined?($zz; $x)` is a syntax error in MRI), and only a hugging parenthesis selects it: MRI answers "expression" to `defined? (1) && nil` and nil to `defined?(1) && nil`, because a space leaves the parenthesis to open the `arg`. All four properties are pinned by tests. Reaching further into language/defined_spec.rb then needed the jump keywords in expression position. A bare `break`/`next`/`redo`/`retry` is an alternative of `primary` (4682-4697) and so is a bare `return` (`k_return`, 4438), while `k_return call_args` / `keyword_break call_args` / `keyword_next call_args` are alternatives of `command` (3518-3534); the parser had them only as statement forms. `defined?(break)`, `defined?(return)` and `defined?(break 1)` are the shapes ruby/spec uses. `redo` has no token here and still lexes as an identifier, which parses. Unblocks language/defined_spec.rb, which now parses. Verified against MRI 4.0.5. Co-Authored-By: Claude Opus 5 --- defined_expr_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++ parser.go | 34 ++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 defined_expr_test.go diff --git a/defined_expr_test.go b/defined_expr_test.go new file mode 100644 index 0000000..cdefe3e --- /dev/null +++ b/defined_expr_test.go @@ -0,0 +1,93 @@ +package parser_test + +import ( + "fmt" + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// TestDefinedTakesAnExpr covers MRI's dedicated `defined?` productions +// (parse.y v3_4_0): `keyword_defined '\n'? '(' begin_defined expr rparen` is an +// alternative of `primary`, and `keyword_defined '\n'? begin_defined arg` an +// alternative of `arg`. The parenthesised form takes a full `expr`, so the +// low-precedence keyword operators are allowed inside it — unlike an ordinary +// call argument. +func TestDefinedTakesAnExpr(t *testing.T) { + t.Parallel() + for _, src := range []string{ + `defined?($zz and true)`, + `defined?($zz or true)`, + `defined?(not $zz)`, + `defined?(a and b or c)`, + "defined?(\n $zz and true\n)", + } { + n := parseOne(t, src) + call, ok := n.(*ast.Call) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.Call", src, n) + } + if call.Name != "defined?" || len(call.Args) != 1 { + t.Errorf("Parse(%q): call %q with %d args, want defined? with 1", src, call.Name, len(call.Args)) + } + } +} + +// TestDefinedParenMustHug pins the other side of the same pair of rules: only a +// hugging parenthesis selects the `primary` production. With a space the +// parenthesis opens the `arg` instead, so MRI answers "expression" to +// `defined? (1) && nil` but nil to `defined?(1) && nil`. +func TestDefinedParenMustHug(t *testing.T) { + t.Parallel() + hug := parseOne(t, `defined?(1) && nil`) + if _, ok := hug.(*ast.BinaryExpr); !ok { + t.Errorf(`defined?(1) && nil: node = %T, want *ast.BinaryExpr`, hug) + } + spaced := parseOne(t, `defined? (1) && nil`) + call, ok := spaced.(*ast.Call) + if !ok { + t.Fatalf(`defined? (1) && nil: node = %T, want *ast.Call`, spaced) + } + if call.Name != "defined?" { + t.Errorf(`defined? (1) && nil: call %q, want defined?`, call.Name) + } + if _, ok := call.Args[0].(*ast.BinaryExpr); !ok { + t.Errorf(`defined? (1) && nil: argument = %T, want *ast.BinaryExpr`, call.Args[0]) + } + if _, err := parser.Parse(`defined?($zz; $x)`); err == nil { + t.Error(`defined?($zz; $x): want a parse error (MRI takes one expr, not a sequence)`) + } +} + +// TestJumpKeywordsArePrimaries covers the bare `break`/`next`/`retry`/`return` +// alternatives of MRI's `primary` (parse.y v3_4_0: `k_return` at 4438, +// `keyword_break`/`keyword_next`/`keyword_redo`/`keyword_retry` at 4682-4697), +// and the `command` forms that carry arguments. `defined?(break)` is what +// ruby/spec uses to reach them. +func TestJumpKeywordsArePrimaries(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + src string + want any + }{ + {`defined?(break)`, (*ast.Break)(nil)}, + {`defined?(next)`, (*ast.Next)(nil)}, + {`defined?(retry)`, (*ast.Retry)(nil)}, + {`defined?(return)`, (*ast.Return)(nil)}, + {`defined?(break 1)`, (*ast.Break)(nil)}, + {`defined?(next 1)`, (*ast.Next)(nil)}, + {`defined?(return 1)`, (*ast.Return)(nil)}, + } { + n := parseOne(t, tc.src) + call, ok := n.(*ast.Call) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.Call", tc.src, n) + } + if got, want := nodeType(call.Args[0]), nodeType(tc.want); got != want { + t.Errorf("Parse(%q): argument = %s, want %s", tc.src, got, want) + } + } +} + +func nodeType(n any) string { return fmt.Sprintf("%T", n) } diff --git a/parser.go b/parser.go index 7bce1d9..a72e266 100644 --- a/parser.go +++ b/parser.go @@ -3032,6 +3032,21 @@ func (p *Parser) parsePrimary() ast.Node { case token.MODULE: // A module definition as an rvalue (`m = module M; …; end`). return p.parseModule() + case token.BREAK: + // A bare `break`/`next`/`retry`/`return` is an alternative of `primary` in + // MRI (parse.y v3_4_0: `k_return` at 4438, `keyword_break`/`keyword_next`/ + // `keyword_redo`/`keyword_retry` at 4682-4697), and `k_return call_args` / + // `keyword_break call_args` / `keyword_next call_args` are alternatives of + // `command`, so both the bare and the valued forms belong in expression + // position: `defined?(break)` and `defined?(break 1)` are both legal. + return p.parseBreak() + case token.NEXT: + return p.parseNext() + case token.RETRY: + p.advance() + return &ast.Retry{} + case token.RETURN: + return p.parseReturn() case token.BEGIN: return p.parseBegin() case token.CASE: @@ -3072,6 +3087,25 @@ func (p *Parser) parseIdentExpr() ast.Node { name := p.cur().Lit next := p.peekTok() + // `defined?` is a keyword, not a method. MRI gives it two productions + // (parse.y v3_4_0): `keyword_defined '\n'? '(' begin_defined expr rparen`, + // an alternative of `primary`, and `keyword_defined '\n'? begin_defined arg`, + // an alternative of `arg`. The parenthesised form takes a full `expr`, which + // admits the low-precedence keyword operators — `defined?($x and true)` is + // legal where an ordinary call argument is not. It takes ONE expr, not a + // statement sequence (`defined?($x; $y)` is a syntax error in MRI), and only + // a hugging paren selects it: `defined? (1) && nil` answers "expression", + // because there the parenthesis opens the `arg` instead. + if name == "defined?" && next.Type == token.LPAREN && !next.SpaceBefore { + p.advance() // defined? + p.advance() // ( + p.skipNewlines() + arg := p.parseKeywordLogical() + p.skipNewlines() + p.expect(token.RPAREN) + return &ast.Call{Name: name, Args: []ast.Node{arg}} + } + // foo(...) — paren call (the '(' must hug the name). if next.Type == token.LPAREN && !next.SpaceBefore { p.advance() // name From e20aa9a03aa4bc596108f543e8d2594af072d590 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:53:51 +0200 Subject: [PATCH 07/14] fix(lexer): `:a=` keeps its `=` when the next `=` opens a `=>` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `{:a==>1}` failed with `unexpected token ">"`: the lexer refused the trailing `=` of a setter symbol whenever another `=` followed, so `:a==>1` lexed as `:a`, `==`, `>`. MRI's parse_ident (parse.y v3_4_0) is finer: c == '=' && IS_lex_state(EXPR_FNAME) && (!peek(p,'~') && !peek(p,'>') && (!peek(p,'=') || peek_n(p,'>',1))) The `=` joins the name unless it opens `=~` or `=>`, or unless it opens `==` that is NOT itself followed by `>`. That last clause is the one that was missing, and it is what makes `{:a==>1}` the pair `:a= => 1` while `{:a ==> 1}` — where the symbol has already ended at the space — stays a syntax error, as language/hash_spec.rb requires of both. Unblocks language/hash_spec.rb, which now parses. Verified against MRI 4.0.5: `p({:a==>1})` prints {"a=": 1}. Co-Authored-By: Claude Opus 5 --- lexer/lexer.go | 19 +++++++++++++++++-- lexer/lexer_round3_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lexer/lexer.go b/lexer/lexer.go index 2ff9296..7aadae6 100644 --- a/lexer/lexer.go +++ b/lexer/lexer.go @@ -91,6 +91,13 @@ func (l *Lexer) peek2() byte { return l.src[l.pos+1] } +func (l *Lexer) peek3() byte { + if l.pos+2 >= len(l.src) { + return 0 + } + return l.src[l.pos+2] +} + func (l *Lexer) advance() byte { c := l.src[l.pos] l.pos++ @@ -897,8 +904,16 @@ func (l *Lexer) lexSymbol(spaceBefore bool, line, col int) token.Token { switch c := l.peek(); { case c == '?' || c == '!': // :empty?, :save! l.advance() - case c == '=' && l.peek2() != '=' && l.peek2() != '~' && l.peek2() != '>': - l.advance() // setter symbol :name= (but not :foo== / :foo=~ / :foo=>) + case c == '=' && l.peek2() != '~' && l.peek2() != '>' && + (l.peek2() != '=' || l.peek3() == '>'): + // Setter symbol `:name=`, but not `:foo=~` / `:foo=>` / `:foo==`. MRI's + // parse_ident (parse.y v3_4_0) takes the `=` into the name when, in + // EXPR_FNAME, the byte after it is neither `~` nor `>` and is either not + // `=` or is an `=` followed by `>`: + // `(!peek(p,'~') && !peek(p,'>') && (!peek(p,'=') || peek_n(p,'>',1)))` + // That last clause is what makes `{:a==>1}` the pair `:a= => 1`, while + // `{:a ==> 1}` (where the `:a` has already ended) is a syntax error. + l.advance() } l.state = exprEnd return token.Token{Type: token.SYMBOL, Lit: string(l.src[start:l.pos]), Line: line, Col: col, SpaceBefore: spaceBefore} diff --git a/lexer/lexer_round3_test.go b/lexer/lexer_round3_test.go index 1a653fa..d2df1ab 100644 --- a/lexer/lexer_round3_test.go +++ b/lexer/lexer_round3_test.go @@ -104,3 +104,36 @@ func TestIgnoreCaseGlobal(t *testing.T) { toks[0].Type, toks[0].Lit, toks[1].Type, "$=") } } + +// TestSetterSymbolBeforeRocket covers MRI's parse_ident rule for a trailing `=` +// in a symbol name (parse.y v3_4_0): +// +// c == '=' && IS_lex_state(EXPR_FNAME) && +// (!peek(p,'~') && !peek(p,'>') && (!peek(p,'=') || peek_n(p,'>',1))) +// +// so `{:a==>1}` is `{:a= => 1}` — the `=` joins the name precisely because the +// `=` after it is followed by `>`. +func TestSetterSymbolBeforeRocket(t *testing.T) { + for _, src := range []string{`{:a==>1}`, `{:a= =>1}`, `{:a= => 1}`} { + toks := New(src).Tokenize() + if toks[1].Type != token.SYMBOL || toks[1].Lit != "a=" { + t.Errorf("%s: second token = %s %q, want SYMBOL %q", src, toks[1].Type, toks[1].Lit, "a=") + } + if toks[2].Type != token.HASHROCKET { + t.Errorf("%s: third token = %s, want ROCKET", src, toks[2].Type) + } + } + // The `=` must still not be taken when it opens `==`, `=~` or `=>`. + for _, tc := range []struct{ src, want string }{ + {`:a == 1`, "a"}, + {`:a =~ /x/`, "a"}, + {`:a => 1`, "a"}, + {`:a = 1`, "a"}, + {`:a= 1`, "a="}, + } { + toks := New(tc.src).Tokenize() + if toks[0].Type != token.SYMBOL || toks[0].Lit != tc.want { + t.Errorf("%s: first token = %s %q, want SYMBOL %q", tc.src, toks[0].Type, toks[0].Lit, tc.want) + } + } +} From 60303c513f4fe955050bbd0221875bc1542840b6 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:54:15 +0200 Subject: [PATCH 08/14] test(lexer): cover peek3's end-of-input branch The 100% gate reads the ROUNDED total, which stayed at 100.0% while peek3's out-of-range return had never run. `:a==` at end of input is the source that reaches it. Co-Authored-By: Claude Opus 5 --- lexer/lexer_round3_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/lexer/lexer_round3_test.go b/lexer/lexer_round3_test.go index d2df1ab..3745590 100644 --- a/lexer/lexer_round3_test.go +++ b/lexer/lexer_round3_test.go @@ -125,6 +125,7 @@ func TestSetterSymbolBeforeRocket(t *testing.T) { } // The `=` must still not be taken when it opens `==`, `=~` or `=>`. for _, tc := range []struct{ src, want string }{ + {`:a==`, "a"}, // end of input right after the `==` {`:a == 1`, "a"}, {`:a =~ /x/`, "a"}, {`:a => 1`, "a"}, From 9b7c51b42912985ce326b5e68cf86aa96cb6aabb Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 21:57:19 +0200 Subject: [PATCH 09/14] fix(parser): a `{` at the lambda's own nesting opens its body, not a block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `-> a=a() { a }` failed with `expected {, got "\n"`: the brace was attached to the `a()` call in the parameter default, leaving nothing to open the lambda body. MRI decides this in the lexer, by bracket nesting. At a `->` it records `p->lex.lpar_beg = p->lex.paren_nest`, and a later `{` returns tLAMBEG — the lambda body — exactly when `lambda_beginning_p()` (`p->lex.lpar_beg == p->lex.paren_nest`) holds (parse.y v3_4_0). paren_nest counts `(`, `[` and `{`, so the rule is: while the unparenthesised parameter list is open and no bracket is, the next `{` is the body. The parser now carries noBraceBlock, set over an unparenthesised stabby-lambda parameter list and cleared by enterBracket wherever a bracket is opened (a parenthesised group, a `(…)`/`[…]` argument list, a parenthesised parameter list). That reproduces both sides: `-> x = ([1].map { |v| v }) { x }` keeps its inner block, while the same expression without the parentheses is a syntax error in MRI too, which is why the flag is not cleared for a bracket that has already closed. Unblocks language/lambda_spec.rb, which now parses. Verified against MRI 4.0.5: `def a; 1; end; p(-> a=a() { a }.call)` prints 1. Co-Authored-By: Claude Opus 5 --- lambda_paren_block_test.go | 76 ++++++++++++++++++++++++++++++++++++++ parser.go | 39 +++++++++++++++++-- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/lambda_paren_block_test.go b/lambda_paren_block_test.go index 081e33d..9fed509 100644 --- a/lambda_paren_block_test.go +++ b/lambda_paren_block_test.go @@ -106,3 +106,79 @@ func TestParenGroupShape(t *testing.T) { t.Fatalf("expected *ast.IntLit for (1), got %T", prog.Body[0]) } } + +// TestLambdaBodyBraceAfterUnparenthesisedParams covers the `{` that closes an +// unparenthesised stabby-lambda parameter list. MRI decides this in the lexer: +// `p->lex.lpar_beg = p->lex.paren_nest` is recorded at the `->`, and a later `{` +// is tLAMBEG — the lambda body — exactly when `lambda_beginning_p()` +// (`p->lex.lpar_beg == p->lex.paren_nest`) holds (parse.y v3_4_0). So in +// `-> a=a() { a }` the brace opens the body, not a block on the `a()` call; +// inside a still-open bracket it is an ordinary block again. +func TestLambdaBodyBraceAfterUnparenthesisedParams(t *testing.T) { + t.Parallel() + + t.Run("brace closes the parameter list", func(t *testing.T) { + t.Parallel() + src := `-> a=a() { a }` + n := lambdaNode(t, src) + call, ok := n.(*ast.Call) + if !ok || call.Name != "lambda" || call.Block == nil { + t.Fatalf("Parse(%q): node = %T, want the lambda call with a block", src, n) + } + if len(call.Block.Params) != 1 || call.Block.Params[0] != "a" { + t.Errorf("Parse(%q): params = %q, want [a]", src, call.Block.Params) + } + if len(call.Block.Defaults) != 1 { + t.Fatalf("Parse(%q): %d defaults, want 1", src, len(call.Block.Defaults)) + } + def, ok := call.Block.Defaults[0].(*ast.Call) + if !ok { + t.Fatalf("Parse(%q): default = %T, want *ast.Call", src, call.Block.Defaults[0]) + } + if def.Block != nil { + t.Errorf("Parse(%q): the brace was attached to the default's call as a block", src) + } + if len(call.Block.Body) != 1 { + t.Errorf("Parse(%q): %d body statements, want 1", src, len(call.Block.Body)) + } + }) + + t.Run("an open bracket restores ordinary blocks", func(t *testing.T) { + t.Parallel() + for _, src := range []string{ + `-> x = ([1].map { |v| v }) { x }`, // parenthesised group + `-> x = f([1].map { |v| v }) { x }`, // parenthesised argument list + `->(a = [1].map { |v| v }) { a }`, // parenthesised parameter list + } { + call, ok := lambdaNode(t, src).(*ast.Call) + if !ok || call.Block == nil { + t.Fatalf("Parse(%q): want the lambda call with a block", src) + } + if len(call.Block.Body) != 1 { + t.Errorf("Parse(%q): %d body statements, want 1", src, len(call.Block.Body)) + } + } + }) + + t.Run("blocks elsewhere are unaffected", func(t *testing.T) { + t.Parallel() + for _, src := range []string{`foo.map { |x| x }`, `[1].each { |x| x }`, `->x { x }`} { + if _, err := Parse(src); err != nil { + t.Errorf("Parse(%q) returned error: %v", src, err) + } + } + }) +} + +// lambdaNode parses src and returns its single top-level statement. +func lambdaNode(t *testing.T, src string) ast.Node { + t.Helper() + prog, err := Parse(src) + if err != nil { + t.Fatalf("Parse(%q) returned error: %v", src, err) + } + if len(prog.Body) != 1 { + t.Fatalf("Parse(%q): want 1 statement, got %d", src, len(prog.Body)) + } + return prog.Body[0] +} diff --git a/parser.go b/parser.go index a72e266..5280fa5 100644 --- a/parser.go +++ b/parser.go @@ -118,6 +118,18 @@ type Parser struct { // nested delimited context (a parenthesised group, a `(…)`/`[…]` argument list, // a `{…}` hash), where an inner modifier-`rescue` is again allowed. noRescueMod bool + // noBraceBlock suppresses `{ … }` block attachment while parsing the + // unparenthesised parameter list of a stabby lambda: there the next `{` at + // the lambda's own bracket nesting opens the lambda BODY, so `-> a=a() { a }` + // is a lambda whose body is `a`, not a call `a() { a }`. MRI decides this in + // the lexer, by comparing the bracket nesting against the one recorded at the + // `->` (parse.y v3_4_0: `lambda_beginning_p()` is + // `p->lex.lpar_beg == p->lex.paren_nest`, with `p->lex.lpar_beg = + // p->lex.paren_nest` at the `->`, and paren_nest counting `(`, `[` and `{`). + // enterBracket clears it inside any still-open bracket, which is why + // `-> x = ([1].map { |v| v }) { x }` keeps its inner block while the same + // expression without the parentheses is a syntax error in MRI too. + noBraceBlock bool // bracketDepth > 0 while parsing a parenthesised call-argument list or a hash // literal, where newlines are insignificant. A `key:` whose value sits on the // next line (`f(`⏎` k:`⏎` v)`) is then a continued pair, not a value-omitted @@ -206,6 +218,16 @@ func (p *Parser) fail(format string, args ...any) ast.Node { panic(parseError{msg: fmt.Sprintf("parse error at line %d: %s", t.Line, fmt.Sprintf(format, args...))}) } +// enterBracket marks the parser as being inside a still-open bracket and +// returns the function restoring the previous state; use it as +// `defer p.enterBracket()()`. Only noBraceBlock depends on bracket nesting: a +// `{` inside a bracket is an ordinary block again, never a lambda body. +func (p *Parser) enterBracket() func() { + saved := p.noBraceBlock + p.noBraceBlock = false + return func() { p.noBraceBlock = saved } +} + func (p *Parser) skipNewlines() { for p.is(token.NEWLINE) { p.advance() @@ -2459,7 +2481,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { args := p.parseCallArgs(token.RBRACKET) p.expect(token.RBRACKET) node = &ast.Call{Recv: node, Name: "[]", Args: args} - case p.is(token.LBRACE) || (p.is(token.DO) && !p.noDo): + case (p.is(token.LBRACE) && !p.noBraceBlock) || (p.is(token.DO) && !p.noDo): // A block binds to the immediately preceding method call; chaining // then continues (`recv.map { … }.join`). A `super` also takes a block // (`super { … }`, `super(x) do … end`). @@ -2590,15 +2612,23 @@ func (p *Parser) parseLambda() ast.Node { splat := -1 if p.accept(token.LPAREN) { // The stabby-lambda `(...)` list shares the block parameter grammar, so it - // supports the same optional, *splat, &block, and destructuring forms. + // supports the same optional, *splat, &block, and destructuring forms. The + // parenthesis is an open bracket, so a `{` inside is an ordinary block. + restore := p.enterBracket() params, defaults, prepends, splat, blockParam = p.parseBlockParams(token.RPAREN) + restore() p.expect(token.RPAREN) p.scope().explicitParams = true } else if p.is(token.IDENT) || p.is(token.STAR) || p.is(token.POW) || p.is(token.AMPER) { // Unparenthesized parameters: `->x { }`, `-> ctx { }`, `->a, b { }`, // `-> message do … end`. The list runs up to the block opener (`{`/`do`); - // parseBlockParams stops at the first token that is not a parameter. + // parseBlockParams stops at the first token that is not a parameter. A `{` + // reached at this nesting is the lambda body, not a block on whatever the + // last default expression called: `-> a=a() { a }` (see noBraceBlock). + savedBrace := p.noBraceBlock + p.noBraceBlock = true params, defaults, prepends, splat, blockParam = p.parseBlockParams(token.LBRACE) + p.noBraceBlock = savedBrace p.scope().explicitParams = true } bs := p.scope() @@ -2980,7 +3010,9 @@ func (p *Parser) parsePrimary() ast.Node { // semicolon/newline-separated statements (`(a; b)`), evaluating to the // last. A single statement returns directly; a sequence is wrapped in a // Begin (whose value is its last expression). + restore := p.enterBracket() stmts := p.parseStatements(map[token.Type]bool{token.RPAREN: true}) + restore() p.expect(token.RPAREN) if len(stmts) == 1 { return stmts[0] @@ -3325,6 +3357,7 @@ func (p *Parser) parseCallArgs(until token.Type) []ast.Node { // less parseCommandArgs and def parameter-default handling. savedMasgn := p.noMasgn p.noMasgn = true + defer p.enterBracket()() defer func() { p.bracketDepth--; p.noRescueMod = savedRescue; p.noMasgn = savedMasgn }() var args []ast.Node var kw *ast.HashLit From 1ba2c4358eda3b4edb4455ff5361356ce3c336f6 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 22:05:53 +0200 Subject: [PATCH 10/14] fix(parser): a space before an argument list does not detach a brace block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults, both found by language/block_spec.rb, which now parses. 1. `o.s (:a){ 1 }` bound the `{ … }` to the argument instead of to the call, silently — rbgo then raised LocalJumpError where MRI prints 1. In MRI a SPACED `(` in argument position lexes as tLPAREN_ARG and its `)` sets EXPR_ENDARG (parse.y v3_4_0: `primary: tLPAREN_ARG compstmt {SET_LEX_STATE(EXPR_ENDARG);} ')'`), so the `{` right after it lexes as tLBRACE_ARG and the grammar hands it to the command through `command: primary_value call_op operation2 command_args cmd_brace_block` (3464-3480). The parser now records where that `)` is (argParenEnd) and attaches the brace block to the command instead of letting the postfix chain take it. A HUGGING paren is unaffected — its `)` leaves EXPR_END, which is why MRI gives the block in `f g(1) { 2 }` to `g(1)` and raises from `f`; that negative case is pinned too. 2. `|a, (b, (c, d))|` failed with `expected IDENT, got "("`. A destructuring block parameter nests in MRI — `f_marg: f_norm_arg | tLPAREN f_margs rparen` puts f_marg back inside f_margs — but parseBlockParams carried a flat copy of the logic that could only read identifiers one level down. It now calls parseDestructureParam, the recursive implementation the `def` parameter list already used, which also makes `|((a, b), c)|` and `|(a, *b)| ` parse. Verified against MRI 4.0.5, values compared through rbgo for each case. Co-Authored-By: Claude Opus 5 --- command_brace_block_test.go | 95 +++++++++++++++++++++++++++++++++++++ parser.go | 86 ++++++++++++++++++++++++--------- 2 files changed, 158 insertions(+), 23 deletions(-) create mode 100644 command_brace_block_test.go diff --git a/command_brace_block_test.go b/command_brace_block_test.go new file mode 100644 index 0000000..0564280 --- /dev/null +++ b/command_brace_block_test.go @@ -0,0 +1,95 @@ +package parser_test + +import ( + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// parseAny parses src and reports only whether it was accepted. +func parseAny(src string) (*ast.Program, error) { return parser.Parse(src) } + +// TestBraceBlockAfterSpacedParenArgument covers MRI's tLPAREN_ARG/EXPR_ENDARG +// pair. A SPACED `(` in argument position lexes as tLPAREN_ARG, and its `)` +// sets EXPR_ENDARG (parse.y v3_4_0: `primary: tLPAREN_ARG compstmt +// {SET_LEX_STATE(EXPR_ENDARG);} ')'`), so the `{` right after it lexes as +// tLBRACE_ARG and the grammar gives it to the COMMAND through +// `command: primary_value call_op operation2 command_args cmd_brace_block`. +// A space before an argument list therefore does NOT detach a following brace +// block from the call. +func TestBraceBlockAfterSpacedParenArgument(t *testing.T) { + t.Parallel() + t.Run("receiver command", func(t *testing.T) { + t.Parallel() + src := `o.s (:a){ 1 }` + call, ok := parseOne(t, src).(*ast.Call) + if !ok || call.Name != "s" { + t.Fatalf("Parse(%q): want a call to s", src) + } + if call.Block == nil { + t.Fatalf("Parse(%q): the brace block did not reach the call", src) + } + if len(call.Args) != 1 { + t.Errorf("Parse(%q): %d args, want 1", src, len(call.Args)) + } + if _, ok := call.Args[0].(*ast.SymbolLit); !ok { + t.Errorf("Parse(%q): argument = %T, want *ast.SymbolLit", src, call.Args[0]) + } + }) + + t.Run("self command", func(t *testing.T) { + t.Parallel() + src := `f (1){ 2 }` + call, ok := parseOne(t, src).(*ast.Call) + if !ok || call.Name != "f" || call.Block == nil { + t.Fatalf("Parse(%q): want a call to f carrying a block", src) + } + }) + + t.Run("a hugging paren keeps the old binding", func(t *testing.T) { + t.Parallel() + // `f g(1) { 2 }`: g's paren hugs, so its `)` leaves EXPR_END, not + // EXPR_ENDARG — MRI gives the block to g(1) and f raises LocalJumpError. + src := `f g(1) { 2 }` + call, ok := parseOne(t, src).(*ast.Call) + if !ok || call.Name != "f" { + t.Fatalf("Parse(%q): want a call to f", src) + } + if call.Block != nil { + t.Errorf("Parse(%q): the block reached f; MRI gives it to g(1)", src) + } + inner, ok := call.Args[0].(*ast.Call) + if !ok || inner.Name != "g" || inner.Block == nil { + t.Errorf("Parse(%q): the block did not reach g(1)", src) + } + }) + + t.Run("ordinary blocks are unaffected", func(t *testing.T) { + t.Parallel() + for _, src := range []string{`[1].map { |x| x }`, `f(1) { 2 }`, `f 1 { 2 }`} { + if _, err := parseAny(src); err != nil { + t.Errorf("Parse(%q) returned error: %v", src, err) + } + } + }) +} + +// TestNestedDestructuringBlockParameter covers MRI's +// `f_marg: f_norm_arg | tLPAREN f_margs rparen` (parse.y v3_4_0): a +// destructuring block parameter contains f_margs, which contains f_marg again, +// so the parentheses nest to any depth and may also be the outermost parameter. +func TestNestedDestructuringBlockParameter(t *testing.T) { + t.Parallel() + for _, src := range []string{ + `f { |a, (b, c)| }`, + `f { |a, (b, (c, d))| }`, + `f { |((a, b), c)| }`, + `f { |(a, *b)| }`, + `->(a, (b, (c, d))) { }`, + } { + if _, err := parseAny(src); err != nil { + t.Errorf("Parse(%q) returned error: %v", src, err) + } + } +} diff --git a/parser.go b/parser.go index 5280fa5..e442454 100644 --- a/parser.go +++ b/parser.go @@ -130,6 +130,17 @@ type Parser struct { // `-> x = ([1].map { |v| v }) { x }` keeps its inner block while the same // expression without the parentheses is a syntax error in MRI too. noBraceBlock bool + // argParenEnd is the token index just past the `)` that closed a SPACED + // parenthesised group standing as the FIRST token of a paren-less command + // argument list — MRI's tLPAREN_ARG, whose `)` sets EXPR_ENDARG (parse.y + // v3_4_0: `primary: tLPAREN_ARG compstmt {SET_LEX_STATE(EXPR_ENDARG);} ')'`). + // A `{` reached in that state lexes as tLBRACE_ARG, and the grammar gives it + // to the COMMAND through `command: primary_value call_op operation2 + // command_args cmd_brace_block` (3464-3480), not to the group. So + // `o.s (:a){ 1 }` yields the block to `s`, while `f g(1) { 2 }` — a hugging + // paren, hence EXPR_END — yields it to `g(1)`. It is an absolute index, so a + // stale value is inert: the cursor only moves forward. + argParenEnd int // bracketDepth > 0 while parsing a parenthesised call-argument list or a hash // literal, where newlines are insignificant. A `key:` whose value sits on the // next line (`f(`⏎` k:`⏎` v)`) is then a continued pair, not a value-omitted @@ -147,7 +158,7 @@ var parseHook func() // parse error rather than propagating to the caller. func Parse(src string) (prog *ast.Program, err error) { toks := lexer.New(src).Tokenize() - p := &Parser{toks: toks, scopes: []*scope{newScope(true)}} + p := &Parser{toks: toks, scopes: []*scope{newScope(true)}, argParenEnd: -1} defer func() { if r := recover(); r != nil { if pe, ok := r.(parseError); ok { @@ -222,6 +233,21 @@ func (p *Parser) fail(format string, args ...any) ast.Node { // returns the function restoring the previous state; use it as // `defer p.enterBracket()()`. Only noBraceBlock depends on bracket nesting: a // `{` inside a bracket is an ordinary block again, never a lambda body. +// atCommandBrace reports whether the cursor sits on the `{` that MRI lexes as +// tLBRACE_ARG: the one immediately after the `)` recorded in argParenEnd. Such a +// brace opens the command's block, so no inner call may take it. +func (p *Parser) atCommandBrace() bool { + return p.is(token.LBRACE) && p.pos == p.argParenEnd +} + +// attachCommandBrace gives that `{ … }` to the command call being built, which +// is what `command_args cmd_brace_block` does in MRI's grammar. +func (p *Parser) attachCommandBrace(block **ast.Block) { + if *block == nil && p.atCommandBrace() { + *block = p.parseBraceBlock() + } +} + func (p *Parser) enterBracket() func() { saved := p.noBraceBlock p.noBraceBlock = false @@ -2417,6 +2443,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { // as MRI attaches it to the outermost command rather than to an // argument call. call := &ast.Call{Recv: node, Name: name, Args: p.parseCommandArgs(), Safe: safe} + p.attachCommandBrace(&call.Block) if p.is(token.DO) && !p.noDo { call.Block = p.parseDoBlock() // The block-bearing command call may itself be chained @@ -2448,6 +2475,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { // scoped constant (`Math::PI`). if p.canStartCommandArg() || p.atHuggingStringArg() { call := &ast.Call{Recv: node, Name: name, Args: p.parseCommandArgs()} + p.attachCommandBrace(&call.Block) if p.is(token.DO) && !p.noDo { call.Block = p.parseDoBlock() node = call @@ -2468,6 +2496,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { } else if p.canStartCommandArg() || p.atHuggingStringArg() { // Paren-less scope-resolution command call: `Mod::meth arg`. call := &ast.Call{Recv: node, Name: name, Args: p.parseCommandArgs()} + p.attachCommandBrace(&call.Block) if p.is(token.DO) && !p.noDo { call.Block = p.parseDoBlock() node = call @@ -2481,7 +2510,8 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { args := p.parseCallArgs(token.RBRACKET) p.expect(token.RBRACKET) node = &ast.Call{Recv: node, Name: "[]", Args: args} - case (p.is(token.LBRACE) && !p.noBraceBlock) || (p.is(token.DO) && !p.noDo): + case (p.is(token.LBRACE) && !p.noBraceBlock && !p.atCommandBrace()) || + (p.is(token.DO) && !p.noDo): // A block binds to the immediately preceding method call; chaining // then continues (`recv.map { … }.join`). A `super` also takes a block // (`super { … }`, `super(x) do … end`). @@ -2820,27 +2850,22 @@ func (p *Parser) parseBlockParams(until token.Type) (names []string, defaults, p names = append(names, "*") } defaults = append(defaults, nil) - } else if p.accept(token.LPAREN) { - var gnames []string - gsplat := -1 - for { - if p.accept(token.STAR) { - gsplat = len(gnames) - } - gn := p.expect(token.IDENT).Lit - gnames = append(gnames, gn) - p.declareLocal(gn) - if !p.accept(token.COMMA) { - break - } - } - p.expect(token.RPAREN) - syn := "(" + strconv.Itoa(group) + ")" - group++ - names = append(names, syn) + } else if p.is(token.LPAREN) { + // A parenthesised destructuring parameter, which nests: MRI's + // `f_marg: f_norm_arg | tLPAREN f_margs rparen` (parse.y v3_4_0) puts + // f_marg back inside f_margs, so `|a, (b, (c, d))|` is as legal as + // `|a, (b, c)|`. parseDestructureParam is the shared implementation — + // the `def` parameter list already used it; this list had a flat copy + // that could only read identifiers one level down. + outer, chained := p.parseDestructureParam(&group) + names = append(names, outer.Values[0].(*ast.VarRef).Name) defaults = append(defaults, nil) - p.declareLocal(syn) - prepends = append(prepends, &ast.MultiAssign{Names: gnames, SplatIndex: gsplat, Values: []ast.Node{&ast.VarRef{Name: syn}}}) + // The outer unpack binds the inner synthetics first; the chained inner + // unpacks then read them. + prepends = append(prepends, outer) + for _, c := range chained { + prepends = append(prepends, c) + } } else { name := p.expect(token.IDENT).Lit names = append(names, name) @@ -3154,6 +3179,7 @@ func (p *Parser) parseIdentExpr() ast.Node { if p.huggingStringArg() { p.advance() // name call := &ast.Call{Name: name, Args: p.parseCommandArgs()} + p.attachCommandBrace(&call.Block) if p.is(token.DO) && !p.noDo { call.Block = p.parseDoBlock() } @@ -3172,6 +3198,7 @@ func (p *Parser) parseIdentExpr() ast.Node { } if p.localCommandArgFollows() { call := &ast.Call{Name: name, Args: p.parseCommandArgs()} + p.attachCommandBrace(&call.Block) if p.is(token.DO) && !p.noDo { call.Block = p.parseDoBlock() } @@ -3194,7 +3221,9 @@ func (p *Parser) parseIdentExpr() ast.Node { // Otherwise it is a method call on self. p.advance() if p.canStartCommandArg() { - return &ast.Call{Name: name, Args: p.parseCommandArgs()} + call := &ast.Call{Name: name, Args: p.parseCommandArgs()} + p.attachCommandBrace(&call.Block) + return call } // Bare `it` (no receiver, no args) inside a param-less block is the implicit // single parameter (Ruby 3.4). With args/parens — or a block, as in RSpec's @@ -3309,6 +3338,17 @@ func (p *Parser) canStartCommandArg() bool { // keyword/hash-pair forms a parenthesized call does, collapsing trailing // `key: value` / `expr => value` pairs into one implicit Hash argument. func (p *Parser) parseCommandArgs() []ast.Node { + // A leading SPACED `(` opens MRI's tLPAREN_ARG group, whose `)` puts the + // lexer in EXPR_ENDARG so that a `{` right after it belongs to this command + // rather than to anything inside the group (see argParenEnd). Record where + // that `)` is before parsing, and restore the value on the way out so a + // nested command's own record does not outlive it. + argParen := -1 + if p.is(token.LPAREN) && p.cur().SpaceBefore { + argParen = p.scanBalanced(p.pos, token.LPAREN, token.RPAREN) + } + p.argParenEnd = argParen + defer func() { p.argParenEnd = argParen }() // A trailing `do…end` binds to the command call, not to an argument that is // itself a call (`foo bar do…end` → the block is foo's). Suppress block // attachment while parsing the arguments so the enclosing postfix chain picks From 74ed65fae1a4611be47a83866cae3c899ffd9118 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 22:07:35 +0200 Subject: [PATCH 11/14] feat(ast): carry a block's block-local variables on ast.Block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `{ |one; bl| bl }` parsed, but the names after the `;` were declared in the parser's scope and then dropped, so `bl` in the body became an ast.VarRef the consumer had no slot for — rbgo answers `compile error: undefined local variable "bl"` where MRI prints nil. The parser's half of that is the loss of the list. MRI's `opt_bv_decl: '\n'? ';' bv_decls '\n'?` with `bvar: tIDENTIFIER { new_bv(p, $1); }` (parse.y v3_4_0) adds each name to the BLOCK's own local table, where it starts nil and shadows any enclosing binding. ast.Block gains Locals, nil when the list has no `;` part, so a consumer can allocate them. This is what language/block_spec.rb line 782 needs; the file now parses (see the preceding commit) and reaches its compiler. Co-Authored-By: Claude Opus 5 --- ast/ast.go | 8 +++++++- block_param_test.go | 38 ++++++++++++++++++++++++++++++++++++++ parser.go | 19 +++++++++++++------ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/ast/ast.go b/ast/ast.go index 2f9d287..05231ad 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -113,7 +113,13 @@ type Block struct { Defaults []Node // parallel to Params; nil for a required or *splat param SplatIndex int // index of the top-level *splat param in Params, or -1 BlockParam string // name of the &block param, or "" if none (parallels MethodDef.BlockParam) - Body []Node + // Locals are the block-local variables declared after a `;` in the parameter + // list (`{ |a; x, y| … }`): fresh, nil-valued locals of this block that shadow + // any enclosing binding of the same name. MRI: `opt_bv_decl: '\n'? ';' + // bv_decls '\n'?` with `bvar: tIDENTIFIER { new_bv(p, $1); }`. Empty when the + // list has no `;` part. + Locals []string + Body []Node } // Yield invokes the block passed to the enclosing method. diff --git a/block_param_test.go b/block_param_test.go index 806d93d..b6ae8cf 100644 --- a/block_param_test.go +++ b/block_param_test.go @@ -60,3 +60,41 @@ func TestDefBlockParamUnchanged(t *testing.T) { t.Errorf("MethodDef.BlockParam=%q, want %q", def.BlockParam, "b") } } + +// TestBlockLocalsAreCarriedOnTheBlock pins that the block-local list after the +// `;` reaches the AST. MRI's `opt_bv_decl: '\n'? ';' bv_decls '\n'?` with +// `bvar: tIDENTIFIER { new_bv(p, $1); }` (parse.y v3_4_0) adds each name to the +// block's own local table, where it is nil-valued and shadows any enclosing +// binding — so a consumer needs the names to give them slots. +func TestBlockLocalsAreCarriedOnTheBlock(t *testing.T) { + for _, tc := range []struct { + src string + params []string + locals []string + }{ + {`f { |one; bl| bl }`, []string{"one"}, []string{"bl"}}, + {`f { |one; bl, bl2| bl }`, []string{"one"}, []string{"bl", "bl2"}}, + {`f { |one| one }`, []string{"one"}, nil}, + } { + prog, err := parser.Parse(tc.src) + if err != nil { + t.Fatalf("Parse(%q) error: %v", tc.src, err) + } + call, ok := prog.Body[0].(*ast.Call) + if !ok || call.Block == nil { + t.Fatalf("Parse(%q): want a call with a block", tc.src) + } + if got := call.Block.Locals; len(got) != len(tc.locals) { + t.Errorf("Parse(%q): locals = %q, want %q", tc.src, got, tc.locals) + } else { + for i, w := range tc.locals { + if got[i] != w { + t.Errorf("Parse(%q): local %d = %q, want %q", tc.src, i, got[i], w) + } + } + } + if len(call.Block.Params) != len(tc.params) { + t.Errorf("Parse(%q): params = %q, want %q", tc.src, call.Block.Params, tc.params) + } + } +} diff --git a/parser.go b/parser.go index e442454..1d0bab4 100644 --- a/parser.go +++ b/parser.go @@ -2701,6 +2701,7 @@ func (p *Parser) parseBlockRest(stop map[token.Type]bool, end token.Type, withRe var params []string var defaults, prepends []ast.Node var blockParam string + var locals []string splat := -1 // The `|params|` list may start on the line(s) after the block opener: // `foo {``|x| … }`, `do``|a, b| … end`. Skip the intervening newlines @@ -2720,7 +2721,7 @@ func (p *Parser) parseBlockRest(stop map[token.Type]bool, end token.Type, withRe // the body sees them, but carried no further than the parameter list. if p.is(token.NEWLINE) { p.advance() - p.parseBlockLocals() + locals = p.parseBlockLocals() } p.expect(token.PIPE) p.scope().explicitParams = true @@ -2745,21 +2746,27 @@ func (p *Parser) parseBlockRest(stop map[token.Type]bool, end token.Type, withRe if len(prepends) > 0 { body = append(prepends, body...) } - return &ast.Block{Params: params, Defaults: defaults, SplatIndex: splat, BlockParam: blockParam, Body: body} + return &ast.Block{Params: params, Defaults: defaults, SplatIndex: splat, BlockParam: blockParam, Locals: locals, Body: body} } // parseBlockLocals parses the block-local variable list after the `;` in a block -// parameter list (`|a, b; x, y|`). Each name is declared in the current block -// scope so the body resolves it as a local; the list is otherwise discarded -// since block-locals carry no arity or default. -func (p *Parser) parseBlockLocals() { +// parameter list (`|a, b; x, y|`) and returns the names. Each is declared in the +// current block scope so the body resolves it as a local, and carried on the +// Block so a consumer can give it a slot: MRI's `opt_bv_decl: '\n'? ';' bv_decls +// '\n'?` with `bvar: tIDENTIFIER { new_bv(p, $1); }` (parse.y v3_4_0) adds each +// to the block's own local table, where it starts out nil and shadows any +// enclosing binding of the same name. They carry no arity and no default. +func (p *Parser) parseBlockLocals() []string { + var locals []string for { name := p.expect(token.IDENT).Lit p.declareLocal(name) + locals = append(locals, name) if !p.accept(token.COMMA) { break } } + return locals } // parseBlockParams parses a block's parameter list (the `|...|` form for brace/do From afab703df92217f604e8e5fd4b07817a61561b68 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 22:12:13 +0200 Subject: [PATCH 12/14] fix(parser): four pattern-matching gaps, one of them a silent mis-parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four are MRI grammar alternatives the parser did not have. They are sequential: each one was the next thing language/pattern_matching_spec.rb stopped at. The file now parses. 1. `in Array(0, 1, 2)` failed with `expected LABEL, got "0"` — a `Const(…)` pattern was always read as a hash pattern. In MRI the parenthesised and the bracketed const pattern have the SAME four alternatives (parse.y v3_4_0, p_expr_basic): `p_const p_lparen p_args rparen`, `… p_find rparen`, `… p_kwargs rparen`, `p_const '(' rparen`, mirroring p_lbracket/rbracket. The body decides, not the bracket, so parseArrayPattern is now parameterised by its bracket pair and serves both. 2. A trailing comma was dropped. `in [0, 1, ]` parsed but became the EXACT pattern `[0, 1]`, so `case [0,1,2,3]` took the else branch where MRI matches — a wrong answer with no diagnostic. MRI's `p_top_expr_body: p_expr ','` makes it an array pattern with an ANONYMOUS REST, matching any array that starts with the listed elements. The un-bracketed spelling `in 0, 1,;` is the same rule and was a parse error; both now build the rest. 3. `in {"a": 0}` failed. p_kw_label has two spellings — `tLABEL` and `tSTRING_BEG string_contents tLABEL_END` — so a quoted string followed by `:` is the same symbol key. `in {"a" => 1}` stays a parse error, which is what ruby/spec asserts. 4. `in ^@a` failed. `p_var_ref: '^' tIDENTIFIER | '^' nonlocal_var` with `nonlocal_var: tIVAR | tGVAR | tCVAR` (5883-5897) pins a non-local too. Verified against MRI 4.0.5, values compared through rbgo for each case including the negative ones (`in [0, 1]` stays exact, `in {"a" => 1}` refused). Co-Authored-By: Claude Opus 5 --- parser.go | 97 +++++++++++++++++++++++------ pattern_const_paren_test.go | 118 ++++++++++++++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 17 deletions(-) create mode 100644 pattern_const_paren_test.go diff --git a/parser.go b/parser.go index 1d0bab4..2786c59 100644 --- a/parser.go +++ b/parser.go @@ -1344,7 +1344,7 @@ func (p *Parser) parseCaseIn(subject ast.Node) ast.Node { // form. func (p *Parser) parsePattern() ast.Pattern { // A leading label is an implicit (brace-less) hash pattern: `in a:, b:`. - if p.is(token.LABEL) || p.is(token.POW) { + if p.atPatternLabel() || p.is(token.POW) { return p.parseHashPatternBody(nil, token.NEWLINE) } first := p.parseArrayPatternElem() @@ -1396,9 +1396,23 @@ func (p *Parser) parsePatternAtom() ast.Pattern { p.expect(token.RPAREN) return &ast.ValuePattern{Value: e} } + // MRI pins a local (`p_var_ref: '^' tIDENTIFIER`) or a non-local + // (`'^' nonlocal_var`, with `nonlocal_var: tIVAR | tGVAR | tCVAR`) — + // parse.y v3_4_0, 5883-5897. + switch t := p.cur(); t.Type { + case token.IVAR: + p.advance() + return &ast.ValuePattern{Value: &ast.IvarRef{Name: t.Lit}} + case token.GVAR: + p.advance() + return &ast.ValuePattern{Value: &ast.GVarRef{Name: t.Lit}} + case token.CVAR: + p.advance() + return &ast.ValuePattern{Value: &ast.CVarRef{Name: t.Lit}} + } return &ast.ValuePattern{Value: &ast.VarRef{Name: p.expect(token.IDENT).Lit}} case token.LBRACKET: - return p.parseArrayPattern(nil) + return p.parseArrayPattern(nil, token.LBRACKET, token.RBRACKET) case token.LBRACE: return p.parseHashPattern(nil) case token.IDENT: @@ -1414,12 +1428,16 @@ func (p *Parser) parsePatternAtom() ast.Pattern { // class match. c := p.parsePatternConst() if p.is(token.LBRACKET) { - return p.parseArrayPattern(c) + return p.parseArrayPattern(c, token.LBRACKET, token.RBRACKET) } - if p.accept(token.LPAREN) { - hp := p.parseHashPatternBody(c, token.RPAREN) - p.expect(token.RPAREN) - return hp + if p.is(token.LPAREN) { + // `Const(…)` and `Const[…]` are the SAME four alternatives in MRI + // (parse.y v3_4_0, p_expr_basic): `p_const p_lparen p_args rparen`, + // `… p_find rparen`, `… p_kwargs rparen`, `p_const '(' rparen`, and the + // identical quartet with p_lbracket/rbracket. So `Array(0, 1, 2)` is an + // array pattern, exactly as `Array[0, 1, 2]` is, and `Point(x:, y:)` a + // hash pattern — the body decides, not the bracket. + return p.parseArrayPattern(c, token.LPAREN, token.RPAREN) } return &ast.ConstPattern{Const: c} default: @@ -1455,30 +1473,35 @@ func (p *Parser) parsePatternValue() ast.Node { // parseArrayPattern parses `[pat, …]`, with an optional leading constant. When // the bracket body begins with a label or `**`, it is a hash pattern written // with bracket delimiters (`Const[key:]`, deconstruct_keys), which MRI accepts. -func (p *Parser) parseArrayPattern(constName ast.Node) ast.Pattern { - p.expect(token.LBRACKET) +func (p *Parser) parseArrayPattern(constName ast.Node, open, close token.Type) ast.Pattern { + p.expect(open) // A bracket pattern body may span several lines (`Const[`⏎` a: 1,`⏎`]`), so - // newlines after the `[` and around the separating commas are insignificant. + // newlines after the opener and around the separating commas are insignificant. p.skipNewlines() - if p.is(token.LABEL) || p.is(token.POW) { - hp := p.parseHashPatternBody(constName, token.RBRACKET) + if p.atPatternLabel() || p.is(token.POW) { + hp := p.parseHashPatternBody(constName, close) p.skipNewlines() - p.expect(token.RBRACKET) + p.expect(close) return hp } var elems []arrayElem - if !p.accept(token.RBRACKET) { + if !p.accept(close) { elems = append(elems, p.parseArrayPatternElem()) p.skipNewlines() for p.accept(token.COMMA) { p.skipNewlines() - if p.is(token.RBRACKET) { // trailing comma + if p.is(close) { + // A trailing comma is MRI's `p_top_expr_body: p_expr ','` + // (parse.y v3_4_0), an array pattern with an ANONYMOUS REST: it + // matches any array STARTING with the listed elements, so + // `in [0, 1, ]` matches [0, 1, 2, 3]. + elems = append(elems, arrayElem{splat: true}) break } elems = append(elems, p.parseArrayPatternElem()) p.skipNewlines() } - p.expect(token.RBRACKET) + p.expect(close) } return p.buildArrayOrFind(constName, elems) } @@ -1495,6 +1518,26 @@ func (p *Parser) parseHashPattern(constName ast.Node) ast.Pattern { // parseHashPatternBody parses the comma-separated entries of a hash pattern up // to (but not consuming) end. Entries are `label: [value-pattern]` or the // double-splat `**name` / `**nil`. +// atPatternLabel reports whether the cursor opens a hash-pattern key. MRI's +// `p_kw_label: tLABEL | tSTRING_BEG string_contents tLABEL_END` (parse.y +// v3_4_0) accepts a quoted string followed by `:` wherever a bare label is +// accepted, so `in {"a": 0}` is `in {a: 0}`. +func (p *Parser) atPatternLabel() bool { + return p.is(token.LABEL) || (p.is(token.STRING) && p.peekTok().Type == token.COLON) +} + +// patternKey reads one hash-pattern key in either of p_kw_label's two spellings +// and returns the symbol it names. A `"a" => 1` rocket entry is deliberately not +// accepted: ruby/spec requires `case {a: 1}; in {"a" => 1}` to raise SyntaxError. +func (p *Parser) patternKey() string { + if p.is(token.STRING) && p.peekTok().Type == token.COLON { + key := p.advance().Lit + p.advance() // ':' + return key + } + return p.expect(token.LABEL).Lit +} + func (p *Parser) parseHashPatternBody(constName ast.Node, end token.Type) *ast.HashPattern { hp := &ast.HashPattern{Const: constName} for !p.is(end) && !p.is(token.NEWLINE) && !p.is(token.THEN) && !p.is(token.IF) && !p.is(token.UNLESS) { @@ -1512,7 +1555,7 @@ func (p *Parser) parseHashPatternBody(constName ast.Node, end token.Type) *ast.H } } } else { - key := p.expect(token.LABEL).Lit + key := p.patternKey() hp.Keys = append(hp.Keys, key) // `name:` with no following pattern binds local `name`; otherwise the // value is a sub-pattern. A clause/entry boundary means no value. @@ -1536,11 +1579,31 @@ func (p *Parser) parseHashPatternBody(constName ast.Node, end token.Type) *ast.H func (p *Parser) parseArrayPatternRest(constName ast.Node, first arrayElem) ast.Pattern { elems := []arrayElem{first} for p.accept(token.COMMA) { + if p.atPatternEnd() { + // The un-bracketed spelling of the same rule: `in 0, 1,` is + // `p_top_expr_body: p_expr ','`, an anonymous rest. + elems = append(elems, arrayElem{splat: true}) + break + } elems = append(elems, p.parseArrayPatternElem()) } return p.buildArrayOrFind(constName, elems) } +// atPatternEnd reports whether the cursor ends an un-bracketed pattern list, so +// that a `,` just consumed was a trailing one. A `case/in` pattern runs up to +// the clause body (a newline — which is what the lexer makes of the `;` in +// `in 0, 1,;` — or `then`) or a guard (`if`/`unless`); a one-line `=>`/`in` +// match also ends at a closing bracket or at the end of input. +func (p *Parser) atPatternEnd() bool { + switch p.cur().Type { + case token.NEWLINE, token.EOF, token.THEN, token.IF, token.UNLESS, + token.RPAREN, token.RBRACKET, token.RBRACE, token.END: + return true + } + return false +} + // arrayElem is one parsed array-pattern element: an ordinary sub-pattern, or a // `*[name]` splat marker (splat true, with the optional capture name). type arrayElem struct { diff --git a/pattern_const_paren_test.go b/pattern_const_paren_test.go new file mode 100644 index 0000000..54344bf --- /dev/null +++ b/pattern_const_paren_test.go @@ -0,0 +1,118 @@ +package parser_test + +import ( + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// casePattern parses `case … in …` and returns the clause's pattern. +func casePattern(t *testing.T, src string) ast.Pattern { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("Parse(%q) returned error: %v", src, err) + } + c, ok := prog.Body[0].(*ast.CaseIn) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.CaseIn", src, prog.Body[0]) + } + if len(c.Clauses) != 1 { + t.Fatalf("Parse(%q): %d in-clauses, want 1", src, len(c.Clauses)) + } + return c.Clauses[0].Pattern +} + +// TestConstPatternParenthesesTakeTheSameBodies covers MRI's p_expr_basic +// (parse.y v3_4_0): `p_const p_lparen p_args rparen`, `… p_find rparen`, +// `… p_kwargs rparen` and `p_const '(' rparen` are the same four alternatives +// as the p_lbracket/rbracket quartet, so `Array(0, 1, 2)` is an array pattern +// and `Point(x:, y:)` a hash pattern — the body decides, not the bracket. +func TestConstPatternParenthesesTakeTheSameBodies(t *testing.T) { + t.Parallel() + if _, ok := casePattern(t, `case x; in Array(0, 1, 2); 1; end`).(*ast.ArrayPattern); !ok { + t.Error("Array(0, 1, 2) is not an *ast.ArrayPattern") + } + if _, ok := casePattern(t, `case x; in Point(a:); 1; end`).(*ast.HashPattern); !ok { + t.Error("Point(a:) is not an *ast.HashPattern") + } + if _, ok := casePattern(t, `case x; in Array(); 1; end`).(*ast.ArrayPattern); !ok { + t.Error("Array() is not an *ast.ArrayPattern") + } + if _, ok := casePattern(t, `case x; in Array(*, 1, *); 1; end`).(*ast.FindPattern); !ok { + t.Error("Array(*, 1, *) is not an *ast.FindPattern") + } +} + +// TestPatternTrailingCommaIsAnAnonymousRest covers MRI's +// `p_top_expr_body: p_expr ','` (parse.y v3_4_0). A trailing comma makes the +// array pattern match any array STARTING with the listed elements, so +// `in [0, 1, ]` matches [0, 1, 2, 3] — before this the comma was dropped, which +// silently turned a partial match into an exact one. +func TestPatternTrailingCommaIsAnAnonymousRest(t *testing.T) { + t.Parallel() + for _, src := range []string{ + `case x; in [0, 1, ]; 1; end`, + "case x\nin 0, 1,;\n1\nend", + `case x; in [0, 1, ] then 1; end`, + } { + ap, ok := casePattern(t, src).(*ast.ArrayPattern) + if !ok { + t.Fatalf("Parse(%q): pattern = %T, want *ast.ArrayPattern", src, casePattern(t, src)) + } + if !ap.HasSplat || ap.SplatName != "" { + t.Errorf("Parse(%q): HasSplat=%v SplatName=%q, want true and \"\"", src, ap.HasSplat, ap.SplatName) + } + if len(ap.Pre) != 2 || len(ap.Post) != 0 { + t.Errorf("Parse(%q): %d pre / %d post elements, want 2 / 0", src, len(ap.Pre), len(ap.Post)) + } + } + // Without the comma the pattern stays exact. + ap, ok := casePattern(t, `case x; in [0, 1]; 1; end`).(*ast.ArrayPattern) + if !ok || ap.HasSplat { + t.Error("in [0, 1] must not carry a rest") + } +} + +// TestHashPatternStringKey covers p_kw_label's second spelling, +// `tSTRING_BEG string_contents tLABEL_END` (parse.y v3_4_0): a quoted string +// followed by `:` names the same symbol key as a bare label. A `"a" => 1` +// rocket entry is not a pattern key, and ruby/spec requires it to be rejected. +func TestHashPatternStringKey(t *testing.T) { + t.Parallel() + hp, ok := casePattern(t, `case x; in {"a": 0}; 1; end`).(*ast.HashPattern) + if !ok { + t.Fatalf(`in {"a": 0} is not an *ast.HashPattern`) + } + if len(hp.Keys) != 1 || hp.Keys[0] != "a" { + t.Errorf(`in {"a": 0}: keys = %q, want ["a"]`, hp.Keys) + } + if _, err := parser.Parse(`case x; in {"a" => 1}; 1; end`); err == nil { + t.Error(`in {"a" => 1}: want a parse error`) + } +} + +// TestPinNonLocalVariable covers `p_var_ref: '^' tIDENTIFIER | '^' nonlocal_var` +// with `nonlocal_var: tIVAR | tGVAR | tCVAR` (parse.y v3_4_0, 5883-5897): a pin +// may name an instance, global or class variable, not only a local. +func TestPinNonLocalVariable(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + src string + want any + }{ + {`case x; in ^@a; 1; end`, (*ast.IvarRef)(nil)}, + {`case x; in ^$a; 1; end`, (*ast.GVarRef)(nil)}, + {`case x; in ^@@a; 1; end`, (*ast.CVarRef)(nil)}, + {`case x; in ^a; 1; end`, (*ast.VarRef)(nil)}, + } { + vp, ok := casePattern(t, tc.src).(*ast.ValuePattern) + if !ok { + t.Fatalf("Parse(%q): pattern is not an *ast.ValuePattern", tc.src) + } + if got, want := nodeType(vp.Value), nodeType(tc.want); got != want { + t.Errorf("Parse(%q): pinned value = %s, want %s", tc.src, got, want) + } + } +} From ac7e3a1f0506e17f5db8ea69c67fe0bfaf1e7eb3 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 22:14:10 +0200 Subject: [PATCH 13/14] fix(parser): a parenthesised receiver is a multiple-assignment target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `(o).a, (o).a = 1, 2` failed with `unexpected "," after statement`: a `(` at the start of a target was always taken for a nested target group, so `(o).a` ended the scan. MRI's mlhs_node takes any primary_value as the receiver — `primary_value call_op tIDENTIFIER`, `primary_value '[' opt_call_args rbracket`, `primary_value tCOLON2 tIDENTIFIER` (parse.y v3_4_0, 3639-3652) — and a parenthesised expression is a primary. looksLikeMlhs and parseMlhsTarget now look past the balanced `(…)`: a postfix chain after it makes the group a RECEIVER, anything else leaves it the nested target list it was. So `(a, b), c = [1, 2], 3` is unchanged and `(o).a = 1` stays a plain attribute assignment rather than becoming a one-target masgn. Unblocks language/assignments_spec.rb, which now parses. ruby/spec uses this shape to pin evaluation order — `(ScratchPad << :a; obj).a, … = …` — and rbgo still evaluates the right-hand side before the target receivers, which is a compiler matter: the AST records the two sides, not an order. Verified against MRI 4.0.5 through rbgo: same targets, same values. Co-Authored-By: Claude Opus 5 --- masgn_targets_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++ parser.go | 28 ++++++++++++++++++- 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/masgn_targets_test.go b/masgn_targets_test.go index 0ede07d..697bd96 100644 --- a/masgn_targets_test.go +++ b/masgn_targets_test.go @@ -278,3 +278,65 @@ func TestMasgnSelfAttrTarget(t *testing.T) { t.Errorf("self.x masgn: recv=%T, want *ast.SelfLit", call.Recv) } } + +// TestParenthesisedReceiverTarget covers MRI's mlhs_node, whose receiver is any +// primary_value: `primary_value call_op tIDENTIFIER`, +// `primary_value '[' opt_call_args rbracket`, `primary_value tCOLON2 +// tIDENTIFIER` (parse.y v3_4_0, 3639-3652). A parenthesised expression is a +// primary, so `(o).a, (o).a = 1, 2` is a multiple assignment — the shape +// ruby/spec uses to pin evaluation order. A parenthesised group NOT followed by +// a postfix chain stays a nested target list. +func TestParenthesisedReceiverTarget(t *testing.T) { + for _, tc := range []struct { + src string + targets int + }{ + {`(o).a, (o).a = 1, 2`, 2}, + {`(o)[1], x = 1, 2`, 2}, + {`(o).a, (b, c) = 1, [2, 3]`, 2}, + } { + prog, err := parser.Parse(tc.src) + if err != nil { + t.Fatalf("Parse(%q) error: %v", tc.src, err) + } + ma, ok := prog.Body[0].(*ast.MultiAssign) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.MultiAssign", tc.src, prog.Body[0]) + } + if len(ma.Targets) != tc.targets { + t.Errorf("Parse(%q): %d targets, want %d", tc.src, len(ma.Targets), tc.targets) + } + } + + // The first target is the setter call on the parenthesised receiver. + prog, err := parser.Parse(`(o).a, x = 1, 2`) + if err != nil { + t.Fatalf("Parse error: %v", err) + } + ma := prog.Body[0].(*ast.MultiAssign) + call, ok := ma.Targets[0].(*ast.Call) + if !ok { + t.Fatalf("first target = %T, want *ast.Call", ma.Targets[0]) + } + if call.Name != "a=" || call.Recv == nil { + t.Errorf("first target = %q on %T, want a= on a receiver", call.Name, call.Recv) + } + + // A group with no postfix chain is still a nested target list, and a single + // `(o).a = 1` is a plain attribute assignment, not a masgn. + prog, err = parser.Parse(`(a, b), c = [1, 2], 3`) + if err != nil { + t.Fatalf("Parse error: %v", err) + } + ma = prog.Body[0].(*ast.MultiAssign) + if _, ok := ma.Targets[0].(*ast.MultiAssign); !ok { + t.Errorf("(a, b) group = %T, want a nested *ast.MultiAssign", ma.Targets[0]) + } + prog, err = parser.Parse(`(o).a = 1`) + if err != nil { + t.Fatalf("Parse error: %v", err) + } + if _, ok := prog.Body[0].(*ast.MultiAssign); ok { + t.Error("(o).a = 1 parsed as a multiple assignment") + } +} diff --git a/parser.go b/parser.go index 2786c59..799db21 100644 --- a/parser.go +++ b/parser.go @@ -1703,6 +1703,28 @@ func (p *Parser) looksLikeMlhs() bool { if j < 0 { return false } + // `(expr).attr` / `(expr)[i]` is a TARGET, not a nested group: MRI's + // mlhs_node takes any primary_value as the receiver + // (`primary_value call_op tIDENTIFIER`, `primary_value '[' opt_call_args + // rbracket`, `primary_value tCOLON2 tIDENTIFIER` — parse.y v3_4_0, + // 3639-3652) and a parenthesised expression is a primary. ruby/spec uses + // it to pin evaluation order: `(ScratchPad << :a; obj).a, … = …`. + if isPostfixStart(p.toks[j].Type) { + i = p.scanMlhsTargetTail(j) + if i < 0 { + return false + } + switch p.toks[i].Type { + case token.COMMA: + sawComma = true + i++ + continue + case token.ASSIGN: + return sawComma || sawSplat + default: + return false + } + } sawGroup = true i = j switch p.toks[i].Type { @@ -1919,7 +1941,11 @@ func (p *Parser) parseMlhsTarget() (string, ast.Node, bool) { // group destructures one value into its own sub-targets; it is represented as // a nested *MultiAssign with no Values (Values stays nil), stored as a target. if p.is(token.LPAREN) { - return "", p.parseMlhsGroup(), false + // Unless the group is followed by a postfix chain, in which case it is the + // receiver of an attribute/index target (see looksLikeMlhs). + if j := p.scanBalanced(p.pos, token.LPAREN, token.RPAREN); j < 0 || !isPostfixStart(p.toks[j].Type) { + return "", p.parseMlhsGroup(), false + } } // Simple local: declare it and use a *VarRef (fast path). if p.is(token.IDENT) && !isPostfixStart(p.peekTok().Type) { From f07f2faa94007624022d80147d09adaa14e2d983 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 21 Sep 2026 22:17:00 +0200 Subject: [PATCH 14/14] feat(parser): a for-loop variable is any lhs or mlhs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `for i, in [[1,2]]` failed with `expected IDENT, got "in"`, and so did every other loop-variable shape beyond a comma-separated list of bare names: `for @v in`, `for $v in`, `for C in`, `for o.a in`, `for o&.a in`, `for a[0] in`, `for i, *j in`, `for (i, j, k) in`, `for i, (j, k) in`, `for (i, j), k in`. MRI's loop variable is `for_var: lhs | mlhs` (parse.y v3_4_0) — any assignment target, or the same target list a multiple assignment takes, `mlhs_basic` and all. parseFor now parses it with the mlhs machinery instead of a loop over IDENTs. Two distinctions the grammar makes and the AST has to keep: - `mlhs_head: mlhs_item ','` is a COMPLETE mlhs, so a trailing comma means destructuring: `for i, in [[1,2]]` binds i to 1 where `for i in [[1,2]]` binds it to [1,2]. MRI 4.0.5 prints 1 and [1, 2] respectively. - a SINGLE target is `for_var: lhs`, not a one-element `mlhs`, so it binds the whole element: `for @v in [[1,2]]` sets @v to [1, 2]. ast.For gains Target for everything Vars cannot spell. Vars is EMPTY whenever Target is set, so a consumer reads the loop variable from exactly one place, and the shapes that already worked — `for i in …`, `for a, b in …` — still arrive in Vars unchanged. Unblocks language/for_spec.rb. With this every one of the 68 files in the pinned ruby/spec language corpus parses. Verified against MRI 4.0.5. Co-Authored-By: Claude Opus 5 --- ast/ast.go | 27 +++++++--- for_var_test.go | 132 ++++++++++++++++++++++++++++++++++++++++++++++++ parser.go | 86 +++++++++++++++++++++++++------ 3 files changed, 224 insertions(+), 21 deletions(-) create mode 100644 for_var_test.go diff --git a/ast/ast.go b/ast/ast.go index 05231ad..21fcf9a 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -147,13 +147,28 @@ type While struct { Body []Node } -// For is a `for VARS in ITER ... end` loop. Vars names the one or more loop -// variables (`for a, b in pairs`); unlike a block, a `for` does not introduce a -// new scope, so the variables remain visible after the loop. +// For is a `for VARS in ITER ... end` loop. Unlike a block, a `for` does not +// introduce a new scope, so its variables remain visible after the loop. +// +// MRI's loop variable is `for_var: lhs | mlhs`, i.e. any assignment target or +// any multiple-assignment target list. The common shapes — one local, or a +// comma-separated list of locals — are in Vars, which is what they have always +// been. Everything else needs a node, and then Target is non-nil and Vars is +// EMPTY, so there is only ever one place to read the loop variable from: +// +// - a single non-local target (`for @v in …`, `for $v in …`, `for C in …`, +// `for o.a in …`, `for a[0] in …`): Target is that target, already in its +// assignable form (a setter call for the attribute and index forms). +// - any list that Vars cannot spell — one holding a `*rest` (`for a, *b in …`), +// a nested group (`for a, (b, c) in …`), a non-local (`for @a, b in …`), or a +// trailing comma (`for a, in …`, which destructures where `for a in …` does +// not): Target is a *MultiAssign with Values nil, the same shape a nested +// masgn target has. type For struct { - Vars []string - Iter Node - Body []Node + Vars []string + Target Node + Iter Node + Body []Node } // MethodDef defines a method on the current self. diff --git a/for_var_test.go b/for_var_test.go new file mode 100644 index 0000000..72d7e07 --- /dev/null +++ b/for_var_test.go @@ -0,0 +1,132 @@ +package parser_test + +import ( + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +func parseFor(t *testing.T, src string) *ast.For { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("Parse(%q) returned error: %v", src, err) + } + f, ok := prog.Body[0].(*ast.For) + if !ok { + t.Fatalf("Parse(%q): node = %T, want *ast.For", src, prog.Body[0]) + } + return f +} + +// TestForVarPlainLocals keeps the simple spellings in Vars, with Target nil. +func TestForVarPlainLocals(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + src string + want []string + }{ + {`for i in 1..3; end`, []string{"i"}}, + {`for a, b in pairs; end`, []string{"a", "b"}}, + {`for i in 1..3 do end`, []string{"i"}}, + } { + f := parseFor(t, tc.src) + if f.Target != nil { + t.Errorf("Parse(%q): Target = %T, want nil", tc.src, f.Target) + } + if len(f.Vars) != len(tc.want) { + t.Fatalf("Parse(%q): Vars = %q, want %q", tc.src, f.Vars, tc.want) + } + for i, w := range tc.want { + if f.Vars[i] != w { + t.Errorf("Parse(%q): Vars[%d] = %q, want %q", tc.src, i, f.Vars[i], w) + } + } + } +} + +// TestForVarIsALhsOrAnMlhs covers MRI's `for_var: lhs | mlhs` (parse.y v3_4_0). +// Every shape a multiple assignment accepts is a loop variable: a non-local, an +// attribute or index target, a `*rest`, a nested group, and the trailing comma +// of `mlhs_head: mlhs_item ','`, which destructures. +func TestForVarIsALhsOrAnMlhs(t *testing.T) { + t.Parallel() + t.Run("single non-local target", func(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + src string + want any + }{ + {`for @v in m; end`, (*ast.IvarRef)(nil)}, + {`for @@v in m; end`, (*ast.CVarRef)(nil)}, + {`for $v in m; end`, (*ast.GVarRef)(nil)}, + {`for C in m; end`, (*ast.ConstRef)(nil)}, + {`for o.a in m; end`, (*ast.Call)(nil)}, + {`for o&.a in m; end`, (*ast.Call)(nil)}, + {`for a[0] in m; end`, (*ast.Call)(nil)}, + } { + f := parseFor(t, tc.src) + if len(f.Vars) != 0 { + t.Errorf("Parse(%q): Vars = %q, want empty", tc.src, f.Vars) + } + if got, want := nodeType(f.Target), nodeType(tc.want); got != want { + t.Errorf("Parse(%q): Target = %s, want %s", tc.src, got, want) + } + } + // The attribute and index forms arrive already assignable. + if call := parseFor(t, `for o.a in m; end`).Target.(*ast.Call); call.Name != "a=" { + t.Errorf(`for o.a: target calls %q, want "a="`, call.Name) + } + if call := parseFor(t, `for a[0] in m; end`).Target.(*ast.Call); call.Name != "[]=" { + t.Errorf(`for a[0]: target calls %q, want "[]="`, call.Name) + } + }) + + t.Run("target lists", func(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + src string + names int + splat int + }{ + {`for i, in [[1,2]]; end`, 1, -1}, + {`for i, * in [[1,2]]; end`, 2, 1}, + {`for i, *j in [[1,2]]; end`, 2, 1}, + {`for i, *j, k in [[1,2,3,4]]; end`, 3, 1}, + {`for (i, j, k) in [[1,2,3]]; end`, 3, -1}, + {`for i, (j, k) in [[1,[2,3]]]; end`, 2, -1}, + {`for (i, j), k in [[[1,2],3]]; end`, 2, -1}, + {`for @a, b in m; end`, 2, -1}, + } { + f := parseFor(t, tc.src) + if len(f.Vars) != 0 { + t.Errorf("Parse(%q): Vars = %q, want empty", tc.src, f.Vars) + } + ma, ok := f.Target.(*ast.MultiAssign) + if !ok { + t.Fatalf("Parse(%q): Target = %T, want *ast.MultiAssign", tc.src, f.Target) + } + if ma.Values != nil { + t.Errorf("Parse(%q): Target carries Values; a for-var drives no right-hand side", tc.src) + } + if len(ma.Names) != tc.names { + t.Errorf("Parse(%q): %d names %q, want %d", tc.src, len(ma.Names), ma.Names, tc.names) + } + if ma.SplatIndex != tc.splat { + t.Errorf("Parse(%q): SplatIndex = %d, want %d", tc.src, ma.SplatIndex, tc.splat) + } + } + }) + + t.Run("a trailing comma is not the bare form", func(t *testing.T) { + t.Parallel() + // `for i in …` binds the whole element; `for i, in …` destructures it. + if parseFor(t, `for i in m; end`).Target != nil { + t.Error("for i in: Target must be nil") + } + if parseFor(t, `for i, in m; end`).Target == nil { + t.Error("for i, in: Target must be set, else the destructuring is lost") + } + }) +} diff --git a/parser.go b/parser.go index 799db21..afbe674 100644 --- a/parser.go +++ b/parser.go @@ -958,28 +958,84 @@ func (p *Parser) parseUntil() ast.Node { return &ast.While{Cond: not(cond), Body: body} } -// parseFor parses `for VAR[, VAR…] in ITER [do] ... end`. The loop variables are -// plain names (one or more, comma-separated) that — unlike block parameters — -// are declared in the enclosing scope and outlive the loop, so they are recorded -// as locals here. The iterator expression is parsed with `do…end` attachment -// suppressed so a trailing `do` belongs to the loop, not to a call within it. +// parseFor parses `for VARS in ITER [do] ... end`. The loop variables — unlike +// block parameters — are declared in the enclosing scope and outlive the loop, +// so they are recorded as locals here. The iterator expression is parsed with +// `do…end` attachment suppressed so a trailing `do` belongs to the loop, not to +// a call within it. func (p *Parser) parseFor() ast.Node { p.expect(token.FOR) - var vars []string - for { - name := p.expect(token.IDENT).Lit - vars = append(vars, name) - p.declareLocal(name) - if !p.accept(token.COMMA) { - break - } - } + vars, target := p.parseForVar() p.expect(token.IN) iter := p.parseLoopCond() p.accept(token.DO) body := p.parseStatements(bodyEnd) p.expect(token.END) - return &ast.For{Vars: vars, Iter: iter, Body: body} + return &ast.For{Vars: vars, Target: target, Iter: iter, Body: body} +} + +// parseForVar parses MRI's `for_var: lhs | mlhs` (parse.y v3_4_0). `lhs` is any +// assignment target, and `mlhs` the same target list a multiple assignment +// takes, `mlhs_basic` and all: a `*rest` (`mlhs_head tSTAR mlhs_node`), nested +// groups (`mlhs_item: tLPAREN mlhs_inner rparen`) and a trailing comma +// (`mlhs_head: mlhs_item ','`, which is a COMPLETE mlhs and therefore +// destructures — `for i, in [[1,2]]` binds i to 1 where `for i in [[1,2]]` binds +// it to [1,2]). +// +// It returns the plain-local spelling in the first result when the list is one +// or more plain locals with none of those features, and otherwise the general +// target in the second (see ast.For). The two are never both set. +func (p *Parser) parseForVar() ([]string, ast.Node) { + var names []string + var targets []ast.Node + onlyLocals := true + splat := -1 + trailingComma := false + for { + if p.accept(token.STAR) { + splat = len(names) + onlyLocals = false + // A nameless rest (`for i, * in …`) is followed straight by `,` or `in`. + if p.is(token.COMMA) || p.is(token.IN) { + names = append(names, "") + targets = append(targets, nil) + } else { + name, tgt, _ := p.parseMlhsTarget() + names = append(names, name) + targets = append(targets, tgt) + } + } else { + name, tgt, local := p.parseMlhsTarget() + names = append(names, name) + targets = append(targets, tgt) + if !local { + onlyLocals = false + } + } + if !p.accept(token.COMMA) { + break + } + if p.is(token.IN) { // `for i, in …` + trailingComma = true + break + } + } + if splat < 0 && !trailingComma { + // One or more plain locals keep the Vars spelling. + if onlyLocals { + return names, nil + } + // A single target is MRI's `for_var: lhs`, which binds the whole element: + // `for @v in [[1,2]]` sets @v to [1,2]. Wrapping it in a one-element + // MultiAssign would make it an `mlhs` instead, and destructure. + if len(names) == 1 { + return nil, targets[0] + } + } + if onlyLocals { + targets = nil // Names alone suffice, as in parseMlhs + } + return nil, &ast.MultiAssign{Names: names, Targets: targets, SplatIndex: splat} } func (p *Parser) parseReturn() ast.Node {