Repository navigation
Add invalid regular expression literal error #3432 - #4387
Avery Morin (Schmavery) wants to merge 1 commit into
Conversation
|
Hi Avery Morin (@Schmavery), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
|
Maybe the error message we report should just be the exception text. Chrome/node:
IE:
|
|
Ryan Cavanaugh (@RyanCavanaugh) the issue with that is localization. |
|
Given the choice between e.g. |
There was a problem hiding this comment.
catch on the next line
|
After thinking about it, since we only have a loc story for |
|
I had considered returning the exception text, but wasn't sure how that would fit into the existing error reporting system, especially since the format of the text can vary a decent amount. |
|
The problem with this it's that regexes are extensible to some extent. So I'm worried about us claiming the regex is invalid when some engine may be ok with it. From: Daniel Rosenwassermailto:[email protected] After thinking about it, since we only have a loc story for tsc.exe, whose messages should be localized as far as I understand anyway, I'm fine with this. — |
|
Should I just close this PR then? Sounds like you guys aren't sure what you want to do with this issue yet. |
|
It's not that i'm not sure. It's that I think the fix isn't correct. You're fix says "if the engine the TS compiler is running in doesn't like the Regex, then the Regex is bad". But that's not the case. You may end up running your regex in an environment that thinks the regex is fine. Now, if you wanted to go more the distance and actually check specific parts of the string against the regex grammar in the ES6 specification, then I would be more ok with that :) |
|
I think we're going to close this PR and defer to using the ECMAScript spec for verification. |
This adds an error "Invalid regular expression literal." to address issue #3432
Basically, at the end of
reScanSlashToken()in scanner.ts, I take the tokenValue and create aRegExpin a try-catch. If this throws an exception, it's not a valid regex. Is this something like you guys imagined?Note: This changes some of the results of the conformance tests that contained invalid regexes.