+ ruby28.y: reject assignment to numparam. (#725)

This commit tracks upstream commit ruby/ruby@d47e124.

lib/parser/builders/default.rb

@@ -589,7 +589,11 @@ def assignable(node)
       when :ident
         name, = *node
 
-        check_assignment_to_numparam(node)
+        var_name = node.children[0].to_s
+        name_loc = node.loc.expression
+
+        check_assignment_to_numparam(var_name, name_loc)
+        check_reserved_for_numparam(var_name, name_loc)
 
         @parser.static_env.declare(name)
 
@@ -689,20 +693,25 @@ def def_module(module_t, name,
 
     def def_method(def_t, name_t, args,
                    body, end_t)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:def, [ value(name_t).to_sym, args, body ],
         definition_map(def_t, nil, name_t, end_t))
     end
 
     def def_endless_method(def_t, name_t, args,
                            assignment_t, body)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:def, [ value(name_t).to_sym, args, body ],
         endless_definition_map(def_t, nil, name_t, assignment_t, body))
     end
 
     def def_singleton(def_t, definee, dot_t,
                       name_t, args,
                       body, end_t)
-      return unless validate_definee(definee)
+      validate_definee(definee)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
 
       n(:defs, [ definee, value(name_t).to_sym, args, body ],
         definition_map(def_t, dot_t, name_t, end_t))
@@ -711,7 +720,8 @@ def def_singleton(def_t, definee, dot_t,
     def def_endless_singleton(def_t, definee, dot_t,
                               name_t, args,
                               assignment_t, body)
-      return unless validate_definee(definee)
+      validate_definee(definee)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
 
       n(:defs, [ definee, value(name_t).to_sym, args, body ],
         endless_definition_map(def_t, dot_t, name_t, assignment_t, body))
@@ -756,11 +766,15 @@ def forward_arg(dots_t)
     end
 
     def arg(name_t)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:arg, [ value(name_t).to_sym ],
         variable_map(name_t))
     end
 
     def optarg(name_t, eql_t, value)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:optarg, [ value(name_t).to_sym, value ],
         variable_map(name_t).
           with_operator(loc(eql_t)).
@@ -769,6 +783,7 @@ def optarg(name_t, eql_t, value)
 
     def restarg(star_t, name_t=nil)
       if name_t
+        check_reserved_for_numparam(value(name_t), loc(name_t))
         n(:restarg, [ value(name_t).to_sym ],
           arg_prefix_map(star_t, name_t))
       else
@@ -778,17 +793,23 @@ def restarg(star_t, name_t=nil)
     end
 
     def kwarg(name_t)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:kwarg, [ value(name_t).to_sym ],
         kwarg_map(name_t))
     end
 
     def kwoptarg(name_t, value)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:kwoptarg, [ value(name_t).to_sym, value ],
         kwarg_map(name_t, value))
     end
 
     def kwrestarg(dstar_t, name_t=nil)
       if name_t
+        check_reserved_for_numparam(value(name_t), loc(name_t))
+
         n(:kwrestarg, [ value(name_t).to_sym ],
           arg_prefix_map(dstar_t, name_t))
       else
@@ -803,11 +824,15 @@ def kwnilarg(dstar_t, nil_t)
     end
 
     def shadowarg(name_t)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:shadowarg, [ value(name_t).to_sym ],
         variable_map(name_t))
     end
 
     def blockarg(amper_t, name_t)
+      check_reserved_for_numparam(value(name_t), loc(name_t))
+
       n(:blockarg, [ value(name_t).to_sym ],
         arg_prefix_map(amper_t, name_t))
     end
@@ -1560,16 +1585,28 @@ def check_duplicate_arg(this_arg, map={})
       end
     end
 
-    def check_assignment_to_numparam(node)
-      name = node.children[0].to_s
+    def check_assignment_to_numparam(name, loc)
+      # MRI < 2.7 treats numbered parameters as regular variables
+      # and so it's allowed to perform assignments like `_1 = 42`.
+      return if @parser.version < 27
 
       assigning_to_numparam =
         @parser.context.in_dynamic_block? &&
         name =~ /\A_([1-9])\z/ &&
         @parser.max_numparam_stack.has_numparams?
 
       if assigning_to_numparam
-        diagnostic :error, :cant_assign_to_numparam, { :name => name }, node.loc.expression
+        diagnostic :error, :cant_assign_to_numparam, { :name => name }, loc
+      end
+    end
+
+    def check_reserved_for_numparam(name, loc)
+      # MRI < 2.8 accepts assignemnt to variables like _1
+      # if it's not a numbererd parameter. MRI 2.8 and newer throws an error.
+      return if @parser.version < 28
+
+      if name =~ /\A_([1-9])\z/
+        diagnostic :error, :reserved_for_numparam, { :name => name }, loc
       end
     end
 

lib/parser/max_numparam_stack.rb

@@ -18,7 +18,7 @@ def has_ordinary_params?
     end
 
     def has_numparams?
-      top > 0
+      top && top > 0
     end
 
     def register(numparam)

lib/parser/messages.rb

@@ -65,6 +65,7 @@ module Parser
     :invalid_return               => 'Invalid return in class/module body',
     :csend_in_lhs_of_masgn        => '&. inside multiple assignment destination',
     :cant_assign_to_numparam      => 'cannot assign to numbered parameter %{name}',
