Add AllowRegexpMatch option to Performance/RedundantEqualityComparisonBlock

changelog/new_add_allow_regexp_match_option_to_performance_redundant_equality_comparison_block.md

@@ -0,0 +1 @@
+* [#347](https://github.com/rubocop/rubocop-performance/pull/347): Add `AllowRegexpMatch` option to `Performance/RedundantEqualityComparisonBlock`. ([@koic][])

config/default.yml

@@ -226,6 +226,7 @@ Performance/RedundantEqualityComparisonBlock:
   Reference: 'https://github.com/rails/rails/pull/41363'
   Enabled: pending
   Safe: false
+  AllowRegexpMatch: false
   VersionAdded: '1.10'
 
 Performance/RedundantMatch:

lib/rubocop/cop/performance/redundant_equality_comparison_block.rb

@@ -10,22 +10,42 @@ module Performance
       # behavior is appropriately overridden in subclass. For example,
       # `Range#===` returns `true` when argument is within the range.
       #
+      # This cop has `AllowRegexpMatch` option and it is false by default because
+      # `regexp.match?('string')` often used in block changes to the opposite result:
+      #
+      # [source,ruby]
+      # ----
+      # [/pattern/].all? { |regexp| regexp.match?('pattern') } # => true
+      # [/pattern/].all? { |regexp| regexp =~ 'pattern' }      # => true
+      # [/pattern/].all?('pattern')                            # => false
+      # ----
+      #
       # @safety
       #   This cop is unsafe because `===` and `==` do not always behave the same.
       #
       # @example
       #   # bad
       #   items.all? { |item| pattern === item }
       #   items.all? { |item| item == other }
-      #   items.all? { |item| item =~ pattern }
       #   items.all? { |item| item.is_a?(Klass) }
       #   items.all? { |item| item.kind_of?(Klass) }
-      #   items.all? { |item| item.match?(pattern) }
       #
       #   # good
       #   items.all?(pattern)
       #   items.all?(Klass)
       #
+      # @example AllowRegexpMatch: true (default)
+      #
+      #   # good
+      #   items.all? { |item| item =~ pattern }
+      #   items.all? { |item| item.match?(pattern) }
+      #
+      # @example AllowRegexpMatch: false
+      #
+      #   # bad
+      #   items.all? { |item| item =~ pattern }
+      #   items.all? { |item| item.match?(pattern) }
+      #
       class RedundantEqualityComparisonBlock < Base
         extend AutoCorrector
         extend TargetRubyVersion
@@ -35,7 +55,8 @@ class RedundantEqualityComparisonBlock < Base
         MSG = 'Use `%<prefer>s` instead of block.'
 
         TARGET_METHODS = %i[all? any? one? none?].freeze
-        COMPARISON_METHODS = %i[== === =~ is_a? kind_of? match?].freeze
+        COMPARISON_METHODS = %i[== === is_a? kind_of?].freeze
+        REGEXP_METHODS = %i[=~ match?].freeze
         IS_A_METHODS = %i[is_a? kind_of?].freeze
 
         def on_block(node)
@@ -63,7 +84,11 @@ def one_block_argument?(block_arguments)
         end
 
         def use_equality_comparison_block?(block_body)
-          block_body.send_type? && COMPARISON_METHODS.include?(block_body.method_name)
+          return false unless block_body.send_type?
+
+          method_name = block_body.method_name
+
+          COMPARISON_METHODS.include?(method_name) || (!allow_regexp_match? && REGEXP_METHODS.include?(method_name))
         end
 
         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)
         def offense_range(node)
           node.send_node.loc.selector.join(node.source_range.end)
         end
+
+        def allow_regexp_match?
+          cop_config.fetch('AllowRegexpMatch', true)
+        end
       end
     end
   end

spec/rubocop/cop/performance/redundant_equality_comparison_block_spec.rb

@@ -25,17 +25,6 @@
         RUBY
       end
 
-      it "registers and corrects an offense when using `#{method_name}` with `=~` comparison block" do
-        expect_offense(<<~RUBY, method_name: method_name)
-          items.#{method_name} { |item| item =~ other }
-                ^{method_name}^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#{method_name}(other)` instead of block.
-        RUBY
-
-        expect_correction(<<~RUBY)
-          items.#{method_name}(other)
-        RUBY
-      end
-
       it "registers and corrects an offense when using `#{method_name}` with `is_a?` comparison block" do
         expect_offense(<<~RUBY, method_name: method_name)
           items.#{method_name} { |item| item.is_a?(Klass) }
@@ -58,17 +47,6 @@
         RUBY
       end
 
-      it "registers and corrects an offense when using `#{method_name}` with `match?` comparison block" do
-        expect_offense(<<~RUBY, method_name: method_name)
-          items.#{method_name} { |item| item.match?(pattern) }
-                ^{method_name}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `#{method_name}(pattern)` instead of block.
-        RUBY
-
-        expect_correction(<<~RUBY)
-          items.#{method_name}(pattern)
-        RUBY
-      end
-
       it "does not register an offense when using `#{method_name}` with `===` comparison block and" \
          'block argument is not used as a receiver for `===`' do
         expect_no_offenses(<<~RUBY, method_name: method_name)
@@ -129,6 +107,40 @@
         items.do_something { |item| item == other }
       RUBY
     end
+
+    context 'when `AllowRegexpMatch: true`' do
+      let(:cop_config) { { 'AllowRegexpMatch' => true } }
+
+      it 'does not register an offense when using target method` with `=~` comparison block' do
+        expect_no_offenses(<<~RUBY)
+          items.all? { |item| item =~ pattern }
+        RUBY
+      end
+
+      it 'does not register an offense when using target method with `match?` comparison block' do
+        expect_no_offenses(<<~RUBY)
+          items.all? { |item| item.match?(pattern) }
+        RUBY
+      end
+    end
+
+    context 'when `AllowRegexpMatch: false`' do
+      let(:cop_config) { { 'AllowRegexpMatch' => false } }
+
+      it 'registers an offense when using target method with `=~` comparison block' do
+        expect_offense(<<~RUBY)
+          items.all? { |item| item =~ pattern }
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `all?(pattern)` instead of block.
+        RUBY
+      end
+
+      it 'registers an offense when using target method with `match?` comparison block' do
+        expect_offense(<<~RUBY)
+          items.all? { |item| item.match?(pattern) }
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `all?(pattern)` instead of block.
+        RUBY
+      end
+    end
   end
 
   context 'when TargetRubyVersion <= 2.4', :ruby24 do