Emit sensible errors for invalid operator tokens (#176)

Here we emit invalid token errors during the token validation pass. This
ensures any invalid token after parsing is guarenteed to have one single
error emitted for it, independent from how it's handled by the parser.
Add the special K"Error**" kind to allow us to emit a specific error for
the `**` operator.

Ensure we reject all the invalid operators which are rejected by the
reference parser including the following which were missing:
* `..+` and similar
* `<---`

Also add an extra rule for parsing invalid binary operators (at some
arbitrarily-chosen precedence) to improve the recovered parse tree.

Author: c42f

src/hooks.jl

@@ -35,14 +35,12 @@ function _incomplete_tag(n::SyntaxNode)
         return :none
     end
     # TODO: Check error hits last character
-    if kind(c) == K"error" && begin
+    if kind(c) == K"ErrorEofMultiComment"
+        return :comment
+    elseif kind(c) == K"error" && begin
                 cs = children(c)
                 length(cs) > 0
             end
-        k1 = kind(cs[1])
-        if k1 == K"ErrorEofMultiComment"
-            return :comment
-        end
         for cc in cs
             if kind(cc) == K"error"
                 return :other

src/kinds.jl

@@ -16,8 +16,10 @@ const _kind_names =
         # Tokenization errors
         "ErrorEofMultiComment"
         "ErrorInvalidNumericConstant"
-        "ErrorInvalidOperator"
         "ErrorInvalidInterpolationTerminator"
+        "ErrorNumericOverflow"
+        "ErrorInvalidEscapeSequence"
+        "ErrorOverLongCharacter"
         # Generic error
         "error"
     "END_ERRORS"
@@ -94,6 +96,9 @@ const _kind_names =
     "END_DELIMITERS"
 
     "BEGIN_OPS"
+    "ErrorInvalidOperator"
+    "Error**"
+
     "..."
 
     # Level 1
@@ -1009,8 +1014,11 @@ const _nonunique_kind_names = Set([
 
     K"ErrorEofMultiComment"
     K"ErrorInvalidNumericConstant"
-    K"ErrorInvalidOperator"
     K"ErrorInvalidInterpolationTerminator"
+    K"ErrorNumericOverflow"
+    K"ErrorInvalidEscapeSequence"
+    K"ErrorOverLongCharacter"
+    K"ErrorInvalidOperator"
 
     K"Integer"
     K"BinInt"
@@ -1049,7 +1057,7 @@ end
 #-------------------------------------------------------------------------------
 # Predicates
 is_contextual_keyword(k::Kind) = K"BEGIN_CONTEXTUAL_KEYWORDS" < k < K"END_CONTEXTUAL_KEYWORDS"
-is_error(k::Kind) = K"BEGIN_ERRORS" < k < K"END_ERRORS"
+is_error(k::Kind) = K"BEGIN_ERRORS" < k < K"END_ERRORS" || k == K"ErrorInvalidOperator" || k == K"Error**"
 is_keyword(k::Kind) = K"BEGIN_KEYWORDS" < k < K"END_KEYWORDS"
 is_block_continuation_keyword(k::Kind) = K"BEGIN_BLOCK_CONTINUATION_KEYWORDS" < k < K"END_BLOCK_CONTINUATION_KEYWORDS"
 is_literal(k::Kind) = K"BEGIN_LITERAL" < k < K"END_LITERAL"

src/parse_stream.jl

@@ -72,7 +72,8 @@ function Base.summary(head::SyntaxHead)
 end
 
 function untokenize(head::SyntaxHead; unique=true, include_flag_suff=true)
-    str = is_error(kind(head)) ? "error" : untokenize(kind(head); unique=unique)::String
+    str = (is_error(kind(head)) ? untokenize(kind(head); unique=false) :
+           untokenize(kind(head); unique=unique))::String
     if is_dotted(head)
         str = "."*str
     end
@@ -850,7 +851,7 @@ end
 #-------------------------------------------------------------------------------
 # ParseStream Post-processing
 
-function validate_literal_tokens(stream::ParseStream)
+function validate_tokens(stream::ParseStream)
     text = sourcetext(stream)
     toks = stream.tokens
     charbuf = IOBuffer()
@@ -860,7 +861,7 @@ function validate_literal_tokens(stream::ParseStream)
         fbyte = toks[i-1].next_byte
         nbyte = t.next_byte
         lbyte = prevind(text, t.next_byte)
-        had_error = false
+        error_kind = K"None"
         if k in KSet"Integer BinInt OctInt HexInt"
             # The following shouldn't be able to error...
             # parse_int_literal
@@ -882,7 +883,7 @@ function validate_literal_tokens(stream::ParseStream)
             elseif code == :overflow
                 emit_diagnostic(stream, fbyte, lbyte,
                                 error="overflow in floating point literal")
-                had_error = true
+                error_kind = K"ErrorNumericOverflow"
             elseif underflow0
                 emit_diagnostic(stream, fbyte, lbyte,
                                 warning="underflow to zero in floating point literal")
@@ -892,21 +893,30 @@ function validate_literal_tokens(stream::ParseStream)
             truncate(charbuf, 0)
             had_error = unescape_julia_string(charbuf, text, fbyte,
                                               nbyte, stream.diagnostics)
-            if !had_error
+            if had_error
+                error_kind = K"ErrorInvalidEscapeSequence"
+            else
                 seek(charbuf,0)
                 read(charbuf, Char)
                 if !eof(charbuf)
-                    had_error = true
+                    error_kind = K"ErrorOverLongCharacter"
                     emit_diagnostic(stream, fbyte, lbyte,
                                     error="character literal contains multiple characters")
                 end
             end
         elseif k == K"String" && !has_flags(t, RAW_STRING_FLAG)
             had_error = unescape_julia_string(devnull, text, fbyte,
                                               nbyte, stream.diagnostics)
+            if had_error
+                error_kind = K"ErrorInvalidEscapeSequence"
+            end
+        elseif is_error(k) && k != K"error"
+            # Emit messages for non-generic token errors
+            emit_diagnostic(stream, fbyte, lbyte,
+                            error=Tokenize.TOKEN_ERROR_DESCRIPTION[k])
         end
-        if had_error
-            toks[i] = SyntaxToken(SyntaxHead(K"error", EMPTY_FLAGS),
+        if error_kind != K"None"
+            toks[i] = SyntaxToken(SyntaxHead(error_kind, EMPTY_FLAGS),
                                   t.orig_kind, t.next_byte)
         end
     end

src/parser.jl

@@ -829,15 +829,15 @@ end
 # flisp: parse-range
 function parse_range(ps::ParseState)
     mark = position(ps)
-    parse_expr(ps)
+    parse_invalid_ops(ps)
     initial_tok = peek_token(ps)
     initial_kind = kind(initial_tok)
     if initial_kind != K":" && is_prec_colon(initial_kind)
         # a..b     ==>   (call-i a .. b)
         # a … b    ==>   (call-i a … b)
         # a .… b    ==>  (dotcall-i a … b)
         bump_dotsplit(ps)
-        parse_expr(ps)
+        parse_invalid_ops(ps)
         emit(ps, mark, is_dotted(initial_tok) ? K"dotcall" : K"call", INFIX_FLAG)
     elseif initial_kind == K":" && ps.range_colon_enabled
         # a ? b : c:d   ==>   (? a b (call-i c : d))
@@ -864,7 +864,7 @@ function parse_range(ps::ParseState)
                 bump(ps) # K"<" or K">"
                 emit(ps, emark, K"error",
                      error="Invalid `:$ks` found, maybe replace with `$ks:`")
-                parse_expr(ps)
+                parse_invalid_ops(ps)
                 emit(ps, mark, K"call", INFIX_FLAG)
                 break
             end
@@ -891,7 +891,7 @@ function parse_range(ps::ParseState)
                 emit(ps, mark, K"call", INFIX_FLAG)
                 return
             end
-            parse_expr(ps)
+            parse_invalid_ops(ps)
             if n_colons == 2
                 emit(ps, mark, K"call", INFIX_FLAG)
                 n_colons = 0
@@ -911,6 +911,23 @@ function parse_range(ps::ParseState)
     end
 end
 
+# Parse invalid binary operators
+#
+# Having this is unnecessary, but it improves error messages and the
+# error-containing parse tree.
+#
+# a--b  ==>  (call-i a (error) b)
+function parse_invalid_ops(ps::ParseState)
+    mark = position(ps)
+    parse_expr(ps)
+    while (t = peek_token(ps); kind(t) in KSet"ErrorInvalidOperator Error**")
+        bump_trivia(ps)
+        bump_dotsplit(ps)
+        parse_expr(ps)
+        emit(ps, mark, is_dotted(t) ? K"dotcall" : K"call", INFIX_FLAG)
+    end
+end
+
 # a - b - c  ==>  (call-i (call-i a - b) - c)
 # a + b + c  ==>  (call-i a + b c)
 # a .+ b     ==>  (dotcall-i a + b)
@@ -3518,6 +3535,10 @@ function parse_atom(ps::ParseState, check_identifiers=true)
               "premature end of input" :
               "unexpected closing token"
         bump_invisible(ps, K"error", error=msg)
+    elseif is_error(leading_kind)
+        # Errors for bad tokens are emitted in validate_tokens() rather than
+        # here.
+        bump(ps)
     else
         bump(ps, error="invalid syntax atom")
     end

src/parser_api.jl

@@ -51,7 +51,7 @@ function parse!(stream::ParseStream; rule::Symbol=:toplevel)
     else
         throw(ArgumentError("Unknown grammar rule $rule"))
     end
-    validate_literal_tokens(stream)
+    validate_tokens(stream)
     stream
 end
 

src/tokenize.jl

@@ -16,9 +16,13 @@ include("tokenize_utils.jl")
 TOKEN_ERROR_DESCRIPTION = Dict{Kind, String}(
     K"ErrorEofMultiComment" => "unterminated multi-line comment #= ... =#",
     K"ErrorInvalidNumericConstant" => "invalid numeric constant",
-    K"ErrorInvalidOperator" => "invalid operator",
     K"ErrorInvalidInterpolationTerminator" => "interpolated variable ends with invalid character; use `\$(...)` instead",
-    K"error" => "unknown error",
+    K"ErrorNumericOverflow"=>"overflow in numeric literal",
+    K"ErrorInvalidEscapeSequence"=>"invalid string escape sequence",
+    K"ErrorOverLongCharacter"=>"character literal contains multiple characters",
+    K"ErrorInvalidOperator" => "invalid operator",
+    K"Error**" => "use `x^y` instead of `x**y` for exponentiation, and `x...` instead of `**x` for splatting",
+    K"error" => "unknown error token",
 )
 
 struct Token
@@ -618,10 +622,14 @@ function lex_less(l::Lexer)
         return emit(l, K"<|")
     elseif dpeekchar(l) == ('-', '-')
         readchar(l); readchar(l)
-        if accept(l, '>')
-            return emit(l, K"<-->")
+        if accept(l, '-')
+            return emit_error(l, K"ErrorInvalidOperator")
         else
-            return emit(l, K"<--")
+            if accept(l, '>')
+                return emit(l, K"<-->")
+            else
+                return emit(l, K"<--")
+            end
         end
     else
         return emit(l, K"<")
@@ -713,7 +721,7 @@ end
 
 function lex_star(l::Lexer)
     if accept(l, '*')
-        return emit_error(l, K"ErrorInvalidOperator") # "**" is an invalid operator use ^
+        return emit_error(l, K"Error**") # "**" is an invalid operator use ^
     elseif accept(l, '=')
         return emit(l, K"*=")
     end
@@ -811,7 +819,7 @@ function lex_digit(l::Lexer, kind)
             accept(l, "+-−")
             if accept_batch(l, isdigit)
                 pc,ppc = dpeekchar(l)
-                if pc === '.' && !dotop2(ppc, ' ')
+                if pc === '.' && !dotop2(ppc)
                     accept(l, '.')
                     return emit_error(l, K"ErrorInvalidNumericConstant")
                 end
@@ -829,7 +837,7 @@ function lex_digit(l::Lexer, kind)
         accept(l, "+-−")
         if accept_batch(l, isdigit)
             pc,ppc = dpeekchar(l)
-            if pc === '.' && !dotop2(ppc, ' ')
+            if pc === '.' && !dotop2(ppc)
                 accept(l, '.')
                 return emit_error(l, K"ErrorInvalidNumericConstant")
             end
@@ -959,7 +967,12 @@ function lex_dot(l::Lexer)
         if accept(l, '.')
             return emit(l, K"...")
         else
-            return emit(l, K"..")
+            if dotop2(peekchar(l))
+                readchar(l)
+                return emit_error(l, K"ErrorInvalidOperator")
+            else
+                return emit(l, K"..")
+            end
         end
     elseif Base.isdigit(peekchar(l))
         return lex_digit(l, K"Float")

src/tokenize_utils.jl

@@ -147,25 +147,23 @@ readchar(io::IO) = eof(io) ? EOF_CHAR : read(io, Char)
     0x0000ffe9 <= c <= 0x0000ffec
 end
 
-function dotop2(pc, dpc)
+function dotop2(pc)
     dotop1(pc) ||
     pc =='+' ||
     pc =='-' ||
+    pc =='−' ||
     pc =='*' ||
     pc =='/' ||
     pc =='\\' ||
     pc =='^' ||
     pc =='<' ||
     pc =='>' ||
-    pc =='&' && dpc === '=' ||
     pc =='&' ||
     pc =='%' ||
-    pc == '=' && dpc != '>' ||
-    pc == '|' && dpc != '|' ||
-    pc == '!' && dpc == '=' ||
+    pc == '=' ||
+    pc == '|' ||
     pc == '⊻' ||
-    pc == '÷' ||
-    pc == '=' && dpc == '>'
+    pc == '÷'
 end
 
 # suffix operators

test/parser.jl

@@ -1,7 +1,7 @@
 function test_parse(production, code; v=v"1.6", expr=false)
     stream = ParseStream(code, version=v)
     production(ParseState(stream))
-    JuliaSyntax.validate_literal_tokens(stream)
+    JuliaSyntax.validate_tokens(stream)
     t = build_tree(GreenNode, stream, wrap_toplevel_as_kind=K"None")
     source = SourceFile(code)
     s = SyntaxNode(source, t)
@@ -127,6 +127,9 @@ tests = [
         "x:y..."   => "(... (call-i x : y))"
         "x..y..."  => "(... (call-i x .. y))"
     ],
+    JuliaSyntax.parse_invalid_ops => [
+        "a--b"  =>  "(call-i a (ErrorInvalidOperator) b)"
+    ],
     JuliaSyntax.parse_expr => [
         "a - b - c"  => "(call-i (call-i a - b) - c)"
         "a + b + c"  => "(call-i a + b c)"
@@ -870,11 +873,11 @@ tests = [
     ],
     JuliaSyntax.parse_atom => [
         # errors in literals
-        "\"\\xqqq\""  =>  "(string (error))"
-        "'ab'"        =>  "(char (error))"
-        "'\\xq'"      =>  "(char (error))"
-        "10.0e1000'"  =>  "(error)"
-        "10.0f100'"   =>  "(error)"
+        "\"\\xqqq\""  =>  "(string (ErrorInvalidEscapeSequence))"
+        "'\\xq'"      =>  "(char (ErrorInvalidEscapeSequence))"
+        "'ab'"        =>  "(char (ErrorOverLongCharacter))"
+        "10.0e1000'"  =>  "(ErrorNumericOverflow)"
+        "10.0f100'"   =>  "(ErrorNumericOverflow)"
     ],
     JuliaSyntax.parse_docstring => [
         """ "notdoc" ]        """ => "(string \"notdoc\")"

test/test_utils.jl

@@ -311,7 +311,7 @@ for debugging.
 function itest_parse(production, code; version::VersionNumber=v"1.6")
     stream = ParseStream(code; version=version)
     production(JuliaSyntax.ParseState(stream))
-    JuliaSyntax.validate_literal_tokens(stream)
+    JuliaSyntax.validate_tokens(stream)
     t = JuliaSyntax.build_tree(GreenNode, stream, wrap_toplevel_as_kind=K"toplevel")
 
     println(stdout, "# Code:\n$code\n")

test/tokenize.jl

@@ -231,6 +231,7 @@ end
 
     @test toks("#=# text=#") == ["#=# text=#"=>K"Comment"]
 
+    @test toks("#=   #=   =#") == ["#=   #=   =#"=>K"ErrorEofMultiComment"]
     @test toks("#=#==#=#") == ["#=#==#=#"=>K"Comment"]
     @test toks("#=#==#=")  == ["#=#==#="=>K"ErrorEofMultiComment"]
 end
@@ -316,11 +317,6 @@ end
     @test length(collect(tokenize("x)"))) == 3
 end
 
-@testset "errors" begin
-    @test tok("#=   #=   =#", 1).kind == K"ErrorEofMultiComment"
-    @test tok("aa **",        3).kind == K"ErrorInvalidOperator"
-end
-
 @testset "xor_eq" begin
     @test tok("1 ⊻= 2", 3).kind==K"⊻="
 end
@@ -772,7 +768,11 @@ end
     test_error(tok("0b3",1),     K"ErrorInvalidNumericConstant")
     test_error(tok("0op",1),     K"ErrorInvalidNumericConstant")
     test_error(tok("--",1),      K"ErrorInvalidOperator")
-    test_error(tok("1**2",2),    K"ErrorInvalidOperator")
+
+    @test toks("1**2") == ["1"=>K"Integer", "**"=>K"Error**", "2"=>K"Integer"]
+    @test toks("a<---b") == ["a"=>K"Identifier", "<---"=>K"ErrorInvalidOperator", "b"=>K"Identifier"]
+    @test toks("a..+b") == ["a"=>K"Identifier", "..+"=>K"ErrorInvalidOperator", "b"=>K"Identifier"]
+    @test toks("a..−b") == ["a"=>K"Identifier", "..−"=>K"ErrorInvalidOperator", "b"=>K"Identifier"]
 end
 
 @testset "hat suffix" begin