Bug #22020: Inner call node without all arguments returned by RubyVM::AbstractSyntaxTree.of for call with a block

• Status: Open
• Assignee: -
• Target version: -
• ruby -v: ruby 4.1.0dev
• Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN

[Eregon (Benoit Daloze)]

begin
  foo(1, 2, kw: :arg) { 42 }
rescue => e
  pp RubyVM::AbstractSyntaxTree.of e.backtrace_locations[0]
end

$ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb
(FCALL@2:2-2:21 :foo
   (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2)
      (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil))

That covers foo(1, 2, kw: :arg) so all arguments except the block argument, which looks inconsistent.

I believe it should be:

$ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb
(ITER@2:2-2:28
   (FCALL@2:2-2:21 :foo
      (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2)
         (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil))
   (SCOPE@2:22-2:28 tbl: [] args: nil body: (INTEGER@2:24-2:26 42)))

which covers foo(1, 2, kw: :arg) { 42 }.

PR: https://github.com/ruby/ruby/pull/16836

[Eregon (Benoit Daloze)]

• ruby -v set to ruby 4.1.0dev

[Eregon (Benoit Daloze)]

• Description updated (diff)

[Eregon (Benoit Daloze)]

The current locations are like this:

foo(1, 2, kw: :arg) { 123 }
|---------ITER------------|
|------FCALL------| |SCOPE|

When foo does not exist, it's a NoMethodError, and RubyVM::AbstractSyntaxTree.of e.backtrace_locations[0] is currently returning the FCALL node.
This is including all arguments but not the block, which seems inconsistent.
https://github.com/ruby/ruby/pull/16836 returns the ITER node instead for that case.

Another error case is:

lambda { }.call(1)
|--ITER--|
|----| |-|
FCALL  SCOPE

This already returns the ITER node currently.

ErrorHighlight needs to differentiate both.
It's able to do so using point_type based on the exception class & message in https://github.com/ruby/ruby/pull/16836, but that might not be ideal.

Also RubyVM::AbstractSyntaxTree.of(lambda { }) returns the ITER node currently.

Alternatives solutions are:

2. Like the PR, return ITER for foo(1, 2, kw: :arg) { 123 }, but return SCOPE instead of ITER for lambda { }.call(1) and for RubyVM::AbstractSyntaxTree.of(lambda { })

This seems clean, the block gets its own separate node from the call.
One thought is methods also have SCOPE (see here), but I think those nodes are never returned from RubyVM::AbstractSyntaxTree.of so there should be no ambiguity.

3. Don't change which nodes are returned, instead extend the location of the FCALL to cover the block as well, same as the location of ITER:

foo(1, 2, kw: :arg) { 123 }
|---------ITER------------|
|---------FCALL-----------|
                    |SCOPE|

Pros: the FCALL node includes all arguments, nice.
Cons: we have two nodes with the same location, feels somewhat redundant but actually it's useful given ITER is returned for lambda { }.call(1).

---

For information, this is what it looks like with Prism:

foo(1, 2, kw: :arg) { 123 }
|-------CallNode----------|
                    |-----|
                    BlockNode

[Eregon (Benoit Daloze)]

My motivation here is I would like to implement Thread::Bactrace::Location#source_range.
For this to work in --parser=parse.y mode it needs to return the same start/end line/column as Prism in all cases of RubyVM::AbstractSyntaxTree.of(a Thread::Bactrace::Location).
Some adjustments could maybe be done specifically in the Thread::Bactrace::Location#source_range method if not ambiguous, but it seems cleaner to align the location in RubyVM::AbstractSyntaxTree, I think at least for this case.

Concretely for no_such_method(1, 2, kw: :arg) { 123 } if we want to highlight the entire expression raising the NoMethodError we should highlight no_such_method(1, 2, kw: :arg) { 123 }, and not no_such_method(1, 2, kw: :arg) which is what RubyVM::AbstractSyntaxTree currently gives as the location.