Skip to content

Commit

Permalink
Fix a false positive for Performance/RegexpMatch
Browse files Browse the repository at this point in the history
Follow up rubocop/rubocop#5569 (comment).

This commit fixes a false positive for `Performance/RegexpMatch`
when `MatchData` exists after guard condition.

The following is an example code with false positive.

```ruby
def foo
  return if /re/ =~ str

  # `Regexp.last_match[1]` cannot be referenced if changed from `=~` to `match?`.
  do_something(Regexp.last_match[1])
end
```
  • Loading branch information
koic committed Jul 27, 2019
1 parent d8026ba commit 367c631
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 19 deletions.
8 changes: 4 additions & 4 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2019-07-27 17:51:55 +0800 using RuboCop version 0.73.0.
# on 2019-07-27 22:39:00 +0800 using RuboCop version 0.73.0.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
Expand All @@ -22,9 +22,9 @@ Metrics/AbcSize:
# Offense count: 2
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 156
Max: 164

# Offense count: 15
# Offense count: 14
# Configuration parameters: CountComments, ExcludedMethods.
Metrics/MethodLength:
Max: 14
Expand All @@ -44,7 +44,7 @@ RSpec/ContextWording:
- 'spec/rubocop/cop/performance/string_replacement_spec.rb'
- 'spec/rubocop/cop/performance/times_map_spec.rb'

# Offense count: 88
# Offense count: 89
# Configuration parameters: Max.
RSpec/ExampleLength:
Enabled: false
Expand Down
41 changes: 26 additions & 15 deletions lib/rubocop/cop/performance/regexp_match.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,30 +177,41 @@ def last_match_used?(match_node)
scope_root = scope_root(match_node)
body = scope_root ? scope_body(scope_root) : match_node.ancestors.last

range =
if match_node.parent.if_type? && match_node.parent.modifier_form?
if_branch_range(match_node)
else
match_node_pos = match_node.loc.expression.begin_pos
next_match_pos = next_match_pos(body, match_node_pos, scope_root)

match_node_pos..next_match_pos
end
range = range_to_search_for_last_matches(match_node, body, scope_root)

find_last_match(body, range, scope_root)
end

def if_branch_range(match_node)
expression = match_node.parent.if_branch.loc.expression
def range_to_search_for_last_matches(match_node, body, scope_root)
expression = if (modifier_form = modifier_form?(match_node))
match_node.parent.if_branch.loc.expression
else
match_node.loc.expression
end

match_node_pos = expression.begin_pos
next_match_pos = next_match_pos(
body, match_node_pos, scope_root, modifier_form
)

match_node_pos..next_match_pos
end

expression.begin_pos..expression.end_pos
def modifier_form?(match_node)
match_node.parent.if_type? && match_node.parent.modifier_form?
end

def next_match_pos(body, match_node_pos, scope_root)
def next_match_pos(body, match_node_pos, scope_root, modifier_form)
node = search_match_nodes(body).find do |match|
match.loc.expression.begin_pos > match_node_pos &&
scope_root(match) == scope_root
begin_pos = if modifier_form
match.parent.if_branch.loc.expression.begin_pos
else
match.loc.expression.begin_pos
end

begin_pos > match_node_pos && scope_root(match) == scope_root
end

node ? node.loc.expression.begin_pos : Float::INFINITY
end

Expand Down
31 changes: 31 additions & 0 deletions spec/rubocop/cop/performance/regexp_match_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,37 @@ def foo
RUBY
end

it "accepts #{name} in guard condition with " \
"#{var} is used in the line after that" do
expect_no_offenses(<<-RUBY)
def foo
return if #{cond}
do_something(#{var})
end
RUBY
end

include_examples 'offense',
"#{name} in if guard condition with " \
"#{var} is used in another method", <<-RUBY, <<-RUBY2
def foo
return if #{cond}
end
def bar
do_something(#{var})
end
RUBY
def foo
return if #{correction}
end
def bar
do_something(#{var})
end
RUBY2

it "accepts #{name} in method with #{var} in block" do
expect_no_offenses(<<-RUBY)
def foo
Expand Down

0 comments on commit 367c631

Please sign in to comment.