Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pattern matching for Lint/OutOfRangeRegexpRef cop #11337

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#10976](https://github.com/rubocop/rubocop/issues/10976): Support pattern matching for `Lint/OutOfRangeRegexpRef` cop. ([@fatkodima][])
19 changes: 19 additions & 0 deletions lib/rubocop/cop/lint/out_of_range_regexp_ref.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ def on_when(node)
@valid_ref = regexp_conditions.map { |condition| check_regexp(condition) }.compact.max
end

def on_in_pattern(node)
regexp_patterns = patterns(node).select(&:regexp_type?)

@valid_ref = regexp_patterns.map { |pattern| check_regexp(pattern) }.compact.max
end

def on_nth_ref(node)
backref, = *node
return if @valid_ref.nil? || backref <= @valid_ref
Expand All @@ -84,6 +90,19 @@ def on_nth_ref(node)

private

def patterns(pattern_node)
pattern = pattern_node.node_parts[0]

case pattern.type
when :array_pattern, :match_alt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fatkodima @bbatsov
I may be wrong, as I only glanced at it, but I don't think it supports hash pattern/find pattern/value pattern with the pin operator(^(expression)).

{a: 'foo'} => {a: /(.)/}
p $1

['foo'] => [*, /(.)/, *]
p $1

'foo' => ^(/(.)/)
p $1

The last one may not need to be fixed since it is difficult to fully support and is not a practical code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting. Indeed, this PR's implementation should be done in a simpler way - #11573

pattern.children
when :match_as
patterns(pattern)
else
[pattern]
end
end

def check_regexp(node)
return if node.interpolation?

Expand Down
148 changes: 148 additions & 0 deletions spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,154 @@
RUBY
end

context 'pattern matching', :ruby27 do
context 'matching variable' do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
case "foobar"
in /(foo)(bar)/
$2
end
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY)
case "foobar"
in /(foo)(bar)/
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end
end

context 'matching arrays' do
it 'uses the maximum number of captures with multiple patterns' do
expect_no_offenses(<<~RUBY)
case array
in [/(foo)(bar)/, /(bar)baz/]
$2
end
RUBY

expect_offense(<<~RUBY)
case array
in [/(foo)(bar)/, /(bar)baz/]
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end
end

context 'matching with aliases' do
context 'variable aliases' do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
case "foobar"
in /(foo)(bar)/ => x
$2
end
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY)
case "foobar"
in /(foo)(bar)/ => x
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end
end

context 'array aliases' do
it 'uses the maximum number of captures with multiple patterns' do
expect_no_offenses(<<~RUBY)
case array
in [/(foo)(bar)/, /(bar)baz/] => x
$2
end
RUBY

expect_offense(<<~RUBY)
case array
in [/(foo)(bar)/, /(bar)baz/] => x
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end
end
end

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

expect_no_offenses(<<~RUBY)
case "foobar"
in /(foo)(bar)/ | "foo" => x
$2
end
RUBY
end

it 'registers an offense when out of range references are used' do
expect_offense(<<~RUBY)
case "foobar"
in /(foo)(bar)/ | "foo"
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end

it 'uses the maximum number of captures with multiple patterns' do
expect_no_offenses(<<~RUBY)
case "foobar"
in /(foo)baz/ | /(foo)(bar)/
$2
end
RUBY

expect_offense(<<~RUBY)
case "foobar"
in /(foo)baz/ | /(foo)(bar)/
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end
end

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

it 'ignores regexp when clause conditions contain interpolations' do
expect_offense(<<~'RUBY')
case array
in [/(foo)(bar)/, /#{var}/]
$3
^^ $3 is out of range (2 regexp capture groups detected).
end
RUBY
end
end

context 'matching with `grep`' do
it 'does not register an offense when in range references are used' do
expect_no_offenses(<<~RUBY)
Expand Down