New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve Recovery of Unterminated Regular Expressions #58289
base: main
Are you sure you want to change the base?
Conversation
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
@@ -2389,7 +2390,8 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean | |||
function reScanSlashToken(): SyntaxKind { | |||
if (token === SyntaxKind.SlashToken || token === SyntaxKind.SlashEqualsToken) { | |||
// Quickly get to the end of regex such that we know the flags | |||
let p = tokenStart + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is eliminated for readability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is bad. Are there any solutions that does not affect pressing enter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this now become division?
@@ -49,5 +49,5 @@ file.tsx(11,20): error TS1161: Unterminated regular expression literal. | |||
!!! error TS2304: Cannot find name 'data'. | |||
~ | |||
!!! error TS1005: ';' expected. | |||
~~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include angle brackets (in addition to parens and braces) to the logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should have named it regExpScanningRecovery.ts
(which is analogous to jsonParserRecovery.ts
, I forgot the fact that the recovery part will still remain in the scanner even after the regex worker is moved out)
"\t\v\r\n", | ||
"\u3000\u2028", | ||
]; | ||
it("stops parsing unterminated regexes at correct position", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn’t name each test case; it’s unrealistic to give each of the test cases a meaningful name. Perhaps they should be named by their indices?
This is now recovered.
The change is no longer necessary since it’s moved to `checker.ts` in microsoft#58295. The partially reverts microsoft@1a5228d.
Comments marked “Outdated” are not actually outdated, they’re just due to the merge |
Please inform me if you would like me to move the last commit to #58320 or a new PR. |
Per #55600 (comment), to not let
scanRegularExpressionWorker
affect the end position of the regex, I separated the logic for unterminated regexes out from the well-terminated ones. This should make movingscanRegularExpressionWorker
away in the future much easier. Note that unterminated regexes are no longer parsed.