Skip to content

Commit 0916a99

Browse files
committed
Add AllowRegexpMatch option to Performance/RedundantEqualityComparisonBlock
Follow up #340. This PR adds `AllowRegexpMatch` option to `Performance/RedundantEqualityComparisonBlock`. ```ruby # `AllowRegexpMatch: true` (default) # good items.all? { |item| item =~ pattern } items.all? { |item| item.match?(pattern) } # `AllowRegexpMatch: false` # bad items.all? { |item| item =~ pattern } items.all? { |item| item.match?(pattern) } ``` `AllowRegexpMatch: false` by default because `regexp.match?('string')` often used in block changes to the opposite result: ```ruby [/pattern/].all? { |regexp| regexp.match?('pattern') } # => true [/pattern/].all? { |regexp| regexp =~ 'pattern' } # => true [/pattern/].all?('pattern') # => false ```
1 parent c155ba5 commit 0916a99

File tree

4 files changed

+69
-26
lines changed

4 files changed

+69
-26
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#347](https://github.com/rubocop/rubocop-performance/pull/347): Add `AllowRegexpMatch` option to `Performance/RedundantEqualityComparisonBlock`. ([@koic][])

config/default.yml

+1
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ Performance/RedundantEqualityComparisonBlock:
226226
Reference: 'https://github.com/rails/rails/pull/41363'
227227
Enabled: pending
228228
Safe: false
229+
AllowRegexpMatch: false
229230
VersionAdded: '1.10'
230231

231232
Performance/RedundantMatch:

lib/rubocop/cop/performance/redundant_equality_comparison_block.rb

+33-4
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,42 @@ module Performance
1010
# behavior is appropriately overridden in subclass. For example,
1111
# `Range#===` returns `true` when argument is within the range.
1212
#
13+
# This cop has `AllowRegexpMatch` option and it is false by default because
14+
# `regexp.match?('string')` often used in block changes to the opposite result:
15+
#
16+
# [source,ruby]
17+
# ----
18+
# [/pattern/].all? { |regexp| regexp.match?('pattern') } # => true
19+
# [/pattern/].all? { |regexp| regexp =~ 'pattern' } # => true
20+
# [/pattern/].all?('pattern') # => false
21+
# ----
22+
#
1323
# @safety
1424
# This cop is unsafe because `===` and `==` do not always behave the same.
1525
#
1626
# @example
1727
# # bad
1828
# items.all? { |item| pattern === item }
1929
# items.all? { |item| item == other }
20-
# items.all? { |item| item =~ pattern }
2130
# items.all? { |item| item.is_a?(Klass) }
2231
# items.all? { |item| item.kind_of?(Klass) }
23-
# items.all? { |item| item.match?(pattern) }
2432
#
2533
# # good
2634
# items.all?(pattern)
2735
# items.all?(Klass)
2836
#
37+
# @example AllowRegexpMatch: true (default)
38+
#
39+
# # good
40+
# items.all? { |item| item =~ pattern }
41+
# items.all? { |item| item.match?(pattern) }
42+
#
43+
# @example AllowRegexpMatch: false
44+
#
45+
# # bad
46+
# items.all? { |item| item =~ pattern }
47+
# items.all? { |item| item.match?(pattern) }
48+
#
2949
class RedundantEqualityComparisonBlock < Base
3050
extend AutoCorrector
3151
extend TargetRubyVersion
@@ -35,7 +55,8 @@ class RedundantEqualityComparisonBlock < Base
3555
MSG = 'Use `%<prefer>s` instead of block.'
3656

3757
TARGET_METHODS = %i[all? any? one? none?].freeze
38-
COMPARISON_METHODS = %i[== === =~ is_a? kind_of? match?].freeze
58+
COMPARISON_METHODS = %i[== === is_a? kind_of?].freeze
59+
REGEXP_METHODS = %i[=~ match?].freeze
3960
IS_A_METHODS = %i[is_a? kind_of?].freeze
4061

