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

prefer-lookaround: Report cases that replacement repeating pattern #492

Open
fisker opened this issue Nov 23, 2022 · 2 comments
Open

prefer-lookaround: Report cases that replacement repeating pattern #492

fisker opened this issue Nov 23, 2022 · 2 comments

Comments

@fisker
Copy link

fisker commented Nov 23, 2022

Should we prefer

'a'.replace(/(?<=a)/, '-')

over

'a'.replace(/a/, 'a-')

?

I'm not sure about this. Since it may make code unreadable.

'Hi'.replace(/Hi/, 'Hello') // Good
'Hi'.replace(/(?<=H)i/, 'ello') // Really BAD
@ota-meshi
Copy link
Owner

Thank you for posting the issue.
Hmm🤔. I'm not sure the prefer-lookaround rule should report that far. As you wrote Hi->Hello is easier to read the first one.

@RunDevelopment
Copy link
Collaborator

My thoughts:

  • In the first example, I prefer 'a'.replace(/a/, 'a-') because it's simpler.
  • The only advantage of this transformation seems to be that it reduced redundancy.
  • This makes regexes more complex. Using a lookbehind to reduce some redundancy is a clever trick, but it's also not trivial to understand.
  • The prefix needs to be at least 6 characters long to produce shorter source code.
  • The resulting regexes might be harder to change. Whether you can do this trick depends on the pattern, so you might need to undo the trick to change the regex.
  • In the first example, I prefer 'a'.replace(/a/, 'a-') because it's simpler.

The downsides outweigh the upsides, IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants