Skip to content
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

Jump to bottom

Update: prefer-regex-literal detects regex literals passed to RegExp (fixes #12840) #12842

Merged
merged 1 commit into from Jun 25, 2020
Merged

Update: prefer-regex-literal detects regex literals passed to RegExp (fixes #12840) #12842

merged 1 commit into from Jun 25, 2020

Conversation

lo1tuma
Copy link
Member

@lo1tuma lo1tuma commented Jan 29, 2020 •
edited

Prerequisites checklist

  • I have read the contributing guidelines.
  • The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to item)

  • Documentation update
  • Bug fix (template)
  • New rule (template)
  • Changes an existing rule (template)
  • Add autofixing to a rule
  • Add a CLI option
  • Add something to the core
  • Other, please explain:

What changes did you make? (Give an overview)

I’ve changed the rule prefer-regex-literal so that it detects regex literals that are passed to the RegExp constructor.

Is there anything you'd like reviewers to focus on?

  • I decided to implement an autofix yet, since the whole rule is not autofixable yet. I think it makes more sense to address this separately
  • I have implemented the rule change as the new default behavior. Once a decision has been made in prefer-regex-literals: should report regex literals passed to RegExp constructor #12840 I’m happy to change the if necessary and move the new behavior behind an option The new behavior is now implemented behind an option.

@jsf-clabot
Copy link

jsf-clabot commented Jan 29, 2020 •
edited

CLA assistant check
All committers have signed the CLA.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jan 29, 2020
@lo1tuma lo1tuma changed the title WIP: Update: prefer-regex-literal detects regex literals passed to RegExp (fixes #12840) Update: prefer-regex-literal detects regex literals passed to RegExp (fixes #12840) Jan 30, 2020
@lo1tuma lo1tuma marked this pull request as ready for review January 30, 2020 13:17
@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Feb 9, 2020
@mdjermanovic mdjermanovic linked an issue Feb 9, 2020 that may be closed by this pull request
prefer-regex-literals: should report regex literals passed to RegExp constructor #12840
Closed
@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Jun 13, 2020
mdjermanovic
mdjermanovic requested changes Jun 20, 2020
View reviewed changes
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, I have just a couple of minor suggestions about the documentation and a few nice-to-have tests.

docs/rules/prefer-regex-literals.md Outdated Show resolved Hide resolved
tests/lib/rules/prefer-regex-literals.js Show resolved Hide resolved
tests/lib/rules/prefer-regex-literals.js Show resolved Hide resolved
docs/rules/prefer-regex-literals.md Outdated Show resolved Hide resolved
@lo1tuma
Update: prefer-regex-literal detect regex literals (fixes eslint#12840)
293d5ea 
The rule `prefer-regex-literal` now detects when regex literals are
unnecessarily passed to the `RegExp` constructor.
@lo1tuma
Copy link
Member Author

lo1tuma commented Jun 21, 2020

@mdjermanovic thanks for your feedback. I’ve addressed all you comments, PTAL.

mdjermanovic
mdjermanovic approved these changes Jun 21, 2020
View reviewed changes
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@kaicataldo kaicataldo merged commit d53d69a into eslint:master Jun 25, 2020
@kaicataldo
Copy link
Member

Thanks for contributing!

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Dec 23, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kaicataldo kaicataldo kaicataldo approved these changes

@mdjermanovic mdjermanovic mdjermanovic approved these changes

Assignees
No one assigned
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

prefer-regex-literals: should report regex literals passed to RegExp constructor
4 participants
@lo1tuma @jsf-clabot @kaicataldo @mdjermanovic