Merge pull request #371 from ymap/fix_an_incorrect_autocorrect_for_pe…

…rformance_redundant_match

[Fix #370] Fix an incorrect autocorrect for `Performance/RedundantMatch`

changelog/fix_an_incorrect_autocorrect_for_performance_redundant_match.md

@@ -0,0 +1 @@
+* [#370](https://github.com/rubocop/rubocop-performance/issues/370): Fix an incorrect autocorrect for `Performance/RedundantMatch` when expressions with lower precedence than `=~` are used as an argument. ([@ymap][])

lib/rubocop/cop/performance/redundant_match.rb

@@ -23,6 +23,8 @@ class RedundantMatch < Base
         MSG = 'Use `=~` in places where the `MatchData` returned by `#match` will not be used.'
         RESTRICT_ON_SEND = %i[match].freeze
 
+        HIGHER_PRECEDENCE_OPERATOR_METHODS = %i[| ^ & + - * / % ** > >= < <= << >>].freeze
+
         # 'match' is a fairly generic name, so we don't flag it unless we see
         # a string or regexp literal on one side or the other
         def_node_matcher :match_call?, <<~PATTERN
@@ -47,7 +49,7 @@ def on_send(node)
         private
 
         def autocorrect(corrector, node)
-          new_source = "#{node.receiver.source} =~ #{node.first_argument.source}"
+          new_source = "#{node.receiver.source} =~ #{replacement(node)}"
 
           corrector.replace(node, new_source)
         end
@@ -57,6 +59,33 @@ def autocorrectable?(node)
           # register an offense in that case
           node.receiver.regexp_type? || node.first_argument.regexp_type?
         end
+
+        def replacement(node)
+          arg = node.first_argument
+
+          if requires_parentheses?(arg)
+            "(#{arg.source})"
+          else
+            arg.source
+          end
+        end
+
+        def requires_parentheses?(arg)
+          return true if arg.if_type? && arg.ternary?
+          return true if arg.and_type? || arg.or_type? || arg.range_type?
+
+          call_like?(arg) && requires_parentheses_for_call_like?(arg)
+        end
+
+        def requires_parentheses_for_call_like?(arg)
+          return false if arg.parenthesized? || !arg.arguments?
+
+          !HIGHER_PRECEDENCE_OPERATOR_METHODS.include?(arg.method_name)
+        end
+
+        def call_like?(arg)
+          arg.call_type? || arg.yield_type? || arg.super_type?
+        end
       end
     end
   end

spec/rubocop/cop/performance/redundant_match_spec.rb

@@ -120,4 +120,50 @@ def method(str)
       something if /regex/ =~ str
     RUBY
   end
+
+  shared_examples 'require parentheses' do |arg|
+    it "registers an offense and corrects when argument is `#{arg}`" do
+      expect_offense(<<~RUBY, arg: arg)
+        something if /regex/.match(%{arg})
+                     ^^^^^^^^^^^^^^^{arg}^ Use `=~` in places where the `MatchData` returned by `#match` will not be used.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        something if /regex/ =~ (#{arg})
+      RUBY
+    end
+  end
+
+  it_behaves_like 'require parentheses', 'a ? b : c'
+  it_behaves_like 'require parentheses', 'a && b'
+  it_behaves_like 'require parentheses', 'a || b'
+  it_behaves_like 'require parentheses', 'a..b'
+  it_behaves_like 'require parentheses', 'method a'
+  it_behaves_like 'require parentheses', 'yield a'
+  it_behaves_like 'require parentheses', 'super a'
+  it_behaves_like 'require parentheses', 'a == b'
+
+  shared_examples 'require no parentheses' do |arg|
+    it "registers an offense and corrects when argument is `#{arg}`" do
+      expect_offense(<<~RUBY, arg: arg)
+        something if /regex/.match(%{arg})
+                     ^^^^^^^^^^^^^^^{arg}^ Use `=~` in places where the `MatchData` returned by `#match` will not be used.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        something if /regex/ =~ #{arg}
+      RUBY
+    end
+  end
+
+  it_behaves_like 'require no parentheses', 'if a then b else c end'
+  it_behaves_like 'require no parentheses', 'method(a)'
+  it_behaves_like 'require no parentheses', 'method'
+  it_behaves_like 'require no parentheses', 'yield'
+  it_behaves_like 'require no parentheses', 'super'
+  it_behaves_like 'require no parentheses', 'a.==(b)'
+
+  %w[| ^ & + - * / % ** > >= < <= << >>].each do |op|
+    it_behaves_like 'require no parentheses', "a #{op} b"
+  end
 end