Improve error message for missing closing tokens (#397)

When a missing closing token like `)`, `]` or `}` is encountered we want
the "Expected `)`" error to point to a location one past the last valid
token, not to the trailing error tokens.

For example from #349 here's a poor error message from the existing
code:

    ERROR: ParseError:
    # Error @ REPL[53]:15:5
        ylims!(p, (0, last(ylims(p)))
        xlabel!(p, "Contig length cutoff (kbp)")
    #   └─────────────────────────────────────┘ ── Expected `)`

After this change, the error location instead points to the end of the
last valid line:

    ERROR: ParseError:
    # Error @ REPL[53]:15:5
        ylims!(p, (0, last(ylims(p)))
    #                                └── Expected `)`

src/parser.jl

@@ -141,15 +141,20 @@ end
 #
 # Crude recovery heuristic: bump any tokens which aren't block or bracket
 # closing tokens.
-function bump_closing_token(ps, closing_kind)
+function bump_closing_token(ps, closing_kind, alternative_closer_hint=nothing)
     # todo: Refactor with recover() ?
-    bump_trivia(ps)
     if peek(ps) == closing_kind
+        bump_trivia(ps)
         bump(ps, TRIVIA_FLAG)
         return
     end
+    errmsg = "Expected `$(untokenize(closing_kind))`"
+    if !isnothing(alternative_closer_hint)
+        errmsg *= alternative_closer_hint
+    end
     # We didn't find the closing token. Read ahead in the stream
     mark = position(ps)
+    emit_diagnostic(ps, mark, mark, error=errmsg)
     while true
         k = peek(ps)
         if is_closing_token(ps, k) && !(k in KSet", ;")
@@ -158,8 +163,7 @@ function bump_closing_token(ps, closing_kind)
         bump(ps)
     end
     # mark as trivia => ignore in AST.
-    emit(ps, mark, K"error", TRIVIA_FLAG,
-         error="Expected `$(untokenize(closing_kind))`")
+    emit(ps, mark, K"error", TRIVIA_FLAG)
     if peek(ps) == closing_kind
         bump(ps, TRIVIA_FLAG)
     end
@@ -3101,7 +3105,6 @@ function parse_brackets(after_parse::Function,
     had_splat = false
     param_start = nothing
     while true
-        bump_trivia(ps)
         k = peek(ps)
         if k == closing_kind
             break
@@ -3127,7 +3130,6 @@ function parse_brackets(after_parse::Function,
             end
             t = peek_token(ps, skip_newlines=true)
             k = kind(t)
-            bump_trivia(ps)
             if k == K","
                 had_commas = true
                 bump(ps, TRIVIA_FLAG)
@@ -3156,7 +3158,7 @@ function parse_brackets(after_parse::Function,
         end
     end
     release_positions(ps.stream, params_positions)
-    bump_closing_token(ps, closing_kind)
+    bump_closing_token(ps, closing_kind, " or `,`")
     return opts
 end
 

test/diagnostics.jl

@@ -64,14 +64,17 @@ end
         Diagnostic(16, 16, :error, "missing condition in `elseif`")
 
     @test diagnostic("f(x::V) where {V) = x", allow_multiple=true) == [
-        Diagnostic(17, 16, :error, "Expected `}`")
+        Diagnostic(17, 16, :error, "Expected `}` or `,`")
         Diagnostic(17, 21, :error, "extra tokens after end of expression")
     ]
     @test diagnostic("[1)", allow_multiple=true) == [
-        Diagnostic(3, 2, :error, "Expected `]`")
+        Diagnostic(3, 2, :error, "Expected `]` or `,`")
         Diagnostic(3, 3, :error, "extra tokens after end of expression")
     ]
-
+    @test diagnostic("f(x, y #=hi=#\ng(z)") == Diagnostic(7, 6, :error, "Expected `)` or `,`")
+    @test diagnostic("(x, y \nz") == Diagnostic(6, 5, :error, "Expected `)` or `,`")
+    @test diagnostic("function f(x, y \nz end") == Diagnostic(16, 15, :error, "Expected `)` or `,`")
+
     @test diagnostic("sin. (1)") ==
         Diagnostic(5, 5, :error, "whitespace is not allowed here")
     @test diagnostic("x [i]") ==

test/hooks.jl

@@ -52,7 +52,7 @@ end
         @test err.source.first_line == 1
         @test err.diagnostics[1].first_byte == 6
         @test err.diagnostics[1].last_byte == 5
-        @test err.diagnostics[1].message == "Expected `}`"
+        @test err.diagnostics[1].message == "Expected `}` or `,`"
     end
 
     @testset "toplevel errors" begin