From fa1fc0b304ae9c29e3b77ad364dd892b7935ae32 Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Thu, 6 Aug 2020 00:49:30 +0100 Subject: [PATCH] Handle more matching methods in Lint/OutOfRangeRegexpRef `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. --- CHANGELOG.md | 1 + .../cop/lint/out_of_range_regexp_ref.rb | 16 +- .../cop/lint/out_of_range_regexp_ref_spec.rb | 151 +++++++++++++++++- 3 files changed, 165 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e05d140f7f..1dabf36eecb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) diff --git a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb index 4c3a52a6064..a5f87ae2c0c 100644 --- a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb +++ b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb @@ -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 @@ -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? diff --git a/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb b/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb index 7c748988f5a..70b12fb6239 100644 --- a/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb +++ b/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb @@ -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 @@ -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