4162
def on_block(node)
@@ -63,7 +84,11 @@ def one_block_argument?(block_arguments)
6384
end
6485

6586
def use_equality_comparison_block?(block_body)
66-
block_body.send_type? && COMPARISON_METHODS.include?(block_body.method_name)
87+
return false unless block_body.send_type?
88+
89+
method_name = block_body.method_name
90+
91+
COMPARISON_METHODS.include?(method_name) || (!allow_regexp_match? && REGEXP_METHODS.include?(method_name))
6792
end
6893

6994
def same_block_argument_and_is_a_argument?(block_body, block_argument)
@@ -102,6 +127,10 @@ def use_block_argument_in_method_argument_of_operand?(block_argument, operand)
102127
def offense_range(node)
103128
node.send_node.loc.selector.join(node.source_range.end)
104129
end
130+
131+
def allow_regexp_match?
132+
cop_config.fetch('AllowRegexpMatch', true)
133+
end
105134
end
106135
end
107136
end

spec/rubocop/cop/performance/redundant_equality_comparison_block_spec.rb

+34-22
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,6 @@
2525
RUBY
2626
end
2727

28-
it "registers and corrects an offense when using `#{method_name}` with `=~` comparison block" do
29-
expect_offense(<<~RUBY, method_name: method_name)
30-
items.#{method_name} { |item| item =~ other }
31-
^{method_name}^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#{method_name}(other)` instead of block.
32-
RUBY
33-
34-
expect_correction(<<~RUBY)
35-
items.#{method_name}(other)
36-
RUBY
37-
end
38-
3928
it "registers and corrects an offense when using `#{method_name}` with `is_a?` comparison block" do
4029
expect_offense(<<~RUBY, method_name: method_name)
4130
items.#{method_name} { |item| item.is_a?(Klass) }
@@ -58,17 +47,6 @@
5847
RUBY
5948
end
6049

61-
it "registers and corrects an offense when using `#{method_name}` with `match?` comparison block" do
62-
expect_offense(<<~RUBY, method_name: method_name)
63-
items.#{method_name} { |item| item.match?(pattern) }
64-
^{method_name}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#{method_name}(pattern)` instead of block.
65-
RUBY
66-
67-
expect_correction(<<~RUBY)
68-
items.#{method_name}(pattern)
69-
RUBY
70-
end
71-
7250
it "does not register an offense when using `#{method_name}` with `===` comparison block and" \
7351
'block argument is not used as a receiver for `===`' do
7452
expect_no_offenses(<<~RUBY, method_name: method_name)
@@ -129,6 +107,40 @@
129107
items.do_something { |item| item == other }
130108
RUBY
131109
end
110+
111+
context 'when `AllowRegexpMatch: true`' do
112+
let(:cop_config) { { 'AllowRegexpMatch' => true } }
113+
114+
it 'does not register an offense when using target method` with `=~` comparison block' do
115+
expect_no_offenses(<<~RUBY)
116+
items.all? { |item| item =~ pattern }
117+
RUBY
118+
end
119+
120+
it 'does not register an offense when using target method with `match?` comparison block' do
121+
expect_no_offenses(<<~RUBY)
122+
items.all? { |item| item.match?(pattern) }
123+
RUBY
124+
end
125+
end
126+
127+
context 'when `AllowRegexpMatch: false`' do
128+
let(:cop_config) { { 'AllowRegexpMatch' => false } }
129+
130+
it 'registers an offense when using target method with `=~` comparison block' do
131+
expect_offense(<<~RUBY)
132+
items.all? { |item| item =~ pattern }
133+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `all?(pattern)` instead of block.
134+
RUBY
135+
end
136+
137+
it 'registers an offense when using target method with `match?` comparison block' do
138+
expect_offense(<<~RUBY)
139+
items.all? { |item| item.match?(pattern) }
140+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `all?(pattern)` instead of block.
141+
RUBY
142+
end
143+
end
132144
end
133145

134146
context 'when TargetRubyVersion <= 2.4', :ruby24 do

0 commit comments

Comments
 (0)