+    :reserved_for_numparam        => '%{name} is reserved for numbered parameter',
     :ordinary_param_defined       => 'ordinary parameter is defined',
     :numparam_used_in_outer_scope => 'numbered parameter is already used in an outer scope',
     :circular_argument_reference  => 'circular argument reference %{var_name}',

test/test_parser.rb

@@ -7514,7 +7514,7 @@ def test_assignment_to_numparams
           s(:nil))),
       %q{proc {_1 = nil}},
       %q{},
-      SINCE_2_7)
+      %w(2.7))
 
     assert_diagnoses(
       [:error, :cant_assign_to_numparam, { :name => '_1' }],
@@ -7542,7 +7542,7 @@ def test_assignment_to_numparams
 
     refute_diagnoses(
       %q{proc { _1 = nil; _1}},
-      SINCE_2_7)
+      %w(2.7))
   end
 
   def test_numparams_in_nested_blocks
@@ -9806,4 +9806,173 @@ def test_invalid_source
       assert_nil(ast)
     end
   end
+
+  def test_reserved_for_numparam__before_28
+    assert_parses(
+      s(:block,
+        s(:send, nil, :proc),
+        s(:args),
+        s(:lvasgn, :_1,
+          s(:nil))),
+      %q{proc {_1 = nil}},
+      %q{},
+      ALL_VERSIONS - SINCE_2_8)
+
+    assert_parses(
+      s(:lvasgn, :_2,
+        s(:int, 1)),
+      %q{_2 = 1},
+      %q{},
+      ALL_VERSIONS - SINCE_2_8)
+
+    assert_parses(
+      s(:block,
+        s(:send, nil, :proc),
+        s(:args,
+          s(:procarg0,
+            s(:arg, :_3))), nil),
+      %q{proc {|_3|}},
+      %q{},
+      SINCE_1_9 - SINCE_2_8)
+
+    assert_parses(
+      s(:def, :x,
+        s(:args,
+          s(:arg, :_4)), nil),
+      %q{def x(_4) end},
+      %q{},
+      ALL_VERSIONS - SINCE_2_8)
+
+    assert_parses(
+      s(:def, :_5,
+        s(:args), nil),
+      %q{def _5; end},
+      %q{},
+      ALL_VERSIONS - SINCE_2_8)
+
+    assert_parses(
+      s(:defs,
+        s(:self), :_6,
+        s(:args), nil),
+      %q{def self._6; end},
+      %q{},
+      ALL_VERSIONS - SINCE_2_8)
+  end
+
+  def test_reserved_for_numparam__since_28
+    # Regular assignments:
+
+    assert_diagnoses(
+      [:error, :reserved_for_numparam, { :name => '_1' }],
+      %q{proc {_1 = nil}},
+      %q{      ^^ location},
+      SINCE_2_8)
+
+    assert_diagnoses(
+      [:error, :reserved_for_numparam, { :name => '_2' }],
+      %q{_2 = 1},
+      %q{^^ location},
+      SINCE_2_8)
+
+    # Arguments:
+
+    [
+      # req (procarg0)
+      [
+        %q{proc {|_3|}},
+        %q{       ^^ location},
+      ],
+
+      # req
+      [
+        %q{proc {|_3,|}},
+        %q{       ^^ location},
+      ],
+
+      # opt
+      [
+        %q{proc {|_3 = 42|}},
+        %q{       ^^ location},
+      ],
+
+      # mlhs
+      [
+        %q{proc {|(_3)|}},
+        %q{        ^^ location},
+      ],
+
+      # rest
+      [
+        %q{proc {|*_3|}},
+        %q{        ^^ location},
+      ],
+
+      # kwarg
+      [
+        %q{proc {|_3:|}},
+        %q{       ^^^ location},
+      ],
+
+      # kwoptarg
+      [
+        %q{proc {|_3: 42|}},
+        %q{       ^^^ location},
+      ],
+
+      # kwrestarg
+      [
+        %q{proc {|**_3|}},
+        %q{         ^^ location},
+      ],
+
+      # block
+      [
+        %q{proc {|&_3|}},
+        %q{        ^^ location},
+      ],
+
+      # shadowarg
+      [
+        %q{proc {|;_3|}},
+        %q{        ^^ location},
+      ],
+    ].each do |(code, location)|
+      assert_diagnoses(
+        [:error, :reserved_for_numparam, { :name => '_3' }],
+        code,
+        location,
+        SINCE_2_8)
+    end
+
+    # Method definitions:
+
+    [
+      # regular method
+      [
+        %q{def _5; end},
+        %q{    ^^ location}
+      ],
+      # regular singleton method
+      [
+        %q{def self._5; end},
+        %q{         ^^ location}
+      ],
+      # endless method
+      [
+        %q{def _5() = nil},
+        %q{    ^^ location}
+      ],
+      # endless singleton method
+      [
+        %q{def self._5() = nil},
+        %q{         ^^ location}
+      ],
+    ].each do |(code, location)|
+      assert_diagnoses(
+        [:error, :reserved_for_numparam, { :name => '_5' }],
+        code,
+        location,
+        SINCE_2_8)
+    end
+  end
 end