Skip to content

parser: refuse a modifier-rescue and a not/nested-! command in an arg position, and require every clause terminator #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tannevaled merged 2 commits into from
Sep 26, 2026
Merged

parser: refuse a modifier-rescue and a not/nested-! command in an arg position, and require every clause terminator #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f1532c
parser: refuse a modifier-`rescue` in an `arg` position, and a comman…
tannevaled Sep 26, 2026
dd5462c
parser: three more productions with no `command`/`rescue`/`not` arm, …
tannevaled Sep 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
328 changes: 328 additions & 0 deletions arg_position_command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,165 @@ func TestParenLessCommandCallRefusedInAnArgPosition(t *testing.T) {
`{k: foo bar}`,
`{k: foo.bar baz}`,
`~foo bar`,
// --- v0.6.1 additions. Three further productions that have no
// `command`/`modifier_rescue`/`keyword_not` alternative, plus the clause
// terminator that makes the refusal observable. Every shape judged on ruby
// 4.0.5 by BOTH `ruby -c` and `ruby --parser=parse.y -c`, which agree on all
// of them.
//
// `arg: '!' arg` (4009) vs `expr: '!' command_call` (3350): the operand of
// `!` may be a paren-less command ONLY at the `expr` level, and a
// `command_call` is not itself a unary expression — hence `!foo bar` legal,
// `!!foo bar` not.
//
// `arg` has no `modifier_rescue` arm — only `stmt` (3184) and the assignment
// right-hand sides `arg_rhs` (4162) / `command_rhs` / `endless_arg` do — and
// `expr` has none either, which covers the `expr_value` headers.
//
// `arg` has no `keyword_not` arm: `not` is `expr: keyword_not '\n'? expr`
// (3345) only.
//
// And `then`/`do` — `term | KEYWORD | term KEYWORD` — is REQUIRED by every
// clause head (`opt_rescue` 5926, `case_body` 5397, `k_if`/`k_unless`,
// `expr_value_do`, `p_case_body`). Without it the stray token of an
// `arg_value` already refused upstream was read as the clause's first
// statement and the clause parsed anyway.
"begin\nx\nrescue foo bar\ny\nend",
"case x\nwhen foo bar\n1\nend",
`!! foo bar`,
`!!!!foo bar`,
`!!!foo bar`,
`!!Foo 1`,
`!![foo bar]`,
`!!a.b foo bar`,
`!!defined? foo bar`,
`!!foo bar, baz`,
`!!foo bar`,
`!!foo.bar baz`,
`!!not foo bar`,
`!!super 1`,
`!!x = foo bar`,
`!-foo bar`,
`!a = foo bar`,
`!~foo bar`,
`+!foo bar`,
`-!foo bar`,
`->() { x; rescue foo bar }`,
`-not x`,
`1 + [x rescue nil]`,
`1 + not x`,
`Foo(x rescue nil)`,
`[!!foo bar]`,
`[*not x]`,
`[*x rescue nil]`,
`[1, 2 rescue nil]`,
`[1, not x]`,
`[1, x rescue nil]`,
`[1].each do; x; rescue foo bar; end`,
`[1].each { x; rescue foo bar; }`,
`[[x rescue nil]]`,
`[not x]`,
`[x rescue nil]`,
`[{k: x rescue nil}]`,
`a = !!foo bar`,
`a ? not x : 1`,
`a ? x rescue nil : 1`,
`a&.b(x rescue nil)`,
`a.b(1, x rescue nil)`,
`a.b(x rescue nil)`,
`a::b(x rescue nil)`,
`a[1, not x]`,
`a[1, x rescue nil]`,
`a[x rescue nil]`,
`begin; x; rescue !foo bar; y; end`,
`begin; x; rescue *foo bar; y; end`,
`begin; x; rescue Foo => e end`,
`begin; x; rescue Foo bar; y; end`,
`begin; x; rescue Foo end`,
`begin; x; rescue Foo if c; y; end`,
`begin; x; rescue Foo rescue Bar; y; end`,
`begin; x; rescue Foo rescue nil; y; end`,
`begin; x; rescue Foo, Bar end`,
`begin; x; rescue Foo, bar baz => e; y; end`,
`begin; x; rescue Foo, bar baz; y; end`,
`begin; x; rescue a = foo bar; y; end`,
`begin; x; rescue end`,
`begin; x; rescue foo bar => e; y; end`,
`begin; x; rescue foo bar baz qux; y; end`,
`begin; x; rescue foo bar then y; end`,
`begin; x; rescue foo bar; y; end`,
`begin; y; rescue not x; z; end`,
`case x rescue nil; when 1; end`,
`case x; in 1 end`,
`case x; when 1 end`,
`case x; when 1, 2 end`,
`case x; when Foo, bar baz; 1; end`,
`case x; when foo bar; 1; end`,
`case x; when foo bar; end`,
`case x; when y rescue nil then 1; end`,
`case x; when y rescue nil; end`,
`case y; when not x; end`,
`class C; x; rescue foo bar; end`,
`def m(*a rescue nil); end`,
`def m(a = not x); end`,
`def m(a = x rescue nil); end`,
`def m(a: not x); end`,
`def m(a: x rescue nil); end`,
`def m; super(x rescue nil); end`,
`def m; x; rescue *foo bar; end`,
`def m; x; rescue Foo, bar baz; end`,
`def m; x; rescue foo bar; end`,
`def m; yield(x rescue nil); end`,
`elsif_check = 1; if a; elsif b end`,
`f(!x rescue nil)`,
`f(&not x)`,
`f(&x rescue nil)`,
`f(**not x)`,
`f(**x rescue nil)`,
`f(*a, x rescue nil)`,
`f(*x rescue nil)`,
`f(-x rescue nil)`,
`f(1, 2 rescue nil)`,
`f(1, not x)`,
`f(1, x rescue nil)`,
`f(a: 1, b: x rescue nil)`,
`f(defined?(x) rescue nil)`,
`f(k: x rescue nil)`,
`f(x + 1 rescue nil)`,
`f(x ? 1 : 2 rescue nil)`,
`f(x rescue nil)`,
`f(x rescue nil, 1)`,
`f(x, y rescue nil)`,
`f(y, x rescue nil, 1)`,
`foo(bar rescue baz)`,
`for i in a end`,
`for i in x rescue nil; end`,
`h = {k: v rescue nil}`,
`if !!foo bar then end`,
`if !!foo bar; end`,
`if x 1 end`,
`if x end`,
`if x rescue nil; end`,
`module M; x; rescue foo bar; end`,
`not !!foo bar`,
`p !!foo bar`,
`p(x rescue nil)`,
`super(x rescue nil)`,
`unless x end`,
`unless x rescue nil; end`,
`until x end`,
`until x rescue nil; end`,
`while !!foo bar; end`,
`while [x rescue nil]; end`,
`while x end`,
`while x rescue nil; end`,
`x = [x rescue nil]`,
`{1 => not x}`,
`{1 => x rescue nil}`,
`{k: [x rescue nil]}`,
`{k: not x}`,
`{k: x rescue nil}`,
`~!foo bar`,
} {
_, err := Parse(src)
if err == nil {
Expand Down Expand Up @@ -234,6 +393,175 @@ func TestParenLessCommandCallStillAcceptedWhereMRIAdmitsIt(t *testing.T) {
`{k: a[foo bar]}`,
`{k: defined?(foo bar)}`,
`{k: foo(bar baz)}`,
// --- v0.6.1 additions: the other direction. The contrasts that name each
// rule sit beside their refused twin above — `f((x rescue nil))` vs
// `f(x rescue nil)`, `f(a = 1 rescue nil)` vs `f(x rescue nil)` (the
// `arg_rhs` carve-out), `!!(foo bar)` vs `!!foo bar`, `when (y rescue nil)`
// vs `when y rescue nil`. `defined? x` in an `arg` position is here because
// `arg: keyword_defined '\n'? begin_defined arg` (4021) IS an `arg`
// production: `[defined? x]` is legal MRI and v0.6.0 refused it.
"begin\nx\nrescue Foo\ny\nend",
"case x\nwhen Foo\n1\nend",
`!!!defined? x`,
`!!!x`,
`!!(begin; foo bar; end)`,
`!!(foo bar)`,
`!!a[foo bar]`,
`!!begin; foo bar; end`,
`!!defined? x`,
`!!defined?(foo bar)`,
`!!foo(bar baz)`,
`!!foo(bar)`,
`!!if foo bar then 1 end`,
`!!lambda { foo bar }`,
`!!while foo bar; end`,
`!!x.y`,
`!!x[0]`,
`!!x`,
`!(foo bar)`,
`!Foo 1`,
`!a.b foo bar`,
`!foo bar rescue 1`,
`!foo bar, baz`,
`!foo.bar baz`,
`(not x)`,
`(x rescue nil)`,
`-(x rescue nil)`,
`-> { x rescue nil }`,
`-defined? x`,
`-x rescue nil`,
`1 + (x rescue nil)`,
`1 + defined? x`,
`1 + x rescue nil`,
`@x = y rescue nil`,
`X = y rescue nil`,
`[!x]`,
`[(x rescue nil)]`,
`[1].map { |x| x rescue nil }`,
`[a += x rescue nil]`,
`[a = 1 rescue nil, 2]`,
`[a = b = 1 rescue nil]`,
`[a = x rescue nil]`,
`[defined? foo]`,
`[defined? x]`,
`[defined?(x)]`,
`a += x rescue nil`,
`a = [1, 2] rescue nil`,
`a = b = x rescue nil`,
`a = x rescue nil`,
`a ? 1 : x rescue nil`,
`a, b = x rescue nil`,
`a.b = y rescue nil`,
`a[(x rescue nil)]`,
`a[0] = y rescue nil`,
`a[a = 1 rescue nil]`,
`a[defined? x]`,
`begin; x rescue nil; end`,
`begin; x; ensure; foo bar; end`,
`begin; x; rescue (Foo rescue nil); y; end`,
`begin; x; rescue (foo bar); y; end`,
`begin; x; rescue *A, B; y; end`,
`begin; x; rescue *Foo; y; end`,
`begin; x; rescue ::Foo; y; end`,
`begin; x; rescue => e; y; end`,
`begin; x; rescue Foo => e then y; end`,
`begin; x; rescue Foo => e; y; end`,
`begin; x; rescue Foo then y; end`,
`begin; x; rescue Foo then; y; end`,
`begin; x; rescue Foo, ::Bar; y; end`,
`begin; x; rescue Foo, Bar => e; y; end`,
`begin; x; rescue Foo, Bar then y; end`,
`begin; x; rescue Foo, Bar; y; end`,
`begin; x; rescue Foo, foo(bar baz); y; end`,
`begin; x; rescue Foo::Bar; y; end`,
`begin; x; rescue Foo; else; y; end`,
`begin; x; rescue Foo; end`,
`begin; x; rescue Foo; rescue Bar; end`,
`begin; x; rescue Foo; y; end rescue nil`,
`begin; x; rescue Foo; y; end`,
`begin; x; rescue [Foo, Bar].first; y; end`,
`begin; x; rescue a.b; y; end`,
`begin; x; rescue a::B; y; end`,
`begin; x; rescue a[foo bar]; y; end`,
`begin; x; rescue foo(bar); y; end`,
`begin; x; rescue; foo bar; end`,
`begin; x; rescue; y; end`,
`begin; y; rescue defined? x; z; end`,
`case (x rescue nil); when 1; end`,
`case x; in 1 then 2; end`,
`case x; in 1; 2; end`,
`case x; when (y rescue nil); end`,
`case x; when *a; 3; end`,
`case x; when 1 then 2; end`,
`case x; when 1 then; 2; end`,
`case x; when 1, 2; 3; end`,
`case x; when 1; 2; end`,
`case x; when 1; end`,
`case x; when Foo; end`,
`case x; when a.b; end`,
`case x; when a[0]; end`,
`case y; when defined? x; end`,
`def m = x rescue nil`,
`def m(a = (x rescue nil)); end`,
`def m(a = defined? x); end`,
`def m(a: (x rescue nil)); end`,
`def m; return x rescue nil; end`,
`def m; super x rescue nil; end`,
`def m; x rescue nil; end`,
`def m; x; ensure; y; end`,
`def m; yield x rescue nil; end`,
`f { x rescue nil }`,
`f($x = 1 rescue nil)`,
`f((x rescue nil))`,
`f(1) { x rescue nil }`,
`f(1, (x rescue nil))`,
`f(1, a = 2 rescue nil)`,
`f(1, defined? x)`,
`f(::A = 1 rescue nil)`,
`f(@x = 1 rescue nil)`,
`f(A = 1 rescue nil)`,
`f(a &&= 1 rescue nil)`,
`f(a = 1 rescue nil)`,
`f(a = b = 1 rescue nil)`,
`f(a ||= 1 rescue nil)`,
`f(a.b = 1 rescue nil)`,
`f(a::B = 1 rescue nil)`,
`f(a[0] = 1 rescue nil)`,
`f(x) rescue nil`,
`foo a = 1 rescue nil`,
`foo x rescue nil`,
`for i in (x rescue nil); end`,
`for i in a do end`,
`if (x rescue nil); end`,
`if a; elsif b; end`,
`if foo bar then 1 end`,
`if not x; end`,
`if x then 1 end`,
`if x; 1; end`,
`if x; end`,
`lambda { x rescue nil }`,
`loop do x rescue nil end`,
`not !foo bar`,
`not not foo bar`,
`not not x`,
`not x`,
`p x rescue nil`,
`return x rescue nil`,
`unless (x rescue nil); end`,
`unless not done; end`,
`until (x rescue nil); end`,
`while (x rescue nil); end`,
`while not x; end`,
`while x do end`,
`while x; end`,
`x = (not y)`,
`x = begin; y; rescue; 1; end rescue nil`,
`x = y rescue nil`,
`x rescue nil`,
`x ||= y rescue nil`,
`{k: (x rescue nil)}`,
`{k: a = x rescue nil}`,
`{k: defined? x}`,
} {
if _, err := Parse(src); err != nil {
t.Errorf("Parse(%q) = %v, want no error (MRI 4.0.5 accepts it)", src, err)
Expand Down
27 changes: 20 additions & 7 deletions modifier_rescue_command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package parser_test
import (
"testing"

"github.com/go-ruby-parser/parser"
"github.com/go-ruby-parser/parser/ast"
)

Expand Down Expand Up @@ -156,9 +157,11 @@ func TestModifierRescueUnchangedNonCommand(t *testing.T) {
})
}

// TestModifierRescueInsideDelimitedArg checks that a modifier rescue inside a
// nested delimited context (an explicit paren group, or a parenthesised argument
// list) is still consumed there — the command-call suppression must not leak in.
// TestModifierRescueInsideDelimitedArg checks where a modifier rescue nested
// inside a delimited context is consumed. A `(…)` GROUP is MRI's `compstmt`, so
// the rescue is consumed there; a `(…)` ARGUMENT LIST is not — its contents are
// `call_args`, which has no `modifier_rescue` alternative. Both directions
// measured on ruby 4.0.5 with `ruby -c` and `ruby --parser=parse.y -c`.
func TestModifierRescueInsideDelimitedArg(t *testing.T) {
// Explicit paren group as a command argument: the rescue binds inside the
// group, so the group becomes a Begin argument of the command.
Expand All @@ -171,10 +174,20 @@ func TestModifierRescueInsideDelimitedArg(t *testing.T) {
}
})

// Parenthesised argument list nested inside a command call: the inner rescue
// belongs to that argument (`foo(bar rescue baz)`), while `outer` keeps both.
t.Run("paren arglist inside command", func(t *testing.T) {
const src = `outer foo(bar rescue baz)`
// A parenthesised ARGUMENT LIST is not a statement context: `f(bar rescue
// baz)` is a SyntaxError in MRI, which wants `f((bar rescue baz))`. Both of
// MRI's parsers refuse it — prism says "unexpected 'rescue' modifier; expected
// a `)` to close the arguments" — because `call_args`/`args`/`arg_value` offer
// no `modifier_rescue` arm; only `stmt` (3184) and `arg_rhs` (4162) do. This
// subtest previously asserted the opposite and pinned our own divergence.
t.Run("paren arglist inside command refuses it", func(t *testing.T) {
for _, src := range []string{`outer foo(bar rescue baz)`, `foo(bar rescue baz)`} {
if _, err := parser.Parse(src); err == nil {
t.Errorf("Parse(%q) = nil error, want a parse error (MRI 4.0.5 refuses it)", src)
}
}
// The shape MRI does accept: the argument's own parentheses make it a group.
const src = `outer foo((bar rescue baz))`
outer := wantCall(t, src, parseOne(t, src), "outer", 1)
foo := wantCall(t, src, outer.Args[0], "foo", 1)
body, _ := beginWrapping(t, src, foo.Args[0])
Expand Down
Loading
Loading