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

New cop: Use String#include? instead of String#match? with /literal-only-pattern/ #81

Closed
pocke opened this issue Oct 18, 2019 · 3 comments · Fixed by #117
Closed

New cop: Use String#include? instead of String#match? with /literal-only-pattern/ #81

pocke opened this issue Oct 18, 2019 · 3 comments · Fixed by #117

Comments

@pocke
Copy link
Contributor

pocke commented Oct 18, 2019

Is your feature request related to a problem? Please describe.

String#include? is faster than String#match? with a simple regexp.

https://gist.github.com/pocke/eeffeaa58ddcc35f862f6e1aca593b21
I wrote a few benchmarks, and I got the result.

The difference is small in simple cases (str1 and str2).
But there are clear differences if the string is long.

Describe the solution you'd like

Add a cop to detect String#match? with simple regexp.
And I guess the cop also can detect String#match and String#=~ if it is used as a condition.

Describe alternatives you've considered

nothing

Additional context

We already have Performance/StartWith and EndWIth. We can refer the cops to help to implement it.

@Drenmi
Copy link
Contributor

Drenmi commented Oct 18, 2019

And I guess the cop also can detect String#match and String#=~ if it is used as a condition.

It's very hard to tell if people are using the global variables stored by String#match, but perhaps can include a configuration option for this?

@pocke
Copy link
Contributor Author

pocke commented Oct 18, 2019

It's very hard to tell if people are using the global variables stored by String#match, but perhaps can include a configuration option for this?

Good point! I forgot the global variables.
Performance/RegexpMatch can be aware of the global variables, so the include cop will be able to support them also by using Performance/RegexpMatch code.

But I guess we can just omit supporting match and =~ from include cop.
RegexpMatch cop corrects match to match if it is possible, so the include cop can detect match + simple regexp usage after running RegexpMatch cop even if the include cop does not support match and =~ directly.

@bbatsov
Copy link
Contributor

bbatsov commented Oct 18, 2019

Agreed. Generally the use of the globals has been on the decline, so I guess we can also advise people not to do it. I guess the only thing the globals have for them is that they are more compact than the alternative.

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

Successfully merging a pull request may close this issue.

3 participants