Skip to content

Commit

Permalink
Parser: give syntax error on comma after newline in argument list
Browse files Browse the repository at this point in the history
  • Loading branch information
@asterite
asterite committed Aug 9, 2018
1 parent eeb53c5 commit d9bdf9e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 34 deletions.
14 changes: 6 additions & 8 deletions spec/compiler/crystal/tools/playground_spec.cr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,29 +317,28 @@ describe Playground::AgentInstrumentorTransformer do
end

it "do not instrument top level macro calls" do
assert_agent(<<-CR
assert_agent(<<-FROM, <<-TO
macro bar
def foo
4
end
end
bar
foo
CR
, <<-CR
FROM
macro bar
def foo
4
end
end
bar
_p.i(7) { foo }
CR
TO
)
end

it "do not instrument class/module declared macro" do
assert_agent(<<-CR
assert_agent(<<-FROM, <<-TO
module Bar
macro bar
4
Expand All @@ -353,8 +352,7 @@ describe Playground::AgentInstrumentorTransformer do
8
end
end
CR
, <<-CR
FROM
module Bar
macro bar
4
Expand All @@ -368,7 +366,7 @@ describe Playground::AgentInstrumentorTransformer do
_p.i(11) { 8 }
end
end
CR
TO
)
end

Expand Down
14 changes: 12 additions & 2 deletions spec/compiler/parser/parser_spec.cr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ module Crystal
it_parses "def foo(\nvar); end", Def.new("foo", ["var".arg])
it_parses "def foo(\nvar\n); end", Def.new("foo", ["var".arg])
it_parses "def foo(var1, var2); end", Def.new("foo", ["var1".arg, "var2".arg])
it_parses "def foo(\nvar1\n,\nvar2\n)\n end", Def.new("foo", ["var1".arg, "var2".arg])
it_parses "def foo; 1; 2; end", Def.new("foo", body: [1.int32, 2.int32] of ASTNode)
it_parses "def foo=(value); end", Def.new("foo=", ["value".arg])
it_parses "def foo(n); foo(n -1); end", Def.new("foo", ["n".arg], "foo".call(Call.new("n".var, "-", 1.int32)))
Expand Down Expand Up @@ -261,7 +260,6 @@ module Crystal
it_parses "def foo(&@block); end", Def.new("foo", body: Assign.new("@block".instance_var, "block".var), block_arg: Arg.new("block"), yields: 0)

it_parses "def foo(\n&block\n); end", Def.new("foo", block_arg: Arg.new("block"), yields: 0)
it_parses "def foo(&block \n: Int ->); end", Def.new("foo", block_arg: Arg.new("block", restriction: ProcNotation.new(["Int".path] of ASTNode)), yields: 1)
it_parses "def foo(&block :\n Int ->); end", Def.new("foo", block_arg: Arg.new("block", restriction: ProcNotation.new(["Int".path] of ASTNode)), yields: 1)
it_parses "def foo(&block : Int ->\n); end", Def.new("foo", block_arg: Arg.new("block", restriction: ProcNotation.new(["Int".path] of ASTNode)), yields: 1)

Expand Down Expand Up @@ -1722,6 +1720,18 @@ module Crystal
assert_syntax_error "<<-HEREDOC", "Unexpected EOF on heredoc identifier"
assert_syntax_error "<<-HEREDOC\n", "Unterminated heredoc"

assert_syntax_error "[1\n,2]", "expecting token ']', not ','"
assert_syntax_error "{1\n,2}", "expecting token '}', not ','"
assert_syntax_error "{1, 2\n,3}", "expecting token '}', not ','"
assert_syntax_error "{1 => 2\n,3 => 4}", "expecting token '}', not ','"
assert_syntax_error "foo(1\n,2)", "expecting token ')', not ','"
assert_syntax_error "foo(a: 1\n,b: 2)", "expecting token ')', not ','"
assert_syntax_error "def foo(x\n,y); 1; end", "expecting token ')', not ','"
assert_syntax_error "macro foo(x\n,y); 1; end", "expecting token ')', not ','"
assert_syntax_error "class Foo(X\n,Y); 1; end", "expecting token ')', not ','"
assert_syntax_error "Foo(X\n,Y)", "expecting token ')', not ','"
assert_syntax_error "Foo(x: X\n,y: Y)", "expecting token ')', not ','"

it_parses "annotation Foo; end", AnnotationDef.new("Foo".path)
it_parses "annotation Foo\n\nend", AnnotationDef.new("Foo".path)
it_parses "annotation Foo::Bar\n\nend", AnnotationDef.new(Path.new(["Foo", "Bar"]))
Expand Down
47 changes: 23 additions & 24 deletions src/compiler/crystal/syntax/parser.cr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1558,9 +1558,12 @@ module Crystal

type_vars.push type_var_name

next_token_skip_space_or_newline
next_token_skip_space
if @token.type == :","
next_token_skip_space_or_newline
else
skip_space_or_newline
check :")"
end

index += 1
Expand Down Expand Up @@ -2179,11 +2182,6 @@ module Crystal
exps << parse_op_assign_no_control
end_location = token_end_location
skip_space
if @token.type == :NEWLINE
skip_space_or_newline
check :"]"
break
end

if @token.type == :","
slash_is_regex!
Expand Down Expand Up @@ -2301,11 +2299,6 @@ module Crystal
next_token_skip_space_or_newline
entries << HashLiteral::Entry.new(key, parse_op_assign)
skip_space
if @token.type == :NEWLINE
next_token_skip_space_or_newline
check :"}"
break
end
if @token.type == :","
slash_is_regex!
next_token_skip_space_or_newline
Expand Down Expand Up @@ -2339,7 +2332,7 @@ module Crystal
exps << first_exp
while @token.type != :"}"
exps << parse_op_assign_no_control
skip_space_or_newline
skip_space
if @token.type == :","
next_token_skip_space_or_newline
else
Expand Down Expand Up @@ -2839,9 +2832,7 @@ module Crystal
next_token_skip_space_or_newline
else
skip_space_or_newline
if @token.type != :")"
unexpected_token @token.to_s, "expected ',' or ')'"
end
check :")"
end
index += 1
end
Expand Down Expand Up @@ -3330,9 +3321,7 @@ module Crystal
next_token_skip_space_or_newline
else
skip_space_or_newline
if @token.type != :")"
unexpected_token @token.to_s, "expected ',' or ')'"
end
check :")"
end
index += 1
end
Expand Down Expand Up @@ -3729,7 +3718,7 @@ module Crystal
found_space = @token.type == :SPACE || @token.type == :NEWLINE
end

skip_space_or_newline
skip_space

{arg_name, external_name, found_space, uses_arg}
end
Expand Down Expand Up @@ -4165,11 +4154,12 @@ module Crystal
end
end

skip_space_or_newline
skip_space
if @token.type == :","
slash_is_regex!
next_token_skip_space_or_newline
else
skip_space_or_newline
check :")"
break
end
Expand Down Expand Up @@ -4343,12 +4333,15 @@ module Crystal
end

named_args << NamedArgument.new(name, value).at(location)
skip_space_or_newline if allow_newline
skip_space
if @token.type == :","
next_token_skip_space_or_newline
if @token.type == :")" || @token.type == :"&" || @token.type == :"]"
break
end
elsif @token.type == :NEWLINE && allow_newline
skip_space_or_newline
break
else
break
end
Expand Down Expand Up @@ -4550,11 +4543,17 @@ module Crystal
next_token_skip_space_or_newline

type = parse_single_type(allow_commas: false)
skip_space_or_newline
skip_space

named_args << NamedArgument.new(name, type)
break unless @token.type == :","
next_token_skip_space_or_newline

if @token.type == :","
next_token_skip_space_or_newline
else
skip_space_or_newline
check end_token
break
end
end

named_args
Expand Down

0 comments on commit d9bdf9e

Please sign in to comment.