fix(29648): Error message related to JSDoc for non-JSDoc syntax error#50793
Conversation
|
I think that as long as you don't have a complex check, it's generally better to put the error in the parser itself. The caveat is that the parser error should gracefully recover. |
a-tarasyuk
force-pushed
the
fix/29648
branch
from
f7ce43f to
375f7a0
Compare
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
I've only looked at the tests so far.
a-tarasyuk
requested review from
DanielRosenwasser and
sandersn
and removed request for
DanielRosenwasser
| } | ||
|
|
||
| function isInvalidConditionalType() { | ||
| if (token() === SyntaxKind.QuestionToken && !(contextFlags & NodeFlags.JavaScriptFile)) { |
There was a problem hiding this comment.
This is going to incur a lot of speculative parsing, isn't it? Specifically, every conditional type is going to speculatively parseType. Then it'll fail and proceed with normal parsing, which will parseType in the exact same location.
It might be possible to salvage either by (1) stopping the speculation before parseType or (2) making the speculative parse succeed when the type is conditional, and return some parsed portion of the conditional type node. (1) is likely to be lossy, but maybe not as much as dropping the speculation entirely. (2) would require passing the conditional part of the type down into parseType.
If those are too complex, I don't think it's worthwhile -- I reverted this change and it only breaks when the token following ? could be the start of a type: { -- but , and ) are fine. Improving some of the errors is good enough in my opinion.
(Reverting this will require splitting the nullable type test into two parts because syntax errors block semantic errors.)
There was a problem hiding this comment.
? could be the start of a type: {
Yes, this is only related to {, as it can be the start of a block or the start of a type. I've removed this speculative parsing, and updated test.
a-tarasyuk
force-pushed
the
fix/29648
branch
from
c6b1baf to
207fd7e
Compare
a-tarasyuk
force-pushed
the
fix/29648
branch
from
207fd7e to
d06486e
Compare
4 participants
Fixes #29648
@DanielRosenwasser @RyanCavanaugh At what stage is it better to handle such errors? At the
parserorcheckerstage?