Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- ruby31.y: handle local variables as hash labels with omitted values #820

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 36 additions & 0 deletions doc/AST_FORMAT.md
Expand Up @@ -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:
Expand Down
53 changes: 44 additions & 9 deletions lib/parser/builders/default.rb
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/parser/messages.rb
Expand Up @@ -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',
Expand Down
86 changes: 45 additions & 41 deletions lib/parser/ruby27.y
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
86 changes: 45 additions & 41 deletions lib/parser/ruby30.y
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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