eslint-plugin-regexp icon indicating copy to clipboard operation
eslint-plugin-regexp copied to clipboard

`prefer-lookaround`: Report cases that replacement repeating pattern

Open fisker opened this issue 3 years ago • 2 comments

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

fisker avatar Nov 23 '22 11:11 fisker

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.

ota-meshi avatar Nov 24 '22 07:11 ota-meshi

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.

RunDevelopment avatar Nov 24 '22 16:11 RunDevelopment