+ Added emit_forward_arg compatibility flag. (#710)

README.md

@@ -29,6 +29,7 @@ below for explanation of `emit_*` calls):
     Parser::Builders::Default.emit_encoding            = true
     Parser::Builders::Default.emit_index               = true
     Parser::Builders::Default.emit_arg_inside_procarg0 = true
+    Parser::Builders::Default.emit_forward_arg         = true
 
 Parse a chunk of code:
 

doc/AST_FORMAT.md

@@ -1164,13 +1164,13 @@ s(:numblock,
 
 ## Forward arguments
 
-### Method definition accepting forwarding arguments
+### Method definition accepting only forwarding arguments
 
 Ruby 2.7 introduced a feature called "arguments forwarding".
 When a method takes any arguments for forwarding them in the future
 the whole `args` node gets replaced with `forward-args` node.
 
-Format:
+Format if `emit_forward_arg` compatibility flag is disabled:
 
 ~~~
 (def :foo
@@ -1181,6 +1181,25 @@ Format:
         ~~~~~ expression
 ~~~
 
+However, Ruby 2.8 added support for leading arguments before `...`, and so
+it can't be used as a replacement of the `(args)` node anymore. To solve it
+`emit_forward_arg` should be enabled.
+
+Format if `emit_forward_arg` compatibility flag is enabled:
+
+~~~
+(def :foo
+  (args
+    (forward-arg)) nil)
+"def foo(...); end"
+        ~ begin (args)
+            ~ end (args)
+        ~~~~~ expression (args)
+         ~~~ expression (forward_arg)
+~~~
+
+Note that the node is called `forward_arg` when emitted separately.
+
 ### Method call taking arguments of the currently forwarding method
 
 Format:

lib/parser/ast/processor.rb

@@ -127,6 +127,7 @@ def process_argument_node(node)
       alias on_kwarg          process_argument_node
       alias on_kwoptarg       process_argument_node
       alias on_kwrestarg      process_argument_node
+      alias on_forward_arg    process_argument_node
 
       def on_procarg0(node)
         if node.children[0].is_a?(Symbol)

lib/parser/builders/default.rb

@@ -80,6 +80,8 @@ class << self
       attr_accessor :emit_index
     end
 
+    @emit_index = false
+
     class << self
       ##
       # AST compatibility attribute; causes a single non-mlhs
@@ -95,7 +97,36 @@ class << self
       attr_accessor :emit_arg_inside_procarg0
     end
 
-    @emit_index = false
+    @emit_arg_inside_procarg0 = false
+
+    class << self
+      ##
+      # AST compatibility attribute; arguments forwarding initially
+      # didn't have support for leading arguments
+      # (i.e. `def m(a, ...); end` was a syntax error). However, Ruby 2.8
+      # added support for any number of arguments in front of the `...`.
+      #
+      # If set to false (the default):
+      #   1. `def m(...) end` is emitted as
+      #      s(:def, :m, s(:forward_args), nil)
+      #   2. `def m(a, b, ...) end` is emitted as
+      #      s(:def, :m,
+      #        s(:args, s(:arg, :a), s(:arg, :b), s(:forward_arg)))
+      #
+      # If set to true it uses a single format:
+      #   1. `def m(...) end` is emitted as
+      #      s(:def, :m, s(:args, s(:forward_arg)))
+      #   2. `def m(a, b, ...) end` is emitted as
+      #      s(:def, :m, s(:args, s(:arg, :a), s(:arg, :b), s(:forward_arg)))
+      #
+      # It does't matter that much on 2.7 (because there can't be any leading arguments),
+      # but on 2.8 it should be better enabled to use a single AST format.
+      #
+      # @return [Boolean]
+      attr_accessor :emit_forward_arg
+    end
+
+    @emit_forward_arg = false
 
     class << self
       ##
@@ -106,6 +137,7 @@ def modernize
         @emit_encoding = true
         @emit_index = true
         @emit_arg_inside_procarg0 = true
+        @emit_forward_arg = true
       end
     end
 
@@ -709,8 +741,14 @@ def numargs(max_numparam)
       n(:numargs, [ max_numparam ], nil)
     end
 
-    def forward_args(begin_t, dots_t, end_t)
-      n(:forward_args, [], collection_map(begin_t, token_map(dots_t), end_t))
+    def forward_only_args(begin_t, dots_t, end_t)
+      if self.class.emit_forward_arg
+        forward_arg = n(:forward_arg, [], token_map(dots_t))
+        n(:args, [ forward_arg ],
+          collection_map(begin_t, [ forward_arg ], end_t))
+      else
+        n(:forward_args, [], collection_map(begin_t, token_map(dots_t), end_t))
+      end
     end
 
     def arg(name_t)

lib/parser/meta.rb

@@ -26,7 +26,7 @@ module class sclass def defs def_e defs_e undef alias args
         ident root lambda indexasgn index procarg0
         restarg_expr blockarg_expr
         objc_kwarg objc_restarg objc_varargs
-        numargs numblock forward_args forwarded_args
+        numargs numblock forward_args forwarded_args forward_arg
         case_match in_match in_pattern
         match_var pin match_alt match_as match_rest
         array_pattern match_with_trailing_comma array_pattern_with_tail

lib/parser/ruby27.y

@@ -2532,7 +2532,7 @@ keyword_variable: kNIL
                     }
                 | tLPAREN2 args_forward rparen
                     {
-                      result = @builder.forward_args(val[0], val[1], val[2])
+                      result = @builder.forward_only_args(val[0], val[1], val[2])
                       @static_env.declare_forward_args
 
                       @lexer.state = :expr_value

lib/parser/ruby28.y

@@ -2595,7 +2595,7 @@ keyword_variable: kNIL
                     }
                 | tLPAREN2 args_forward rparen
                     {
-                      result = @builder.forward_args(val[0], val[1], val[2])
+                      result = @builder.forward_only_args(val[0], val[1], val[2])
                       @static_env.declare_forward_args
 
                       @lexer.state = :expr_value

lib/parser/runner.rb

@@ -37,7 +37,7 @@ def execute(options)
 
     private
 
-    LEGACY_MODES = %i[lambda procarg0 encoding index arg_inside_procarg0].freeze
+    LEGACY_MODES = %i[lambda procarg0 encoding index arg_inside_procarg0 forward_arg].freeze
 
     def runner_name
       raise NotImplementedError, "implement #{self.class}##{__callee__}"

test/test_parser.rb

@@ -7811,7 +7811,8 @@ def test_circular_argument_reference_error
     end
   end
 
-  def test_forward_args
+  def test_forward_args_legacy
+    Parser::Builders::Default.emit_forward_arg = false
     assert_parses(
       s(:def, :foo,
         s(:forward_args),
@@ -7843,7 +7844,27 @@ def test_forward_args
       %q{def foo(...); end},
       %q{},
       SINCE_2_7)
+  ensure
+    Parser::Builders::Default.emit_forward_arg = true
+  end
 
+  def test_forward_arg
+    assert_parses(
+      s(:def, :foo,
+        s(:args,
+          s(:forward_arg)),
+        s(:send, nil, :bar,
+          s(:forwarded_args))),
+      %q{def foo(...); bar(...); end},
+      %q{       ~ begin (args)
+        |       ~~~~~ expression (args)
+        |           ~ end (args)
+        |        ~~~ expression (args.forward_arg)
+        |                  ~~~ expression (send.forwarded_args)},
+      SINCE_2_7)
+  end
+
+  def test_forward_args_invalid
     assert_diagnoses(
       [:error, :block_and_blockarg],
       %q{def foo(...) bar(...) { }; end},
@@ -9556,7 +9577,10 @@ def test_endless_method
         |               ^ assignment
         |~~~~~~~~~~~~~~~~~~~~~~ expression},
       SINCE_2_8)
+  end
 
+  def test_endless_method_forwarded_args_legacy
+    Parser::Builders::Default.emit_forward_arg = false
     assert_parses(
       s(:def_e, :foo,
         s(:forward_args),
@@ -9568,6 +9592,7 @@ def test_endless_method
         |             ^ assignment
         |~~~~~~~~~~~~~~~~~~~~~~~ expression},
       SINCE_2_8)
+    Parser::Builders::Default.emit_forward_arg = true
   end
 
   def test_endless_method_without_brackets