Bug #21984: RubyVM::AST can't tell if an array is passed to break/next/return

• Status: Open
• Assignee: -
• Target version: -
• Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN

[kddnewton (Kevin Newton)]

irb(main):005> RubyVM::AbstractSyntaxTree.parse("tap { break [1, 2, 3] }")
=> (SCOPE@1:0-1:23 tbl: [] args: nil body: (ITER@1:0-1:23 (FCALL@1:0-1:3 :tap nil) (SCOPE@1:4-1:23 tbl: [] args: nil body: (BREAK@1:6-1:21 (LIST@1:12-1:21 (INTEGER@1:13-1:14 1) (INTEGER@1:16-1:17 2) (INTEGER@1:19-1:20 3) nil)))))
irb(main):006> RubyVM::AbstractSyntaxTree.parse("tap { break 1, 2, 3 }")
=> (SCOPE@1:0-1:21 tbl: [] args: nil body: (ITER@1:0-1:21 (FCALL@1:0-1:3 :tap nil) (SCOPE@1:4-1:21 tbl: [] args: nil body: (BREAK@1:6-1:19 (LIST@1:12-1:19 (INTEGER@1:12-1:13 1) (INTEGER@1:15-1:16 2) (INTEGER@1:18-1:19 3) nil)))))
irb(main):007> 

The issue is, break [1, 2, 3] has the same AST as break 1, 2, 3 (presumably because they are effectively the same thing). But a consumer has no way to know if an array was used short of re-parsing the source.

[nobu (Nobuyoshi Nakada)]

Changing this causes the rbs tests to fail.

https://github.com/ruby/ruby/actions/runs/24117591586/job/70364654030?pr=16678#step:14:828

  class Hello
-   def with_return: () -> (1 | "2" | :x)
?                              ^
+   def with_return: () -> (::Array[1] | ::Array["2"] | :x)
?                           ++++++++ +  ^^^^^^^^^   +
  
-   def with_untyped_return: () -> (untyped | :x)
+   def with_untyped_return: () -> (::Array[untyped] | :x)
?                                   ++++++++       +
  
-   def with_return_same_types: () -> 1
+   def with_return_same_types: () -> (::Array[1] | 1)
?                                     +++++++++ ++++++
  
    def with_return_without_value: () -> (nil | 42)
  
    def when_last_is_nil: () -> nil
  end

https://github.com/ruby/ruby/actions/runs/24117591586/job/70364654030?pr=16678#step:14:920

  class ReturnTypeWithIF
    def with_if: () -> (true | nil)
  
-   def with_if_and_block: () -> (false | true | nil)
+   def with_if_and_block: () -> (::Array[false] | true | nil)
?                                 ++++++++     +
  
    def with_else_and_bool: () -> (true | false)
  
    def with_else_and_elsif_and_bool_nil: () -> (nil | true | false)
  
    def with_nested_if: () -> (1 | 2 | 3 | 4)
  
    def with_unless: () -> (:sym | nil)
  end

[kddnewton (Kevin Newton)]

I don't think that fix works:

$ ./ruby -e 'pp RubyVM::AbstractSyntaxTree.parse("tap { break 1, 2 }")'
(SCOPE@1:0-1:18 tbl: [] args: nil body: (ITER@1:0-1:18 (FCALL@1:0-1:3 :tap nil) (SCOPE@1:4-1:18 tbl: [] args: nil body: (BREAK@1:6-1:16 (LIST@1:12-1:16 (INTEGER@1:12-1:13 1) (INTEGER@1:15-1:16 2) nil)))))
$ ./ruby -e 'pp RubyVM::AbstractSyntaxTree.parse("tap { break [1, 2] }")'
(SCOPE@1:0-1:20 tbl: [] args: nil body: (ITER@1:0-1:20 (FCALL@1:0-1:3 :tap nil) (SCOPE@1:4-1:20 tbl: [] args: nil body: (BREAK@1:6-1:18 (LIST@1:12-1:18 (LIST@1:12-1:18 (INTEGER@1:13-1:14 1) (INTEGER@1:16-1:17 2) nil) nil)))))
[10:15:45] kddnewton ~/Code/ruby/ruby @ast-ret_args $

That makes it looks like it's a nested array.

$ ./ruby -e 'pp RubyVM::AbstractSyntaxTree.parse("tap { break 1 }")'     
(SCOPE@1:0-1:15 tbl: [] args: nil body: (ITER@1:0-1:15 (FCALL@1:0-1:3 :tap nil) (SCOPE@1:4-1:15 tbl: [] args: nil body: (BREAK@1:6-1:13 (LIST@1:12-1:13 (INTEGER@1:12-1:13 1) nil)))))

Also, now when you have a single argument it looks like an array, which is probably what is breaking RBS.

[tompng (tomoya ishida)]

The same thing can be also said to a=[1,2] and a=1,2.
In Prism, these are both parsed as ArrayNode with/without opening and closing.
In AbstractSyntaxTree, the non-abstract parts (bracket) are dropped. I think it's just a restriction of abstract trees. Is this a problem to be fixed?

[kddnewton (Kevin Newton)]

The honest answer is, not really. It's probably fine to stay as is, and I would be okay with closing this.

For context, I've been building a translation layer from RubyVM::AbstractSyntaxTree to Prism, hoping it might help with a migration process. I'm running into problems like this one, #21986, #21985, #21983, #21980, etc when we don't have enough information to be able to reliably reproduce the source.