Skip to content

Commit

Permalink
[Fix rubocop#370] Fix an incorrect autocorrect for `Performance/Redun…
Browse files Browse the repository at this point in the history
…dantMatch`

Fixes rubocop#370.

This PR fixes an incorrect autocorrect for `Performance/RedundantMatch`
when expressions with lower precedence than `=~` are used as an argument.
  • Loading branch information
ymap committed Sep 14, 2023
1 parent ded9641 commit 4742c2a
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -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][])
33 changes: 32 additions & 1 deletion lib/rubocop/cop/performance/redundant_match.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -57,6 +59,35 @@ 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?
return requires_parentheses_for_call_like?(arg) if call_like?(arg)

false
end

def requires_parentheses_for_call_like?(arg)
return false if arg.parenthesized? || !arg.arguments?
return false if HIGHER_PRECEDENCE_OPERATOR_METHODS.include?(arg.method_name)

true
end

def call_like?(arg)
arg.call_type? || arg.yield_type? || arg.super_type?
end
end
end
end
Expand Down
46 changes: 46 additions & 0 deletions spec/rubocop/cop/performance/redundant_match_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 4742c2a

Please sign in to comment.