Add new Performance/RegexpMatch cop
#3824
Conversation
| PATTERN | ||
|
|
||
| def on_if(node) | ||
| return if target_ruby_version < 2.4 |
There was a problem hiding this comment.
Probably we simply need to add to the Cop class some method like minimum_target_ruby_version or something. I hate having to write such checks all over the place.
There was a problem hiding this comment.
Sounds good. I think it should be declarative as a class method.
For example.
module RuboCop
...
class SomeCop
minimum_target_ruby_version 2.4
end
...
endI'll implement it in other pull-request.
|
Check the commit message. :-) |
pocke
changed the title
Performance/RegexpMerge copPerformance/RegexpMatch cop
🙀 I fixed it. |
| @@ -1408,6 +1408,10 @@ Performance/RedundantSortBy: | |||
| Description: 'Use `sort` instead of `sort_by { |x| x }`.' | |||
| Enabled: true | |||
|
|
|||
| Performance/RegexpMatch: | |||
| Description: 'Use `match?` instead of `Regexp#match`, `String#match` or `=~` when `MatchData` is not used.' | |||
There was a problem hiding this comment.
We might be better off not explicitly listing Regexp#match and String#match since match? is left more generic. This does not have reference to Symbol#match. Alternatively, we might want to be more explicit about saying use Regexp#match?, String#match?, or Symbol#match?.
There was a problem hiding this comment.
It's reasonable. 👍
I'll add to the description, and I'll update the spec.
| module RuboCop | ||
| module Cop | ||
| module Performance | ||
| # In Ruby 2.4, `String#match?` and `Regexp#match?` have been added. |
There was a problem hiding this comment.
We should reference Symbol#match? here.
| '/re/.match(foo, 1)', | ||
| '/re/.match?(foo, 1)'], | ||
| ['matching by =~`', '/re/ =~ foo', '/re/.match?(foo)'], | ||
| ['matching by =~`', 'foo =~ /re/', 'foo.match?(/re/)'] |
There was a problem hiding this comment.
Should this include test cases for regex matching against symbols?
In Ruby 2.4, `String#match?` and `Regexp#match?` have been added. https://www.ruby-lang.org/en/news/2016/12/25/ruby-2-4-0-released/ https://bugs.ruby-lang.org/issues/8110 The methods are faster than `match`. Because the methods avoid creating a `MatchData` object or saving backref. So, when `MatchData` is not used, use `match?` instead of `match`. This cop suggests using `match?` method.
|
@rrosenblum Thanks for your suggestion! |
In Ruby 2.4,
String#match?andRegexp#match?have been added.https://www.ruby-lang.org/en/news/2016/12/25/ruby-2-4-0-released/
https://bugs.ruby-lang.org/issues/8110
The methods are faster than
match.Because the methods avoid creating a
MatchDataobject or saving backref.So, when
MatchDatais not used, usematch?instead ofmatch.This cop suggests using
match?method.Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).and description in grammatically correct, complete sentences.
rake generate_cops_documentation(required only when you've added a new cop or changed the configuration/documentation of an existing cop).