Skip to content

Commit

Permalink
Parser: disallow arguments after double splat. Fixes #4965
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite authored and Martin Verzilli committed Sep 16, 2017
1 parent cd03b6f commit eb1f102
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 10 deletions.
16 changes: 13 additions & 3 deletions spec/compiler/parser/parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ describe "Parser" do
it_parses "def foo(**args : Foo)\n1\nend", Def.new("foo", body: 1.int32, double_splat: Arg.new("args", restriction: "Foo".path))
it_parses "def foo(**args : **Foo)\n1\nend", Def.new("foo", body: 1.int32, double_splat: Arg.new("args", restriction: DoubleSplat.new("Foo".path)))

assert_syntax_error "def foo(**args, **args2)"
assert_syntax_error "def foo(**args, **args2); end", "only block argument is allowed after double splat"
assert_syntax_error "def foo(**args, x); end", "only block argument is allowed after double splat"
assert_syntax_error "def foo(**args, *x); end", "only block argument is allowed after double splat"

it_parses "def foo(x y); y; end", Def.new("foo", args: [Arg.new("y", external_name: "x")], body: "y".var)
it_parses "def foo(x @var); end", Def.new("foo", [Arg.new("var", external_name: "x")], [Assign.new("@var".instance_var, "var".var)] of ASTNode)
Expand All @@ -257,6 +259,8 @@ describe "Parser" do
it_parses "macro foo(**args)\n1\nend", Macro.new("foo", body: MacroLiteral.new("1\n"), double_splat: "args".arg)

assert_syntax_error "macro foo(x, *); 1; end", "named arguments must follow bare *"
assert_syntax_error "macro foo(**x, **y)", "only block argument is allowed after double splat"
assert_syntax_error "macro foo(**x, y)", "only block argument is allowed after double splat"

it_parses "abstract def foo", Def.new("foo", abstract: true)
it_parses "abstract def foo; 1", [Def.new("foo", abstract: true), 1.int32]
Expand Down Expand Up @@ -1120,8 +1124,14 @@ describe "Parser" do
it_parses "foo 1, **bar, &block", Call.new(nil, "foo", args: [1.int32, DoubleSplat.new("bar".call)], block_arg: "block".call)
it_parses "foo(1, **bar, &block)", Call.new(nil, "foo", args: [1.int32, DoubleSplat.new("bar".call)], block_arg: "block".call)

# assert_syntax_error "foo **bar, 1"
# assert_syntax_error "foo(**bar, 1)"
assert_syntax_error "foo **bar, 1", "argument not allowed after double splat"
assert_syntax_error "foo(**bar, 1)", "argument not allowed after double splat"

assert_syntax_error "foo **bar, *x", "splat not allowed after double splat"
assert_syntax_error "foo(**bar, *x)", "splat not allowed after double splat"

assert_syntax_error "foo **bar, out x", "out argument not allowed after double splat"
assert_syntax_error "foo(**bar, out x)", "out argument not allowed after double splat"

it_parses "private def foo; end", VisibilityModifier.new(Visibility::Private, Def.new("foo"))
it_parses "protected def foo; end", VisibilityModifier.new(Visibility::Protected, Def.new("foo"))
Expand Down
31 changes: 24 additions & 7 deletions src/compiler/crystal/syntax/parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -3327,6 +3327,10 @@ module Crystal
return ArgExtras.new(block_arg, false, false, false)
end

if found_double_splat
raise "only block argument is allowed after double splat"
end

splat = false
double_splat = false
arg_location = @token.location
Expand All @@ -3342,10 +3346,6 @@ module Crystal
allow_external_name = false
next_token_skip_space
when :"**"
if found_double_splat
unexpected_token
end

double_splat = true
allow_external_name = false
next_token_skip_space
Expand Down Expand Up @@ -3918,6 +3918,7 @@ module Crystal
# We found a prentheses, so calls inside it will get the `do`
# attached to them
@stop_on_do = false
found_double_splat = false

next_token_skip_space_or_newline
while @token.type != :")"
Expand All @@ -3928,11 +3929,12 @@ module Crystal
if @token.type == :IDENT && current_char == ':'
return parse_call_args_named_args(@token.location, args, first_name: nil, allow_newline: true)
else
arg = parse_call_arg
arg = parse_call_arg(found_double_splat)
if @token.type == :":" && arg.is_a?(StringLiteral)
return parse_call_args_named_args(arg.location.not_nil!, args, first_name: arg.value, allow_newline: true)
else
args << arg
found_double_splat = arg.is_a?(DoubleSplat)
end
end

Expand Down Expand Up @@ -4028,6 +4030,8 @@ module Crystal
# never to `return`.
@stop_on_do = true unless control

found_double_splat = false

while @token.type != :NEWLINE && @token.type != :";" && @token.type != :EOF && @token.type != end_token && @token.type != :":" && !end_token?
if call_block_arg_follows?
return parse_call_block_arg(args, false)
Expand All @@ -4036,11 +4040,12 @@ module Crystal
if @token.type == :IDENT && current_char == ':'
return parse_call_args_named_args(@token.location, args, first_name: nil, allow_newline: false)
else
arg = parse_call_arg
arg = parse_call_arg(found_double_splat)
if @token.type == :":" && arg.is_a?(StringLiteral)
return parse_call_args_named_args(arg.location.not_nil!, args, first_name: arg.value, allow_newline: false)
else
args << arg
found_double_splat = arg.is_a?(DoubleSplat)
end
end_location = arg.end_location
end
Expand Down Expand Up @@ -4126,14 +4131,22 @@ module Crystal
named_args
end

def parse_call_arg
def parse_call_arg(found_double_splat = false)
if @token.keyword?(:out)
if found_double_splat
raise "out argument not allowed after double splat"
end

parse_out
else
splat = nil
case @token.type
when :"*"
unless current_char.ascii_whitespace?
if found_double_splat
raise "splat not allowed after double splat"
end

splat = :single
next_token
end
Expand All @@ -4146,6 +4159,10 @@ module Crystal

arg = parse_op_assign_no_control

if found_double_splat && splat != :double
raise "argument not allowed after double splat", arg.location.not_nil!
end

case splat
when :single
arg = Splat.new(arg).at(arg.location)
Expand Down

0 comments on commit eb1f102

Please sign in to comment.