From d1e1f648b34c1d03cc156783a8884d5d6615678e Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Mon, 27 Jul 2020 22:19:26 +0530 Subject: [PATCH 01/10] Add new Lint/OutOfRangeRefInRegexp cop #7755 --- CHANGELOG.md | 2 + config/default.yml | 5 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_lint.adoc | 15 +++ docs/modules/ROOT/pages/installation.adoc | 2 +- lib/rubocop.rb | 1 + .../cop/lint/out_of_range_ref_in_regexp.rb | 71 +++++++++++ .../lint/out_of_range_ref_in_regexp_spec.rb | 111 ++++++++++++++++++ 8 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb create mode 100644 spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 613f859a6ba..a92a4a658ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ * [#8417](https://github.com/rubocop-hq/rubocop/pull/8417): Add new `Style/GlobalStdStream` cop. ([@fatkodima][]) * [#7949](https://github.com/rubocop-hq/rubocop/issues/7949): Add new `Style/SingleArgumentDig` cop. ([@volfgox][]) * [#8341](https://github.com/rubocop-hq/rubocop/pull/8341): Add new `Lint/EmptyConditionalBody` cop. ([@fatkodima][]) +* [#7755](https://github.com/rubocop-hq/rubocop/issues/7755): Add new `Lint/OutOfRangeRefInRegexp` cop. ([@sonalinavlakhe][]) ### Bug fixes @@ -4750,3 +4751,4 @@ [@iamravitejag]: https://github.com/iamravitejag [@volfgox]: https://github.com/volfgox [@dsavochkin]: https://github.com/dmytro-savochkin +[@sonalinavlakhe]: https://github.com/sonalinavlakhe diff --git a/config/default.yml b/config/default.yml index d12af9e9150..29a998997ba 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1640,6 +1640,11 @@ Lint/OrderedMagicComments: Enabled: true VersionAdded: '0.53' +Lint/OutOfRangeRefInRegexp: + Description: 'Checks for out of range reference for Regep because it always returns nil.' + Enabled: pending + VersionAdded: '0.89' + Lint/ParenthesesAsGroupedExpression: Description: >- Checks for method calls with a space before the opening diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index b971767730f..2d6acef737d 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -230,6 +230,7 @@ In the following section you find all available cops: * xref:cops_lint.adoc#lintnonlocalexitfromiterator[Lint/NonLocalExitFromIterator] * xref:cops_lint.adoc#lintnumberconversion[Lint/NumberConversion] * xref:cops_lint.adoc#lintorderedmagiccomments[Lint/OrderedMagicComments] +* xref:cops_lint.adoc#lintoutofrangerefinregexp[Lint/OutOfRangeRefInRegexp] * xref:cops_lint.adoc#lintparenthesesasgroupedexpression[Lint/ParenthesesAsGroupedExpression] * xref:cops_lint.adoc#lintpercentstringarray[Lint/PercentStringArray] * xref:cops_lint.adoc#lintpercentsymbolarray[Lint/PercentSymbolArray] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 440dfe09301..f04003d9415 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2356,6 +2356,21 @@ p [''.frozen?, ''.encoding] #=> [true, #] p [''.frozen?, ''.encoding] #=> [true, #] ---- +== Lint/OutOfRangeRefInRegexp + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| No +| 0.89 +| - +|=== + +# good + puts $1 # => foo + == Lint/ParenthesesAsGroupedExpression |=== diff --git a/docs/modules/ROOT/pages/installation.adoc b/docs/modules/ROOT/pages/installation.adoc index 6e2f1c8a7d6..d3aa4f188b2 100644 --- a/docs/modules/ROOT/pages/installation.adoc +++ b/docs/modules/ROOT/pages/installation.adoc @@ -1,4 +1,4 @@ -= Installation + = Installation RuboCop's installation is pretty standard: diff --git a/lib/rubocop.rb b/lib/rubocop.rb index aee3e7f71b1..ac95663721c 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -288,6 +288,7 @@ require_relative 'rubocop/cop/lint/non_local_exit_from_iterator' require_relative 'rubocop/cop/lint/number_conversion' require_relative 'rubocop/cop/lint/ordered_magic_comments' +require_relative 'rubocop/cop/lint/out_of_range_ref_in_regexp' require_relative 'rubocop/cop/lint/parentheses_as_grouped_expression' require_relative 'rubocop/cop/lint/percent_string_array' require_relative 'rubocop/cop/lint/percent_symbol_array' diff --git a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb new file mode 100644 index 00000000000..fca27431e90 --- /dev/null +++ b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cops looks for out of range referencing for Regexp, as while capturing groups out of + # out of range reference always returns nil. + + # @example + # /(foo)bar/ =~ 'foobar'\ + + # # bad - always returns nil + # puts $2 # => nil + + # # good + # puts $1 # => foo + # + class OutOfRangeRefInRegexp < Cop + MSG = 'Do not use out of range reference for the Regexp.' + + def investigate(processed_source) + ast = processed_source.ast + valid_ref = cop_config['Count'] + ast.each_node do |node| + if node.regexp_type? + break if contain_non_literal?(node) + + tree = parse_node(node.content) + break if tree.nil? + + valid_ref = regexp_captures(tree) + elsif node.nth_ref_type? + backref, = *node + add_offense(node) if backref > valid_ref + end + end + end + + private + + def contain_non_literal?(node) + if node.respond_to?(:type) && (node.variable? || node.send_type? || node.const_type?) + return true + end + return false unless node.respond_to?(:children) + + node.children.any? { |child| contain_non_literal?(child) } + end + + def parse_node(content) + Regexp::Parser.parse(content) + rescue Regexp::Scanner::ScannerError + nil + end + + def regexp_captures(tree) + named_capture = numbered_capture = 0 + tree.each_expression do |e| + named_capture += 1 if e.instance_of?(Regexp::Expression::Group::Named) + numbered_capture += 1 if e.instance_of?(Regexp::Expression::Group::Capture) + end + return named_capture if numbered_capture.zero? + + return numbered_capture if named_capture.zero? + + named_capture + end + end + end + end +end diff --git a/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb b/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb new file mode 100644 index 00000000000..6e5069d5d45 --- /dev/null +++ b/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::OutOfRangeRefInRegexp do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + it 'registers an offense when out of range references are used for named captures' do + expect_offense(<<~RUBY) + /(?FOO)(?BAR)/ =~ "FOOBAR" + puts $3 + ^^ Do not use out of range reference for the Regexp. + RUBY + end + + it 'registers an offense when out of range references are used for numbered captures' do + expect_offense(<<~RUBY) + /(foo)(bar)/ =~ "foobar" + puts $3 + ^^ Do not use out of range reference for the Regexp. + RUBY + end + + it 'registers an offense when out of range references are used for mix of numbered and named captures' do + expect_offense(<<~RUBY) + /(?FOO)(BAR)/ =~ "FOOBAR" + puts $2 + ^^ Do not use out of range reference for the Regexp. + RUBY + end + + it 'registers an offense when out of range references are used for non captures' do + expect_offense(<<~RUBY) + /bar/ =~ 'foo' + puts $1 + ^^ Do not use out of range reference for the Regexp. + RUBY + end + + it 'does not register offense to a regexp with valid references for named captures' do + expect_no_offenses(<<~RUBY) + /(?FOO)(?BAR)/ =~ "FOOBAR" + puts $1 + puts $2 + RUBY + end + + it 'does not register offense to a regexp with valid references for numbered captures' do + expect_no_offenses(<<~RUBY) + /(foo)(bar)/ =~ "foobar" + puts $1 + puts $2 + RUBY + end + + it 'does not register offense to a regexp with valid references for a mix named and numbered captures' do + expect_no_offenses(<<~RUBY) + /(?FOO)(BAR)/ =~ "FOOBAR" + puts $1 + RUBY + end + + # See https://github.com/rubocop-hq/rubocop/issues/8083 + it 'does not register offense when using a Regexp cannot be processed by regexp_parser gem' do + expect_no_offenses(<<~'RUBY') + /data = ({"words":.+}}}[^}]*})/m + RUBY + end + + # RuboCop does not know a value of variables that it will contain in the regexp literal. + # For example, `/(?#{var}*)` is interpreted as `/(?*)`. + # So it does not offense when variables are used in regexp literals. + context 'when containing a non-regexp literal' do + it 'does not register an offence when containing a lvar' do + expect_no_offenses(<<~'RUBY') + var = '(\d+)' + /(?#{var}*)/ + RUBY + end + + it 'does not register an offence when containing a ivar' do + expect_no_offenses(<<~'RUBY') + /(?#{@var}*)/ + RUBY + end + + it 'does not register an offence when containing a cvar' do + expect_no_offenses(<<~'RUBY') + /(?#{@@var}*)/ + RUBY + end + + it 'does not register an offence when containing a gvar' do + expect_no_offenses(<<~'RUBY') + /(?#{$var}*)/ + RUBY + end + + it 'does not register an offence when containing a method' do + expect_no_offenses(<<~'RUBY') + /(?#{do_something}*)/ + RUBY + end + + it 'does not register an offence when containing a constant' do + expect_no_offenses(<<~'RUBY') + /(?#{CONST}*)/ + RUBY + end + end +end From 98d8cc4c26dab2b55c5c04ae3dd86ec8ded5d52f Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Tue, 28 Jul 2020 15:25:46 +0530 Subject: [PATCH 02/10] code review fixes --- .../cop/lint/out_of_range_ref_in_regexp.rb | 40 +++++++++---------- .../lint/out_of_range_ref_in_regexp_spec.rb | 31 +++++++++++--- 2 files changed, 44 insertions(+), 27 deletions(-) diff --git a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb index fca27431e90..a3384dc6082 100644 --- a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb +++ b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb @@ -15,25 +15,29 @@ module Lint # # good # puts $1 # => foo # - class OutOfRangeRefInRegexp < Cop + class OutOfRangeRefInRegexp < Base MSG = 'Do not use out of range reference for the Regexp.' - def investigate(processed_source) - ast = processed_source.ast - valid_ref = cop_config['Count'] - ast.each_node do |node| - if node.regexp_type? - break if contain_non_literal?(node) + def on_regexp(node) + @valid_ref = cop_config['Count'] + return if contain_non_literal?(node) - tree = parse_node(node.content) - break if tree.nil? - - valid_ref = regexp_captures(tree) - elsif node.nth_ref_type? - backref, = *node - add_offense(node) if backref > valid_ref - end + begin + tree = Regexp::Parser.parse(node.content) + # Returns if a regular expression that cannot be processed by regexp_parser gem. + # https://github.com/rubocop-hq/rubocop/issues/8083 + rescue Regexp::Scanner::ScannerError + return end + + @valid_ref = regexp_captures(tree) + end + + def on_nth_ref(node) + backref, = *node + return if @valid_ref.nil? + + add_offense(node) if backref > @valid_ref end private @@ -47,12 +51,6 @@ def contain_non_literal?(node) node.children.any? { |child| contain_non_literal?(child) } end - def parse_node(content) - Regexp::Parser.parse(content) - rescue Regexp::Scanner::ScannerError - nil - end - def regexp_captures(tree) named_capture = numbered_capture = 0 tree.each_expression do |e| diff --git a/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb b/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb index 6e5069d5d45..e25124221e6 100644 --- a/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb +++ b/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb @@ -74,37 +74,56 @@ it 'does not register an offence when containing a lvar' do expect_no_offenses(<<~'RUBY') var = '(\d+)' - /(?#{var}*)/ + /(?#{var}*)/ =~ "12" + puts $1 + puts $2 RUBY end it 'does not register an offence when containing a ivar' do expect_no_offenses(<<~'RUBY') - /(?#{@var}*)/ + @var = '(\d+)' + /(?#{@var}*)/ =~ "12" + puts $1 + puts $3 RUBY end it 'does not register an offence when containing a cvar' do expect_no_offenses(<<~'RUBY') - /(?#{@@var}*)/ + @@var = '(\d+)' + /(?#{@@var}*)/ =~ "12" + puts $1 + puts $4 RUBY end it 'does not register an offence when containing a gvar' do expect_no_offenses(<<~'RUBY') - /(?#{$var}*)/ + $var = '(\d+)' + /(?#{$var}*)/ =~ "12" + puts $1 + puts $2 RUBY end it 'does not register an offence when containing a method' do expect_no_offenses(<<~'RUBY') - /(?#{do_something}*)/ + def do_something + '(\d+)' + end + /(?#{do_something}*)/ =~ "12" + puts $1 + puts $4 RUBY end it 'does not register an offence when containing a constant' do expect_no_offenses(<<~'RUBY') - /(?#{CONST}*)/ + CONST = "12" + /(?#{CONST}*)/ =~ "12" + puts $1 + puts $3 RUBY end end From 2beea659e31fc45ad557008c616cee05730ea27a Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Wed, 29 Jul 2020 15:53:51 +0530 Subject: [PATCH 03/10] code review fixes --- docs/modules/ROOT/pages/cops_lint.adoc | 2 +- lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index f04003d9415..3814ba782c8 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2362,7 +2362,7 @@ p [''.frozen?, ''.encoding] #=> [true, #] | Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged | Pending -| Yes +| No | No | 0.89 | - diff --git a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb index a3384dc6082..a140d8a42ee 100644 --- a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb +++ b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb @@ -57,11 +57,7 @@ def regexp_captures(tree) named_capture += 1 if e.instance_of?(Regexp::Expression::Group::Named) numbered_capture += 1 if e.instance_of?(Regexp::Expression::Group::Capture) end - return named_capture if numbered_capture.zero? - - return numbered_capture if named_capture.zero? - - named_capture + named_capture.positive? ? named_capture : numbered_capture end end end From a5e89f5d4e9fd81ebb6a90bfd63d96ae227433dd Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Wed, 29 Jul 2020 16:47:50 +0530 Subject: [PATCH 04/10] mark a cop unsafe --- config/default.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/default.yml b/config/default.yml index 29a998997ba..63f9a64e8e7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1643,6 +1643,7 @@ Lint/OrderedMagicComments: Lint/OutOfRangeRefInRegexp: Description: 'Checks for out of range reference for Regep because it always returns nil.' Enabled: pending + Safe: false VersionAdded: '0.89' Lint/ParenthesesAsGroupedExpression: From 51dda408d4640914e119ae38520910fdbff5cf2c Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Thu, 30 Jul 2020 12:23:06 +0530 Subject: [PATCH 05/10] refactor the contain_non_literal? method --- lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb index a140d8a42ee..a989054f1ef 100644 --- a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb +++ b/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb @@ -43,12 +43,7 @@ def on_nth_ref(node) private def contain_non_literal?(node) - if node.respond_to?(:type) && (node.variable? || node.send_type? || node.const_type?) - return true - end - return false unless node.respond_to?(:children) - - node.children.any? { |child| contain_non_literal?(child) } + node.children.size != 2 || !node.children.first.str_type? end def regexp_captures(tree) From 4efe602000bc16af8be38b8a5e2b498eeebe5ca3 Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Thu, 30 Jul 2020 20:57:54 +0530 Subject: [PATCH 06/10] code review fixes and rename lint to OutOfRangeRegexpRef --- CHANGELOG.md | 1 + config/default.yml | 4 ++-- docs/modules/ROOT/pages/cops.adoc | 2 +- docs/modules/ROOT/pages/cops_lint.adoc | 2 +- docs/modules/ROOT/pages/installation.adoc | 2 +- lib/rubocop.rb | 2 +- ...t_of_range_ref_in_regexp.rb => out_of_range_regexp_ref.rb} | 2 +- ..._ref_in_regexp_spec.rb => out_of_range_regexp_ref_spec.rb} | 2 +- 8 files changed, 9 insertions(+), 8 deletions(-) rename lib/rubocop/cop/lint/{out_of_range_ref_in_regexp.rb => out_of_range_regexp_ref.rb} (97%) rename spec/rubocop/cop/lint/{out_of_range_ref_in_regexp_spec.rb => out_of_range_regexp_ref_spec.rb} (98%) diff --git a/CHANGELOG.md b/CHANGELOG.md index a92a4a658ef..7cb6775e397 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ * [#7949](https://github.com/rubocop-hq/rubocop/issues/7949): Add new `Style/SingleArgumentDig` cop. ([@volfgox][]) * [#8341](https://github.com/rubocop-hq/rubocop/pull/8341): Add new `Lint/EmptyConditionalBody` cop. ([@fatkodima][]) * [#7755](https://github.com/rubocop-hq/rubocop/issues/7755): Add new `Lint/OutOfRangeRefInRegexp` cop. ([@sonalinavlakhe][]) +* [#7755](https://github.com/rubocop-hq/rubocop/issues/7755): Add new `Lint/OutOfRangeRegexpRef` cop. ([@sonalinavlakhe][]) ### Bug fixes diff --git a/config/default.yml b/config/default.yml index 63f9a64e8e7..d8e2d72c4d7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1640,8 +1640,8 @@ Lint/OrderedMagicComments: Enabled: true VersionAdded: '0.53' -Lint/OutOfRangeRefInRegexp: - Description: 'Checks for out of range reference for Regep because it always returns nil.' +Lint/OutOfRangeRegexpRef: + Description: 'Checks for out of range reference for Regexp because it always returns nil.' Enabled: pending Safe: false VersionAdded: '0.89' diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 2d6acef737d..f900a87f38b 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -230,7 +230,7 @@ In the following section you find all available cops: * xref:cops_lint.adoc#lintnonlocalexitfromiterator[Lint/NonLocalExitFromIterator] * xref:cops_lint.adoc#lintnumberconversion[Lint/NumberConversion] * xref:cops_lint.adoc#lintorderedmagiccomments[Lint/OrderedMagicComments] -* xref:cops_lint.adoc#lintoutofrangerefinregexp[Lint/OutOfRangeRefInRegexp] +* xref:cops_lint.adoc#lintoutofrangeregexpref[Lint/OutOfRangeRegexpRef] * xref:cops_lint.adoc#lintparenthesesasgroupedexpression[Lint/ParenthesesAsGroupedExpression] * xref:cops_lint.adoc#lintpercentstringarray[Lint/PercentStringArray] * xref:cops_lint.adoc#lintpercentsymbolarray[Lint/PercentSymbolArray] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 3814ba782c8..ed4e5cd96d1 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2356,7 +2356,7 @@ p [''.frozen?, ''.encoding] #=> [true, #] p [''.frozen?, ''.encoding] #=> [true, #] ---- -== Lint/OutOfRangeRefInRegexp +== Lint/OutOfRangeRegexpRef |=== | Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/docs/modules/ROOT/pages/installation.adoc b/docs/modules/ROOT/pages/installation.adoc index d3aa4f188b2..6e2f1c8a7d6 100644 --- a/docs/modules/ROOT/pages/installation.adoc +++ b/docs/modules/ROOT/pages/installation.adoc @@ -1,4 +1,4 @@ - = Installation += Installation RuboCop's installation is pretty standard: diff --git a/lib/rubocop.rb b/lib/rubocop.rb index ac95663721c..e96e16eb34f 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -288,7 +288,7 @@ require_relative 'rubocop/cop/lint/non_local_exit_from_iterator' require_relative 'rubocop/cop/lint/number_conversion' require_relative 'rubocop/cop/lint/ordered_magic_comments' -require_relative 'rubocop/cop/lint/out_of_range_ref_in_regexp' +require_relative 'rubocop/cop/lint/out_of_range_regexp_ref' require_relative 'rubocop/cop/lint/parentheses_as_grouped_expression' require_relative 'rubocop/cop/lint/percent_string_array' require_relative 'rubocop/cop/lint/percent_symbol_array' diff --git a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb similarity index 97% rename from lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb rename to lib/rubocop/cop/lint/out_of_range_regexp_ref.rb index a989054f1ef..328a8bb5250 100644 --- a/lib/rubocop/cop/lint/out_of_range_ref_in_regexp.rb +++ b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb @@ -15,7 +15,7 @@ module Lint # # good # puts $1 # => foo # - class OutOfRangeRefInRegexp < Base + class OutOfRangeRegexpRef < Base MSG = 'Do not use out of range reference for the Regexp.' def on_regexp(node) diff --git a/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb b/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb similarity index 98% rename from spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb rename to spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb index e25124221e6..5a95e05b54d 100644 --- a/spec/rubocop/cop/lint/out_of_range_ref_in_regexp_spec.rb +++ b/spec/rubocop/cop/lint/out_of_range_regexp_ref_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Lint::OutOfRangeRefInRegexp do +RSpec.describe RuboCop::Cop::Lint::OutOfRangeRegexpRef do subject(:cop) { described_class.new(config) } let(:config) { RuboCop::Config.new } From 6984914f70563b41bf80ff0baf364fbaa7db8a15 Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Sat, 1 Aug 2020 16:21:20 +0530 Subject: [PATCH 07/10] refactor regexp_captures method --- lib/rubocop/cop/lint/out_of_range_regexp_ref.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 328a8bb5250..d9c153f2eaa 100644 --- a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb +++ b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb @@ -7,7 +7,7 @@ module Lint # out of range reference always returns nil. # @example - # /(foo)bar/ =~ 'foobar'\ + # /(foo)bar/ =~ 'foobar' # # bad - always returns nil # puts $2 # => nil @@ -18,8 +18,12 @@ module Lint class OutOfRangeRegexpRef < Base MSG = 'Do not use out of range reference for the Regexp.' + def on_new_investigation + @valid_ref = 0 + end + def on_regexp(node) - @valid_ref = cop_config['Count'] + @valid_ref = nil return if contain_non_literal?(node) begin @@ -49,8 +53,9 @@ def contain_non_literal?(node) def regexp_captures(tree) named_capture = numbered_capture = 0 tree.each_expression do |e| - named_capture += 1 if e.instance_of?(Regexp::Expression::Group::Named) - numbered_capture += 1 if e.instance_of?(Regexp::Expression::Group::Capture) + if e.type?(:group) + e.respond_to?(:name) ? named_capture += 1 : numbered_capture += 1 + end end named_capture.positive? ? named_capture : numbered_capture end From 29d162fe70f08821de39517c5130c53261e6ac1d Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Sun, 2 Aug 2020 19:10:06 +0530 Subject: [PATCH 08/10] Update examples in specs --- .../cop/lint/out_of_range_regexp_ref_spec.rb | 71 ++++--------------- 1 file changed, 15 insertions(+), 56 deletions(-) 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 5a95e05b54d..50a2ff73e74 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 @@ -5,6 +5,13 @@ let(:config) { RuboCop::Config.new } + it 'registers an offense when references are used before any Regexp' do + expect_offense(<<~RUBY) + puts $3 + ^^ Do not use out of range reference for the Regexp. + RUBY + end + it 'registers an offense when out of range references are used for named captures' do expect_offense(<<~RUBY) /(?FOO)(?BAR)/ =~ "FOOBAR" @@ -64,67 +71,19 @@ it 'does not register offense when using a Regexp cannot be processed by regexp_parser gem' do expect_no_offenses(<<~'RUBY') /data = ({"words":.+}}}[^}]*})/m + puts $1 RUBY end # RuboCop does not know a value of variables that it will contain in the regexp literal. # For example, `/(?#{var}*)` is interpreted as `/(?*)`. # So it does not offense when variables are used in regexp literals. - context 'when containing a non-regexp literal' do - it 'does not register an offence when containing a lvar' do - expect_no_offenses(<<~'RUBY') - var = '(\d+)' - /(?#{var}*)/ =~ "12" - puts $1 - puts $2 - RUBY - end - - it 'does not register an offence when containing a ivar' do - expect_no_offenses(<<~'RUBY') - @var = '(\d+)' - /(?#{@var}*)/ =~ "12" - puts $1 - puts $3 - RUBY - end - - it 'does not register an offence when containing a cvar' do - expect_no_offenses(<<~'RUBY') - @@var = '(\d+)' - /(?#{@@var}*)/ =~ "12" - puts $1 - puts $4 - RUBY - end - - it 'does not register an offence when containing a gvar' do - expect_no_offenses(<<~'RUBY') - $var = '(\d+)' - /(?#{$var}*)/ =~ "12" - puts $1 - puts $2 - RUBY - end - - it 'does not register an offence when containing a method' do - expect_no_offenses(<<~'RUBY') - def do_something - '(\d+)' - end - /(?#{do_something}*)/ =~ "12" - puts $1 - puts $4 - RUBY - end - - it 'does not register an offence when containing a constant' do - expect_no_offenses(<<~'RUBY') - CONST = "12" - /(?#{CONST}*)/ =~ "12" - puts $1 - puts $3 - RUBY - end + it 'does not register an offence Regexp containing non literal' do + expect_no_offenses(<<~'RUBY') + var = '(\d+)' + /(?#{var}*)/ =~ "12" + puts $1 + puts $2 + RUBY end end From b17f32a8c664f93b486d63afd038febabcf91526 Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Mon, 3 Aug 2020 12:06:15 +0530 Subject: [PATCH 09/10] fix review comments --- docs/modules/ROOT/pages/cops_lint.adoc | 17 ++++++++++++++++- lib/rubocop/cop/lint/out_of_range_regexp_ref.rb | 9 ++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index ed4e5cd96d1..f40c2fc2627 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2368,8 +2368,23 @@ p [''.frozen?, ''.encoding] #=> [true, #] | - |=== +This cops looks for out of range referencing for Regexp, as while capturing groups out of +out of range reference always returns nil. + +=== Examples + +[source,ruby] +---- +/(foo)bar/ =~ 'foobar' + +# bad - always returns nil + +puts $2 # => nil + # good - puts $1 # => foo + +puts $1 # => foo +---- == Lint/ParenthesesAsGroupedExpression 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 d9c153f2eaa..0ca5e562abb 100644 --- a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb +++ b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb @@ -5,14 +5,17 @@ module Cop module Lint # This cops looks for out of range referencing for Regexp, as while capturing groups out of # out of range reference always returns nil. - + # # @example + # # /(foo)bar/ =~ 'foobar' - + # # # bad - always returns nil + # # puts $2 # => nil - + # # # good + # # puts $1 # => foo # class OutOfRangeRegexpRef < Base From b5e412af659e53eeda8f546a6d23c167edbdc176 Mon Sep 17 00:00:00 2001 From: Sonali Bhavsar Date: Wed, 5 Aug 2020 20:37:57 +0530 Subject: [PATCH 10/10] Remove rescue block and update test --- CHANGELOG.md | 1 - docs/modules/ROOT/pages/cops_lint.adoc | 4 ++-- lib/rubocop/cop/lint/out_of_range_regexp_ref.rb | 13 +++---------- .../cop/lint/out_of_range_regexp_ref_spec.rb | 8 -------- 4 files changed, 5 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb6775e397..8896c4390c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ * [#8417](https://github.com/rubocop-hq/rubocop/pull/8417): Add new `Style/GlobalStdStream` cop. ([@fatkodima][]) * [#7949](https://github.com/rubocop-hq/rubocop/issues/7949): Add new `Style/SingleArgumentDig` cop. ([@volfgox][]) * [#8341](https://github.com/rubocop-hq/rubocop/pull/8341): Add new `Lint/EmptyConditionalBody` cop. ([@fatkodima][]) -* [#7755](https://github.com/rubocop-hq/rubocop/issues/7755): Add new `Lint/OutOfRangeRefInRegexp` cop. ([@sonalinavlakhe][]) * [#7755](https://github.com/rubocop-hq/rubocop/issues/7755): Add new `Lint/OutOfRangeRegexpRef` cop. ([@sonalinavlakhe][]) ### Bug fixes diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index f40c2fc2627..aad7d1e956d 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -2368,8 +2368,8 @@ p [''.frozen?, ''.encoding] #=> [true, #] | - |=== -This cops looks for out of range referencing for Regexp, as while capturing groups out of -out of range reference always returns nil. +This cops looks for references of Regexp captures that are out of range +and thus always returns nil. === Examples 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 0ca5e562abb..8a81e2a2f9b 100644 --- a/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb +++ b/lib/rubocop/cop/lint/out_of_range_regexp_ref.rb @@ -3,8 +3,8 @@ module RuboCop module Cop module Lint - # This cops looks for out of range referencing for Regexp, as while capturing groups out of - # out of range reference always returns nil. + # This cops looks for references of Regexp captures that are out of range + # and thus always returns nil. # # @example # @@ -29,14 +29,7 @@ def on_regexp(node) @valid_ref = nil return if contain_non_literal?(node) - begin - tree = Regexp::Parser.parse(node.content) - # Returns if a regular expression that cannot be processed by regexp_parser gem. - # https://github.com/rubocop-hq/rubocop/issues/8083 - rescue Regexp::Scanner::ScannerError - return - end - + tree = Regexp::Parser.parse(node.content) @valid_ref = regexp_captures(tree) end 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 50a2ff73e74..c386b08b8c0 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 @@ -67,14 +67,6 @@ RUBY end - # See https://github.com/rubocop-hq/rubocop/issues/8083 - it 'does not register offense when using a Regexp cannot be processed by regexp_parser gem' do - expect_no_offenses(<<~'RUBY') - /data = ({"words":.+}}}[^}]*})/m - puts $1 - RUBY - end - # RuboCop does not know a value of variables that it will contain in the regexp literal. # For example, `/(?#{var}*)` is interpreted as `/(?*)`. # So it does not offense when variables are used in regexp literals.