diff --git a/changelog/fix_an_incorrect_autocorrect_for_performance_redundant_match.md b/changelog/fix_an_incorrect_autocorrect_for_performance_redundant_match.md new file mode 100644 index 0000000000..cf7d8dfe26 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/performance/redundant_match.rb b/lib/rubocop/cop/performance/redundant_match.rb index ef7a1a8eef..a4dc15b56e 100644 --- a/lib/rubocop/cop/performance/redundant_match.rb +++ b/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 diff --git a/spec/rubocop/cop/performance/redundant_match_spec.rb b/spec/rubocop/cop/performance/redundant_match_spec.rb index 7ec04defe4..0d6a8767ea 100644 --- a/spec/rubocop/cop/performance/redundant_match_spec.rb +++ b/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