- ruby31.y: fix parsing of hash pairs with lvar/const keys

doc/AST_FORMAT.md

@@ -266,6 +266,42 @@ Format:
  ~~~~~~~~~~ expression (pair)
 ~~~
 
+#### With local variable
+
+Format:
+
+~~~
+(pair (sym :foo) (lvar :foo))
+"{foo:}"
+     ^ operator (pair)
+  ~~~ expression (sym)
+  ~~~ expression (lvar)
+~~~
+
+#### With constant
+
+Format:
+
+~~~
+(pair (sym :foo) (const nil :foo))
+"{FOO:}"
+     ^ operator (pair)
+  ~~~ expression (const)
+  ~~~ expression (lvar)
+~~~
+
+#### With method call
+
+Format:
+
+~~~
+(pair (sym :puts) (send nil :puts))
+"{puts:}"
+      ^ operator (pair)
+  ~~~~ expression (sym)
+  ~~~~ expression (send)
+~~~
+
 #### Plain
 
 Format:

lib/parser/builders/default.rb

@@ -518,6 +518,20 @@ def pair_quoted(begin_t, parts, end_t, value)
       n(:pair, [ key, value ], pair_map)
     end
 
+    def pair_label(key_t)
+      key_l = loc(key_t)
+      value_l = key_l.adjust(end_pos: -1)
+
+      label = value(key_t)
+      value =
+        if label =~ /\A[[:lower:]]/
+          n(:ident, [ label.to_sym ], Source::Map::Variable.new(value_l))
+        else
+          n(:const, [ nil, label.to_sym ], Source::Map::Constant.new(nil, value_l, value_l))
+        end
+      pair_keyword(key_t, accessible(value))
+    end
+
     def kwsplat(dstar_t, arg)
       n(:kwsplat, [ arg ],
         unary_op_map(dstar_t, arg))
@@ -608,19 +622,29 @@ def accessible(node)
       when :ident
         name, = *node
 
-        if @parser.static_env.declared?(name)
-          if name.to_s == parser.current_arg_stack.top
-            diagnostic :error, :circular_argument_reference,
-                       { :var_name => name.to_s }, node.loc.expression
-          end
+        if %w[? !].any? { |c| name.to_s.end_with?(c) }
+          diagnostic :error, :invalid_id_to_get,
+                     { :identifier => name.to_s }, node.loc.expression
+        end
 
-          node.updated(:lvar)
-        else
-          name, = *node
-          n(:send, [ nil, name ],
+        # Numbered parameters are not declared anywhere,
+        # so they take precedence over method calls in numblock contexts
+        if @parser.version >= 27 && @parser.try_declare_numparam(node)
+          return node.updated(:lvar)
+        end
+
+        unless @parser.static_env.declared?(name)
+          return n(:send, [ nil, name ],
             var_send_map(node))
         end
 
+        if name.to_s == parser.current_arg_stack.top
+          diagnostic :error, :circular_argument_reference,
+                     { :var_name => name.to_s }, node.loc.expression
+        end
+
+        node.updated(:lvar)
+
       else
         node
       end
@@ -683,6 +707,17 @@ def assignable(node)
 
         node.updated(:lvasgn)
 
+      when :match_var
+        name, = *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)
+
+        node
+
       when :nil, :self, :true, :false,
            :__FILE__, :__LINE__, :__ENCODING__
         diagnostic :error, :invalid_assignment, nil, node.loc.expression

lib/parser/messages.rb

@@ -76,6 +76,7 @@ module Parser
     :duplicate_variable_name      => 'duplicate variable name %{name}',
     :duplicate_pattern_key        => 'duplicate hash pattern key %{name}',
     :endless_setter               => 'setter method cannot be defined in an endless method definition',
+    :invalid_id_to_get            => 'identifier %{identifier} is not valid to get',
 
     # Parser warnings
     :useless_else            => 'else without rescue is useless',

lib/parser/ruby27.y

@@ -2133,7 +2133,7 @@ opt_block_args_tail:
 
       p_variable: tIDENTIFIER
                     {
-                      result = @builder.match_var(val[0])
+                      result = @builder.assignable(@builder.match_var(val[0]))
                     }
 
        p_var_ref: tCARET tIDENTIFIER
@@ -2470,46 +2470,6 @@ keyword_variable: kNIL
 
          var_ref: user_variable
                     {
-                      if (node = val[0]) && node.type == :ident
-                        name = node.children[0]
-
-                        if name =~ /\A_[1-9]\z/ && !static_env.declared?(name) && context.in_dynamic_block?
-                          # definitely an implicit param
-                          location = node.loc.expression
-
-                          if max_numparam_stack.has_ordinary_params?
-                            diagnostic :error, :ordinary_param_defined, nil, [nil, location]
-                          end
-
-                          raw_context = context.stack.dup
-                          raw_max_numparam_stack = max_numparam_stack.stack.dup
-
-                          # ignore current block scope
-                          raw_context.pop
-                          raw_max_numparam_stack.pop
-
-                          raw_context.reverse_each do |outer_scope|
-                            if outer_scope == :block || outer_scope == :lambda
-                              outer_scope_has_numparams = raw_max_numparam_stack.pop > 0
-
-                              if outer_scope_has_numparams
-                                diagnostic :error, :numparam_used_in_outer_scope, nil, [nil, location]
-                              else
-                                # for now it's ok, but an outer scope can also be a block
-                                # with numparams, so we need to continue
-                              end
-                            else
-                              # found an outer scope that can't have numparams
-                              # like def/class/etc
-                              break
-                            end
-                          end
-
-                          static_env.declare(name)
-                          max_numparam_stack.register(name[1].to_i)
-                        end
-                      end
-
                       result = @builder.accessible(val[0])
                     }
                 | keyword_variable
