Skip to content

Commit

Permalink
Handle more matching methods in Lint/OutOfRangeRegexpRef
Browse files Browse the repository at this point in the history
`when`, `grep`, `gsub`, `gsub!` `sub`, `sub!`, `[]`, `slice`, `slice!`,
`scan`, `index`, `rindex`, `partition`, `rpartition`, `start_with?`, and
`end_with?` can all be used to match regexps, so we should update the
range of valid references when we encounter them.
  • Loading branch information
eugeneius committed Aug 6, 2020
1 parent be47dc7 commit fa1fc0b
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#8463](https://github.com/rubocop-hq/rubocop/pull/8463): Fix false positives for `Lint/OutOfRangeRegexpRef` when a regexp is defined and matched in separate steps. ([@eugeneius][])
* [#8464](https://github.com/rubocop-hq/rubocop/pull/8464): Handle regexps matched with `when`, `grep`, `gsub`, `gsub!`, `sub`, `sub!`, `[]`, `slice`, `slice!`, `scan`, `index`, `rindex`, `partition`, `rpartition`, `start_with?`, and `end_with?` in `Lint/OutOfRangeRegexpRef`. ([@eugeneius][])
* [#8466](https://github.com/rubocop-hq/rubocop/issues/8466): Fix a false positive for `Lint/UriRegexp` when using `regexp` method without receiver. ([@koic][])
* [#8478](https://github.com/rubocop-hq/rubocop/issues/8478): Relax `Lint/BinaryOperatorWithIdenticalOperands` for mathematical operations. ([@marcandre][])

Expand Down
16 changes: 14 additions & 2 deletions lib/rubocop/cop/lint/out_of_range_regexp_ref.rb
Expand Up @@ -21,7 +21,10 @@ module Lint
class OutOfRangeRegexpRef < Base
MSG = 'Do not use out of range reference for the Regexp.'

REGEXP_CAPTURE_METHODS = %i[=~ === match].to_set.freeze
REGEXP_RECEIVER_METHODS = %i[=~ === match].to_set.freeze
REGEXP_ARGUMENT_METHODS = %i[=~ match grep gsub gsub! sub sub! [] slice slice! index rindex
scan partition rpartition start_with? end_with?].to_set.freeze
REGEXP_CAPTURE_METHODS = (REGEXP_RECEIVER_METHODS + REGEXP_ARGUMENT_METHODS).freeze

def on_new_investigation
@valid_ref = 0
Expand All @@ -38,11 +41,20 @@ def on_send(node)

if node.receiver&.regexp_type?
check_regexp(node.receiver)
elsif node.first_argument&.regexp_type? && node.method?(:=~)
elsif node.first_argument&.regexp_type? \
&& REGEXP_ARGUMENT_METHODS.include?(node.method_name)
check_regexp(node.first_argument)
end
end

def on_when(node)
regexp_conditions = node.conditions.select(&:regexp_type?)

@valid_ref = regexp_conditions.map do |condition|
check_regexp(condition)
end.compact.max
end

def on_nth_ref(node)
backref, = *node
return if @valid_ref.nil?
Expand Down
151 changes: 150 additions & 1 deletion spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb
Expand Up @@ -79,7 +79,7 @@
RUBY
end

it 'registers an offense when the regexp comes after `=~`' do
it 'registers an offense when the regexp appears on the right hand side of `=~`' do
expect_offense(<<~RUBY)
"foobar" =~ /(foo)(bar)/
puts $3
Expand Down Expand Up @@ -133,4 +133,153 @@
puts $2
RUBY
end

it 'does not register an offense when in range references are used inside a when clause' do
expect_no_offenses(<<~RUBY)
case "foobar"
when /(foo)(bar)/
$2
end
RUBY
end

it 'registers an offense when out of range references are used inside a when clause' do
expect_offense(<<~RUBY)
case "foobar"
when /(foo)(bar)/
$3
^^ Do not use out of range reference for the Regexp.
end
RUBY
end

it 'uses the maximum number of captures for when clauses with multiple conditions' do
expect_no_offenses(<<~RUBY)
case "foobarbaz"
when /(foo)(bar)/, /(bar)baz/
$2
end
RUBY

expect_offense(<<~RUBY)
case "foobarbaz"
when /(foo)(bar)/, /(bar)baz/
$3
^^ Do not use out of range reference for the Regexp.
end
RUBY
end

it 'only registers an offense for when clauses when the regexp is matched as a literal' do
expect_no_offenses(<<~RUBY)
case some_string
when some_regexp
$2
end
RUBY
end

it 'ignores regexp when clause conditions that contain interpolations' do
expect_offense(<<~'RUBY')
case "foobarbaz"
when /(foo)(bar)/, /#{var}/
$3
^^ Do not use out of range reference for the Regexp.
end
RUBY
end

context 'matching with `grep`' do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
%w[foo foobar].grep(/(foo)/) { $1 }
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY)
%w[foo foobar].grep(/(foo)/) { $2 }
^^ Do not use out of range reference for the Regexp.
RUBY
end

it 'only registers an offense when the regexp is matched as a literal' do
expect_no_offenses(<<~RUBY)
%w[foo foobar].grep(some_regexp) { $2 }
RUBY
end
end

context 'matching with `[]`' do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
"foobar"[/(foo)(bar)/]
puts $2
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY)
"foobar"[/(foo)(bar)/]
puts $3
^^ Do not use out of range reference for the Regexp.
RUBY
end

it 'only registers an offense when the regexp is matched as a literal' do
expect_no_offenses(<<~RUBY)
"foobar"[some_regexp]
puts $3
RUBY
end
end

%i[gsub gsub! sub sub! scan].each do |method|
context "matching with #{method}" do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
"foobar".#{method}(/(foo)(bar)/) { $2 }
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY, method: method)
"foobar".%{method}(/(foo)(bar)/) { $3 }
_{method} ^^ Do not use out of range reference for the Regexp.
RUBY
end

it 'only registers an offense when the regexp is matched as a literal' do
expect_no_offenses(<<~RUBY)
some_string.#{method}(some_regexp) { $3 }
RUBY
end
end
end

%i[match slice slice! index rindex partition rpartition start_with? end_with?].each do |method|
context "matching with #{method}" do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
"foobar".#{method}(/(foo)(bar)/)
puts $2
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY)
"foobar".#{method}(/(foo)(bar)/)
puts $3
^^ Do not use out of range reference for the Regexp.
RUBY
end

it 'only registers an offense when the regexp is matched as a literal' do
expect_no_offenses(<<~RUBY)
"foobar".#{method}(some_regexp)
puts $3
RUBY
end
end
end
end

0 comments on commit fa1fc0b

Please sign in to comment.