Suggested changes to #2026 (Implement RegExp literal syntax checking)#2107
Suggested changes to #2026 (Implement RegExp literal syntax checking)#2107graphemecluster wants to merge 20 commits intomicrosoft:jabaile/regexp-errorsfrom
Conversation
Just for consistency Partially ports microsoft/TypeScript@c44a057
Fixes: no error on `/((?<foo>))(?<foo>)/` Ports microsoft/TypeScript@c435526#diff-65d73a953f046cf34092e70dd4a376ad5c885da833e750f88afeca31b6d63926
There was a problem hiding this comment.
This file is from microsoft/TypeScript#60249; I should've extract the changes out as a separate Strada PR
| } else { | ||
| v.error(diagnostics.Unexpected_0_Did_you_mean_to_escape_it_with_backslash, v.pos, 1, string(ch)) |
There was a problem hiding this comment.
This is removed in order to partially get rid of microsoft/TypeScript#62707 (not a full fix). I won't port microsoft/TypeScript#62716 until it gets a minimal review
| atomStart := v.pos | ||
| atom := v.scanClassAtom() | ||
| if v.charAtOffset(0) == '-' && v.charAtOffset(1) != ']' { | ||
| if v.charAtOffset(0) == '-' { |
There was a problem hiding this comment.
Redundant check
| } | ||
| if v.charAtOffset(0) == '}' { | ||
| v.pos++ | ||
| } else if hasDigits { |
There was a problem hiding this comment.
Redundant check
| if v.anyUnicodeModeOrNonAnnexB { | ||
| v.error(diagnostics.X_c_must_be_followed_by_an_ASCII_letter, v.pos-2, 2) | ||
| } else if atomEscape { | ||
| } else { |
There was a problem hiding this comment.
This is weird, but in Annex B, \c are really two characters regardless of whether it is an atom escape. I believe it is due to the ClassAtomNoDash production.
Also some changes from microsoft/TypeScript#60249; didn't file a separate PR due to insignificance.
| if ch != 0 && (scanner.IsIdentifierStart(ch) || ch == '_' || ch == '$') { | ||
| if ch != 0 && scanner.IsIdentifierStart(ch) { | ||
| v.pos++ | ||
| for v.pos < v.end { | ||
| ch = v.charAtOffset(0) | ||
| if scanner.IsIdentifierPart(ch) || ch == '_' || ch == '$' { | ||
| if scanner.IsIdentifierPart(ch) { |
There was a problem hiding this comment.
Again, some redundant checks
|
|
||
| propertyNameOrValueStart := v.pos | ||
| v.scanIdentifier(v.charAtOffset(0)) | ||
| v.scanWordCharacters(v.charAtOffset(0)) |
There was a problem hiding this comment.
I'm not sure if your original intention in relaxing the checks was to suppress the "Expected '}'" error when the } appears after a several exotic identifier characters. If so, you might want to revert that commit.
|
Closing since #3061 is merged. |
The merge target of this PR is
jabaile/regexp-errors, notmain. It is intended as a supplement of #2026.Feel free to fast-forward/cherry-pick/rebase some of the commits here.