chore(eslint): Add eslint-plugin-regexp rule#18053
Conversation
| // Note: CodeQL complains that this regex potentially has n^2 runtime. This likely won't affect realistic files. | ||
| new RegExp('^(?:\\s*|/\\*(?:.|\\r|\\n)*?\\*/|//.*[\\n\\r])*(?:"[^"]*";?|\'[^\']*\';?)?'); | ||
| // eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor,prefer-regex-literals | ||
| new RegExp('^(?:\\s|/\\*(?:.|[\\r\\n])*?\\*/|//.*[\\n\\r])*(?:"[^"]*";?|\'[^\']*\';?)?'); |
There was a problem hiding this comment.
Bug: Regex Pattern Error: Incorrectly Handles Whitespace
The regex pattern incorrectly removed the * quantifier from \s*. The original pattern ^\s* (zero or more whitespace) was changed to ^\s (exactly one whitespace), which fundamentally changes the regex semantics. The pattern should be ^(?:\s|/\*(?:.|[\r\n])*?\*\/|//.*[\n\r])* not ^(?:\\s|/\*(?:.|[\r\n])*?\*\/|//.*[\n\r])* to maintain backward compatibility. This breaks the directive-matching logic that needs to handle cases with no leading whitespace.
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
|
Closes in favor of separating this into two PRs, which apply those rules once only for |
Enabling eslint rules for RegEx in `dev-packages`. This is a part of this (now closed) PR: #18053 The first commit contains all changes that were automatically made with `--fix`, the second commit contains the manual changes.
The problem with polynomial regular expressions was mentioned in a couple of comments already:
dirnameandbasenameshould handle Windows paths #8737 (comment)This ESLint package includes a rule which checks for polynomial expressions. Beside that it also adds some performance improvements like using Non-Capturing groups in case the groups are not used. Or also using
/dinstead of0-9Review Notes
--fixWhen making manual changes, I tested the regex on https://regex101.com/ and although I don't have scientific benchmarks, I saw a huge improvement (steps and time reduced by half) in the performance note.