refactor: handle parsing errors with BadExpressions over panics #2804
Conversation
There was a problem hiding this comment.
Great! Just wondering if those parse functions will need to be exposed at some point? If not, could they not be returning expression directly?
Also wondering what the cases were that tripped the parse failures, which then became panics? Overflow probably one ... but are there others? Would be nice to categorize the errors, otherwise our language definition is just borrowing from rust's definition for these parse functions and we have bound ourselves to a specific implementation.
bthesorceror
force-pushed
the
bad-expression-handling
branch
from
8059721 to
d9d57c4
Compare
|
@adrian-thurston I've added a test around this to illuminate where we were seeing the panic in the LSP. |
bthesorceror
force-pushed
the
bad-expression-handling
branch
from
d9d57c4 to
337b823
Compare
|
@bthesorceror Okay thanks. At first I thought this was a regression in the rust parser because the scanner allows for a time literal with no timezone information on the end. But it looks like the go parser does the same ... allows the missing timezone in the scanner and fails during the RFC3339 parse. Perhaps it is to get more detailed error information, other than "scanner failed". |
|
Yes, the SPEC states that we support those shorter formats https://github.com/influxdata/flux/blob/master/docs/SPEC.md#date-and-time-literals but we have not implemented them yet. See #152 although it seems that the scanner supports those formats, just not the parser. So for now we can just improve the error message to say something like shorthand time literals are not implemented yet. |
|
Ah okay, thank you for the background @nathanielc |
No description provided.