From 2876cebd9a914e6dd7cff2f0664423fd47c7ad95 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Mon, 28 Sep 2026 20:20:19 +0200 Subject: [PATCH] ast: record the parentheses on a call site (Call.Paren) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `foo` and `foo()` built identical `ast.Call`s, so nothing downstream could tell a VCALL from an FCALL. `ast.Call` carries no position either, so a consumer could not re-read the source to recover the parentheses. The difference is observable in MRI: nope_xyz # NameError: undefined local variable or method 'nope_xyz' nope_xyz() # NoMethodError: undefined method 'nope_xyz' MRI keeps them apart by building different nodes from different productions: `fcall: operation` feeding `method_call: fcall paren_args` (parse.y-ruby_4_0:3572-3577, 5242-5248) makes an FCALL, while a bare name reaching `gettable` makes a VCALL (parse.y-ruby_4_0:13086). This records the same distinction as one bit of written form, set wherever the parser consumes a written `(` … `)` argument list: the bare-identifier call, a capitalised call, a `.` or `&.` call, a `::` call, and the `.()` shorthand. Paren is written form and nothing more. It is true for an empty list (`foo()`) and false for every unparenthesised call whatever else is present, including `a[0]` — brackets are not parentheses (MRI: `primary_value '[' opt_call_args rbracket`, parse.y-ruby_4_0:5292-5297). Deciding VCALL takes more than this bit, and the doc comment says so: MRI's `operation` admits tFID, so `foo!` and `foo?` are FCALLs with no parentheses (parse.y-ruby_4_0:4370-4374, 6696-6698), and a literal block makes an FCALL too (`primary: fcall brace_block`, parse.y-ruby_4_0:4458-4463). The rule a consumer wants is `Recv == nil && !Paren && len(Args) == 0 && Block == nil` on a plain identifier name; a test exercises exactly that rule over the edge set. The zero value is false, so no existing consumer changes. No existing test was modified; the full suite passes unchanged. Closes #41 Co-Authored-By: Claude Opus 5 --- ast/ast.go | 24 +++++++ call_paren_test.go | 164 +++++++++++++++++++++++++++++++++++++++++++++ parser.go | 27 +++++--- 3 files changed, 207 insertions(+), 8 deletions(-) create mode 100644 call_paren_test.go diff --git a/ast/ast.go b/ast/ast.go index 2a87117..d6dc9bb 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -122,6 +122,30 @@ type Call struct { Args []Node Block *Block Safe bool // &. safe navigation: nil receiver short-circuits to nil + // Paren records one bit of WRITTEN FORM: the call site had a parenthesised + // argument list, `foo(…)`, rather than none. It is what separates MRI's + // FCALL from its VCALL, which are different nodes built by different + // productions — `fcall: operation` feeding `method_call: fcall paren_args` + // (parse.y-ruby_4_0:3572-3577 and 5242-5248) makes an FCALL, while a bare + // name reaching `gettable` makes a VCALL (parse.y-ruby_4_0:13086). The + // difference is observable: `nope` raises NameError, `nope()` raises + // NoMethodError. + // + // It is written form and nothing else. It is true for an empty list + // (`foo()`, where Args is nil) and false for every call written without + // parentheses whatever else is there — `puts 1`, `obj.m`, and `a[0]`, + // since brackets are not parentheses. It is also false on the Calls the + // parser synthesises with no source of their own, such as the read half + // of an op-assign. + // + // Deciding VCALL takes more than this one bit, because MRI's `operation` + // admits tFID: `foo!` and `foo?` are FCALLs with no parentheses + // (parse.y-ruby_4_0:4370-4374 and 6696-6698), and a literal block makes an + // FCALL too (`primary: fcall brace_block`, parse.y-ruby_4_0:4458-4463). A + // consumer wanting VCALL should ask for Recv == nil && !Paren && + // len(Args) == 0 && Block == nil && a Name that is a plain identifier + // (no trailing ! or ?). + Paren bool } // Block is a literal block: parameters and a body. It is a closure over the diff --git a/call_paren_test.go b/call_paren_test.go new file mode 100644 index 0000000..45cdae3 --- /dev/null +++ b/call_paren_test.go @@ -0,0 +1,164 @@ +package parser_test + +import ( + "testing" + + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// lastNode parses src and returns its last top-level node, so a case may set up +// a local variable first (`x = 1; x()`). +func lastNode(t *testing.T, src string) ast.Node { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("Parse(%q): %v", src, err) + } + if len(prog.Body) == 0 { + t.Fatalf("Parse(%q): empty program", src) + } + return prog.Body[len(prog.Body)-1] +} + +// TestCallParen is the bit itself: whether the call site was written with a +// parenthesised argument list. It is what separates MRI's FCALL from its +// VCALL, built by different productions — `fcall: operation` into +// `method_call: fcall paren_args` (parse.y-ruby_4_0:3572-3577, 5242-5248) +// versus a bare name reaching `gettable` (parse.y-ruby_4_0:13086) — and +// observable as NoMethodError versus NameError. +func TestCallParen(t *testing.T) { + cases := []struct { + src string + wantName string + wantParen bool + wantArgs int + }{ + // The pair from issue #41. Same name, same (empty) arguments. + {"foo", "foo", false, 0}, + {"foo()", "foo", true, 0}, + {"foo(1)", "foo", true, 1}, + // `foo ()` is not a parenthesised argument list: the space makes the + // `()` a grouped expression passed as a command argument, so Paren is + // false while an argument appears. It is an FCALL in MRI, and it stays + // one for a consumer, because a VCALL also requires no arguments. + {"foo ()", "foo", false, 1}, + {"puts 1", "puts", false, 1}, + // With a receiver. + {"obj.foo", "foo", false, 0}, + {"obj.foo()", "foo", true, 0}, + {"obj.foo(1)", "foo", true, 1}, + {"obj.foo bar", "foo", false, 1}, + // Safe navigation, both spellings. + {"obj&.foo", "foo", false, 0}, + {"obj&.foo()", "foo", true, 0}, + // The `.()` shorthand is parenthesised by construction. + {"foo.()", "call", true, 0}, + {"foo.(1)", "call", true, 1}, + // Scope resolution, both spellings, lowercase and capitalised name. + {"Foo::bar", "bar", false, 0}, + {"Foo::bar(1)", "bar", true, 1}, + {"Foo::Bar(1)", "Bar", true, 1}, + // A capitalised method call. + {`Integer("42")`, "Integer", true, 1}, + // Brackets are not parentheses: an index is `recv.[](…)` with no + // written parens (MRI: `primary_value '[' opt_call_args rbracket`, + // parse.y-ruby_4_0:5292-5297). + {"a[0]", "[]", false, 1}, + // A name that is also a local variable: with parens it is a call, and + // the bare form is not a Call at all (covered below). + {"x = 1; x()", "x", true, 0}, + // tFID: `foo!` and `foo?` carry no parentheses, so Paren is false — + // MRI nonetheless makes them FCALLs (`primary: tFID`, + // parse.y-ruby_4_0:4370-4374), which is why a consumer deciding VCALL + // must also look at the name. Pinned here so that stays visible. + {"foo!", "foo!", false, 0}, + {"foo?", "foo?", false, 0}, + } + for _, tc := range cases { + t.Run(tc.src, func(t *testing.T) { + node := lastNode(t, tc.src) + call, ok := node.(*ast.Call) + if !ok { + t.Fatalf("Parse(%q): node is %T, want *ast.Call", tc.src, node) + } + if call.Name != tc.wantName { + t.Fatalf("Parse(%q): Name = %q, want %q", tc.src, call.Name, tc.wantName) + } + if call.Paren != tc.wantParen { + t.Errorf("Parse(%q): Paren = %v, want %v", tc.src, call.Paren, tc.wantParen) + } + if len(call.Args) != tc.wantArgs { + t.Errorf("Parse(%q): %d args, want %d", tc.src, len(call.Args), tc.wantArgs) + } + }) + } +} + +// TestCallParenBareLocalIsNotACall is the other half of the local-variable +// case: a bare name that IS a visible local reads as a variable, so there is no +// Call to ask Paren of. `x = 1; x()` above is the same name with parentheses. +func TestCallParenBareLocalIsNotACall(t *testing.T) { + node := lastNode(t, "x = 1; x") + if _, ok := node.(*ast.VarRef); !ok { + t.Fatalf(`Parse("x = 1; x"): node is %T, want *ast.VarRef`, node) + } +} + +// TestCallParenBlockIsNotAParen pins the remaining input to a consumer's VCALL +// test: a literal block makes an FCALL in MRI (`primary: fcall brace_block`, +// parse.y-ruby_4_0:4458-4463) although no parentheses were written. Paren is +// false and Block is set, so the two are distinguishable. +func TestCallParenBlockIsNotAParen(t *testing.T) { + for _, src := range []string{"foo {}", "foo do end"} { + node := lastNode(t, src) + call, ok := node.(*ast.Call) + if !ok { + t.Fatalf("Parse(%q): node is %T, want *ast.Call", src, node) + } + if call.Paren { + t.Errorf("Parse(%q): Paren = true, want false", src) + } + if call.Block == nil { + t.Errorf("Parse(%q): Block = nil, want a block", src) + } + } +} + +// TestCallParenVCallRule exercises the rule the documentation on Call.Paren +// gives a consumer, over the whole edge set at once: a VCALL is a bare plain +// identifier with no receiver, no parentheses, no arguments and no block. The +// expectations are MRI 4.0.5's, which raises NameError for a VCALL and +// NoMethodError for an FCALL. +func TestCallParenVCallRule(t *testing.T) { + isVCall := func(n ast.Node) bool { + c, ok := n.(*ast.Call) + if !ok || c.Recv != nil || c.Paren || len(c.Args) != 0 || c.Block != nil { + return false + } + last := c.Name[len(c.Name)-1] + return last != '!' && last != '?' && last != '=' + } + cases := []struct { + src string + want bool // true == VCALL (MRI raises NameError) + }{ + {"nope", true}, + {"nope()", false}, + {"nope(1)", false}, + {"nope!", false}, + {"nope?", false}, + {"nope {}", false}, + {"nope do end", false}, + {"nope 1", false}, + {"self.nope", false}, + {"obj.nope", false}, + } + for _, tc := range cases { + t.Run(tc.src, func(t *testing.T) { + if got := isVCall(lastNode(t, tc.src)); got != tc.want { + t.Errorf("Parse(%q): VCALL = %v, want %v", tc.src, got, tc.want) + } + }) + } +} diff --git a/parser.go b/parser.go index 3bcad0f..e965bd1 100644 --- a/parser.go +++ b/parser.go @@ -3173,13 +3173,15 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { p.advance() args := p.parseCallArgs(token.RPAREN) p.expect(token.RPAREN) - node = &ast.Call{Recv: node, Name: "call", Args: args, Safe: safe} + node = &ast.Call{Recv: node, Name: "call", Args: args, Safe: safe, Paren: true} break } name := p.methodName() var args []ast.Node + paren := false if p.is(token.LPAREN) && !p.cur().SpaceBefore { p.advance() + paren = true args = p.parseCallArgs(token.RPAREN) p.expect(token.RPAREN) } else if p.commandArgsFollow(cmdArgRecv) { @@ -3201,7 +3203,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { } return call } - node = &ast.Call{Recv: node, Name: name, Args: args, Safe: safe} + node = &ast.Call{Recv: node, Name: name, Args: args, Safe: safe, Paren: paren} case p.is(token.SCOPE): p.advance() // `Const::Name(args)` — a capitalized scope-resolution method call: @@ -3213,7 +3215,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { p.advance() // consume '(' args := p.parseCallArgs(token.RPAREN) p.expect(token.RPAREN) - node = &ast.Call{Recv: node, Name: name, Args: args} + node = &ast.Call{Recv: node, Name: name, Args: args, Paren: true} break } if p.is(token.CONST) { @@ -3237,8 +3239,10 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { // Foo::bar(args) or `Mod::meth arg` — a method call, like the dot form. name := p.methodName() var args []ast.Node + paren := false if p.is(token.LPAREN) && !p.cur().SpaceBefore { p.advance() + paren = true args = p.parseCallArgs(token.RPAREN) p.expect(token.RPAREN) } else if p.commandArgsFollow(cmdArgRecv) { @@ -3252,7 +3256,7 @@ func (p *Parser) parsePostfixTail(node ast.Node) ast.Node { } return call } - node = &ast.Call{Recv: node, Name: name, Args: args} + node = &ast.Call{Recv: node, Name: name, Args: args, Paren: paren} case p.is(token.LBRACKET): // index: recv[args] → recv.[](args) p.advance() args := p.parseCallArgs(token.RBRACKET) @@ -3859,7 +3863,7 @@ func (p *Parser) parsePrimary() ast.Node { p.advance() args := p.parseCallArgs(token.RPAREN) p.expect(token.RPAREN) - return &ast.Call{Name: t.Lit, Args: args} + return &ast.Call{Name: t.Lit, Args: args, Paren: true} } // A string hugging the constant is a paren-less call: `Integer"42"`. if p.commandArgsFollow(cmdArgHugStringHere) { @@ -3973,16 +3977,20 @@ func (p *Parser) parseIdentExpr() ast.Node { restoreCmd() p.skipNewlines() p.expect(token.RPAREN) - return &ast.Call{Name: name, Args: []ast.Node{arg}} + return &ast.Call{Name: name, Args: []ast.Node{arg}, Paren: true} } - // foo(...) — paren call (the '(' must hug the name). + // foo(...) — paren call (the '(' must hug the name). This is MRI's + // `method_call: fcall paren_args` (parse.y-ruby_4_0:5242-5248), an FCALL, + // and Paren records it: `foo()` reaches here with an empty argument list, + // where the bare `foo` handled at the end of this function does not reach + // here at all. if next.Type == token.LPAREN && !next.SpaceBefore { p.advance() // name p.advance() // ( args := p.parseCallArgs(token.RPAREN) p.expect(token.RPAREN) - return &ast.Call{Name: name, Args: args} + return &ast.Call{Name: name, Args: args, Paren: true} } // A string literal hugging the name (no space) is a paren-less argument: @@ -4062,6 +4070,9 @@ func (p *Parser) parseIdentExpr() ast.Node { return &ast.VarRef{Name: name} } } + // A bare name, no parentheses, no arguments: MRI's VCALL (gettable, + // parse.y-ruby_4_0:13086). Paren stays false, and that is the whole of + // the difference from the `foo(…)` branch above. return &ast.Call{Name: name} }