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

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

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
49 changes: 43 additions & 6 deletions lib/parser/builders/default.rb
Expand Up @@ -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)

Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand Down Expand Up @@ -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)).
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/parser/max_numparam_stack.rb
Expand Up @@ -18,7 +18,7 @@ def has_ordinary_params?
end

def has_numparams?
top > 0
top && top > 0
end

def register(numparam)
Expand Down
1 change: 1 addition & 0 deletions lib/parser/messages.rb
Expand Up @@ -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}',
Expand Down
173 changes: 171 additions & 2 deletions test/test_parser.rb
Expand Up @@ -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' }],
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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