New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(eslint-plugin): [prefer-regexp-exec] respect flags when using RegExp
#3855
Conversation
Thanks for the PR, @rafaelss95! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! |
Nx Cloud ReportCI ran the following commands for commit 9fa7537. Click to see the status, the terminal output, and the build insights. Sent with |
Codecov Report
@@ Coverage Diff @@
## master #3855 +/- ##
=======================================
Coverage 93.50% 93.51%
=======================================
Files 151 151
Lines 8134 8140 +6
Branches 2580 2583 +3
=======================================
+ Hits 7606 7612 +6
Misses 167 167
Partials 361 361
Flags with carried forward coverage won't be shown. Click here to find out more.
|
66b8fc2
to
7200b22
Compare
RegExp
RegExp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like there currently aren't any existing tests for new RegExp
- but I just checked and the rule currently supports its.
It just doesn't do the "global" check on them.
Your change causes the new RegExp
constructor to be completely ignored - which is not correct.
Instead we should inspect the new
constructor to extract the second argument to determine if it has a 'g'
in the string
Oh, wait, I see... there's some inconsistent behavior. I'll look ASAP. |
85b7de5
to
1453074
Compare
1453074
to
9fa7537
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding all of those tests!
LGTM!
Fixes #3477.