@@ -2978,3 +2938,47 @@ require 'parser'
   def default_encoding
     Encoding::UTF_8
   end
+
+  def try_declare_numparam(node)
+    name = node.children[0]
+
+    if name =~ /\A_[1-9]\z/ && !static_env.declared?(name) && context.in_dynamic_block?
+      # definitely an implicit param
+      location = node.loc.expression
+
+      if max_numparam_stack.has_ordinary_params?
+        diagnostic :error, :ordinary_param_defined, nil, [nil, location]
+      end
+
+      raw_context = context.stack.dup
+      raw_max_numparam_stack = max_numparam_stack.stack.dup
+
+      # ignore current block scope
+      raw_context.pop
+      raw_max_numparam_stack.pop
+
+      raw_context.reverse_each do |outer_scope|
+        if outer_scope == :block || outer_scope == :lambda
+          outer_scope_has_numparams = raw_max_numparam_stack.pop > 0
+
+          if outer_scope_has_numparams
+            diagnostic :error, :numparam_used_in_outer_scope, nil, [nil, location]
+          else
+            # for now it's ok, but an outer scope can also be a block
+            # with numparams, so we need to continue
+          end
+        else
+          # found an outer scope that can't have numparams
+          # like def/class/etc
+          break
+        end
+      end
+
+      static_env.declare(name)
+      max_numparam_stack.register(name[1].to_i)
+
+      true
+    else
+      false
+    end
+  end

lib/parser/ruby30.y

@@ -2219,7 +2219,7 @@ opt_block_args_tail:
 
       p_variable: tIDENTIFIER
                     {
-                      result = @builder.match_var(val[0])
+                      result = @builder.assignable(@builder.match_var(val[0]))
                     }
 
        p_var_ref: tCARET tIDENTIFIER
@@ -2556,46 +2556,6 @@ keyword_variable: kNIL
 
          var_ref: user_variable
                     {
-                      if (node = val[0]) && node.type == :ident
-                        name = node.children[0]
-
-                        if name =~ /\A_[1-9]\z/ && !static_env.declared?(name) && context.in_dynamic_block?
-                          # definitely an implicit param
-                          location = node.loc.expression
-
-                          if max_numparam_stack.has_ordinary_params?
-                            diagnostic :error, :ordinary_param_defined, nil, [nil, location]
-                          end
-
-                          raw_context = context.stack.dup
-                          raw_max_numparam_stack = max_numparam_stack.stack.dup
-
-                          # ignore current block scope
-                          raw_context.pop
-                          raw_max_numparam_stack.pop
-
-                          raw_context.reverse_each do |outer_scope|
-                            if outer_scope == :block || outer_scope == :lambda
-                              outer_scope_has_numparams = raw_max_numparam_stack.pop > 0
-
-                              if outer_scope_has_numparams
-                                diagnostic :error, :numparam_used_in_outer_scope, nil, [nil, location]
-                              else
-                                # for now it's ok, but an outer scope can also be a block
-                                # with numparams, so we need to continue
-                              end
-                            else
-                              # found an outer scope that can't have numparams
-                              # like def/class/etc
-                              break
-                            end
-                          end
-
-                          static_env.declare(name)
-                          max_numparam_stack.register(name[1].to_i)
-                        end
-                      end
-
                       result = @builder.accessible(val[0])
                     }
                 | keyword_variable
@@ -3075,3 +3035,47 @@ require 'parser'
       diagnostic :error, :endless_setter, nil, name_t
     end
   end
+
+  def try_declare_numparam(node)
+    name = node.children[0]
+
+    if name =~ /\A_[1-9]\z/ && !static_env.declared?(name) && context.in_dynamic_block?
+      # definitely an implicit param
+      location = node.loc.expression
+
+      if max_numparam_stack.has_ordinary_params?
+        diagnostic :error, :ordinary_param_defined, nil, [nil, location]
+      end
+
+      raw_context = context.stack.dup
+      raw_max_numparam_stack = max_numparam_stack.stack.dup
+
+      # ignore current block scope
+      raw_context.pop
+      raw_max_numparam_stack.pop
+
+      raw_context.reverse_each do |outer_scope|
+        if outer_scope == :block || outer_scope == :lambda
+          outer_scope_has_numparams = raw_max_numparam_stack.pop > 0
+
+          if outer_scope_has_numparams
+            diagnostic :error, :numparam_used_in_outer_scope, nil, [nil, location]
+          else
+            # for now it's ok, but an outer scope can also be a block
+            # with numparams, so we need to continue
+          end
+        else
+          # found an outer scope that can't have numparams
+          # like def/class/etc
+          break
+        end
+      end
+
+      static_env.declare(name)
+      max_numparam_stack.register(name[1].to_i)
+
+      true
+    else
+      false
+    end
+  end