You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Provide Basic Syntax Check for Regular Expression Literals #54744
Edit: Since someone is complaining about the failure to follow issue templates, I copied the content to #3432 (comment). Please continue the discussion in that issue.
Original content
Previously worked on #51837, I found that TypeScript gives almost no syntax errors for regular expressions. I would like to file a PR about it, thus I am opening this issue for easier tracking.
But before I work on the issue, I would like to hear some opinions from the Team. I am not sure, but I suppose the job of providing syntax check should not be shirked to linters right?
Something I would like to do are:
Check for duplicated or unknown flags
Check for unbalanced parentheses, which is the most common mistake people make
Check for invalid escapes
But this should be done only for RegExps with u or v flag, i.e. in UnicodeMode, that means if we encounter a u or v flag we will need to rescan the whole RegExp again (!!) (i.e. redoing what is done in the current reScanSlashToken method)
And to check for invalid DecimalEscapes and k<GroupName>s we will also need to count the number of capture groups and record the names of all named capture groups along the way.
Am I doing too much or too less? I know doing too much may cause serious performace regressions (well, luckily regular expression literals are not that common compared with string literals). It should be better than doing nothing after all though.
It does look like a duplicate, but this issue does cover something more than #3432.
I just want some feedback from the members before I work on a PR. It’s better than working in vain.
And of course I am not forgetting the issue templates, but none of the templates seem appropriate in this case. I don’t think this kind of things should be made mandatory as long as the cause and effect is clearly conveyed. Team members sometimes don’t use them either.
I don’t think [the issue templates] should be made mandatory as long as the cause and effect is clearly conveyed.
Nonetheless, they are (note the canned response followed by issue closure). Maintainers will almost invariably ignore issues that don't follow the template.
I don’t mean to start a dispute, but this is called Quod licet Iovi, non licet bovi.
Anyway, since comments to issues do not need to follow any templates, I’ve just moved the whole thing to #3432.
graphemecluster (@graphemecluster) the reason we ask people to fill out the template is to ensure that people have done their due diligence working through necessary background research. Generally if someone is not willing to do this minimum amount of legwork, they are filing a very low-quality issue, or a duplicate of an existing issue, as occurred here.
Reacted by Bruce Pascoe, Martin Johns and ExE Boss
Ryan Cavanaugh (@RyanCavanaugh) I apologize for my faults. I did search for existing things beforehand. To be specific, I firstly searched if there are any PRs related to RegExp. Since no significant PRs exist, I looked over the source code and started to work on it while encountering something to inquiry. So I quickly wrote the issue without other checks. This is the reason why I made the duplication. Please pardon me and comment on #3432.
Activity
MartinJohns commentedon Jun 22, 2023
You forgot to fill out the issue template. Duplicate of #3432.
graphemecluster commentedon Jun 22, 2023
It does look like a duplicate, but this issue does cover something more than #3432.
I just want some feedback from the members before I work on a PR. It’s better than working in vain.
And of course I am not forgetting the issue templates, but none of the templates seem appropriate in this case. I don’t think this kind of things should be made mandatory as long as the cause and effect is clearly conveyed. Team members sometimes don’t use them either.
fatcerberus commentedon Jun 22, 2023
Nonetheless, they are (note the canned response followed by issue closure). Maintainers will almost invariably ignore issues that don't follow the template.
graphemecluster commentedon Jun 23, 2023
I don’t mean to start a dispute, but this is called Quod licet Iovi, non licet bovi.
Anyway, since comments to issues do not need to follow any templates, I’ve just moved the whole thing to #3432.
RyanCavanaugh commentedon Jun 23, 2023
graphemecluster (@graphemecluster) the reason we ask people to fill out the template is to ensure that people have done their due diligence working through necessary background research. Generally if someone is not willing to do this minimum amount of legwork, they are filing a very low-quality issue, or a duplicate of an existing issue, as occurred here.
graphemecluster commentedon Jun 23, 2023
Ryan Cavanaugh (@RyanCavanaugh) I apologize for my faults. I did search for existing things beforehand. To be specific, I firstly searched if there are any PRs related to RegExp. Since no significant PRs exist, I looked over the source code and started to work on it while encountering something to inquiry. So I quickly wrote the issue without other checks. This is the reason why I made the duplication. Please pardon me and comment on #3432.
RyanCavanaugh commentedon Jun 23, 2023
No worries, I file dupes sometimes too 😅