JuliaSyntax.jl icon indicating copy to clipboard operation
JuliaSyntax.jl copied to clipboard

Diagnostics for string->value conversion

Open pfitzseb opened this issue 3 years ago • 2 comments

With #77, #80, and #81, the only remaining parsing errors are escape sequence errors like https://github.com/JuliaLang/JuliaSyntax.jl/issues/67.

Imho unescape_julia_string should never throw an exception. Instead, we should already check escape sequence validity when parsing (so that we can emit the correct diagnostics) and then emit ErrorVals during SyntaxNode construction.

pfitzseb avatar Aug 30 '22 11:08 pfitzseb

https://github.com/JuliaLang/JuliaSyntax.jl/pull/83 handles part of this and makes unescape_julia_string non-throwing. Diagnostics still aren't emitted though.

pfitzseb avatar Aug 30 '22 12:08 pfitzseb

we should already check escape sequence validity when parsing

This leads to processing all escapes twice which seems unfortunate. Also the same problem arises for numeric parsing (to detect overflow, for example).

I'd potentially favor doing this as a postprocessing pass over the token stream instead, maybe with a Diagnostics type separated off from ParseStream to collect any errors. This could be run during or prior to tree building:

  • When producing GreenNode to get correct diagnostics (discarding any values)
  • When producing SyntaxNode to get both diagnostics and parsed values

c42f avatar Aug 31 '22 00:08 c42f