From 5de911893a469a7c867a7787dcab479e8c6a00f8 Mon Sep 17 00:00:00 2001
From: Owen Stephens <owen@owenstephens.co.uk>
Date: Wed, 10 Jun 2020 19:26:10 +0100
Subject: [PATCH] [Fix #8098]: Fix false-positive in
 Style/RedundantRegexpCharacterClass

Also correct the handling of interpolations and comments - we need to
preserve their width in the checked pattern source without their
contents triggering the cop in question, so that we can correctly
highlight following offenses based on the offset in the pattern_source.
---
 CHANGELOG.md                                  |  1 +
 lib/rubocop/cop/mixin/regexp_literal_help.rb  | 27 +++++++++
 .../style/redundant_regexp_character_class.rb |  4 +-
 .../cop/style/redundant_regexp_escape.rb      | 15 -----
 .../redundant_regexp_character_class_spec.rb  | 60 ++++++++++++++++---
 .../cop/style/redundant_regexp_escape_spec.rb | 38 ++++++++++++
 6 files changed, 121 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 166f0e55c92..a294b44599c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@
 * [#8115](https://github.com/rubocop-hq/rubocop/issues/8115): Fix false negative for `Lint::FormatParameterMismatch` when argument contains formatting. ([@andrykonchin][])
 * [#8131](https://github.com/rubocop-hq/rubocop/pull/8131): Fix false positive for `Style/RedundantRegexpEscape` with escaped delimiters. ([@owst][])
 * [#8124](https://github.com/rubocop-hq/rubocop/issues/8124): Fix a false positive for `Lint/FormatParameterMismatch` when using named parameters with escaped `%`. ([@koic][])
+* [#8098](https://github.com/rubocop-hq/rubocop/issues/8098): Fix a false positive for `Style/RedundantRegexpCharacterClass` when using interpolations. ([@owst][])
 
 ## 0.85.1 (2020-06-07)
 
diff --git a/lib/rubocop/cop/mixin/regexp_literal_help.rb b/lib/rubocop/cop/mixin/regexp_literal_help.rb
index e28f8671c56..bba68606e9a 100644
--- a/lib/rubocop/cop/mixin/regexp_literal_help.rb
+++ b/lib/rubocop/cop/mixin/regexp_literal_help.rb
@@ -11,6 +11,33 @@ def freespace_mode_regexp?(node)
 
         regopt.children.include?(:x)
       end
+
+      def pattern_source(node)
+        freespace_mode = freespace_mode_regexp?(node)
+
+        node.children.reject(&:regopt_type?).map do |child|
+          source_with_comments_and_interpolations_blanked(child, freespace_mode)
+        end.join
+      end
+
+      def source_with_comments_and_interpolations_blanked(child, freespace_mode)
+        source = child.source
+
+        # We don't want to consider the contents of interpolations or free-space mode comments as
+        # part of the pattern source, but need to preserve their width, to allow offsets to
+        # correctly line up with the original source: spaces have no effect, and preserve width.
+        if child.begin_type?
+          replace_match_with_spaces(source, /.*/) # replace all content
+        elsif freespace_mode
+          replace_match_with_spaces(source, /(?<!\\)#.*/) # replace any comments
+        else
+          source
+        end
+      end
+
+      def replace_match_with_spaces(source, pattern)
+        source.sub(pattern) { ' ' * Regexp.last_match[0].length }
+      end
     end
   end
 end
diff --git a/lib/rubocop/cop/style/redundant_regexp_character_class.rb b/lib/rubocop/cop/style/redundant_regexp_character_class.rb
index 24935d7de94..de0f08b5c83 100644
--- a/lib/rubocop/cop/style/redundant_regexp_character_class.rb
+++ b/lib/rubocop/cop/style/redundant_regexp_character_class.rb
@@ -67,8 +67,8 @@ def autocorrect(node)
         end
 
         def each_redundant_character_class(node)
-          each_match_range(node.loc.expression, PATTERN) do |loc|
-            yield loc
+          pattern_source(node).scan(PATTERN) do
+            yield match_range(node.loc.begin.end, Regexp.last_match)
           end
         end
 
diff --git a/lib/rubocop/cop/style/redundant_regexp_escape.rb b/lib/rubocop/cop/style/redundant_regexp_escape.rb
index 8ee6ade144f..4be9f78f92b 100644
--- a/lib/rubocop/cop/style/redundant_regexp_escape.rb
+++ b/lib/rubocop/cop/style/redundant_regexp_escape.rb
@@ -115,21 +115,6 @@ def escape_range_at_index(node, index)
 
           range_between(start, start + 2)
         end
-
-        def pattern_source(node)
-          freespace_mode = freespace_mode_regexp?(node)
-
-          node.children.reject(&:regopt_type?).map do |child|
-            source = child.source
-
-            if freespace_mode
-              # Remove comments to avoid misleading results
-              source.sub(/(?<!\\)#.*/, '')
-            else
-              source
-            end
-          end.join
-        end
       end
     end
   end
diff --git a/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb b/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb
index d48f980e3c2..b79057b1d18 100644
--- a/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb
+++ b/spec/rubocop/cop/style/redundant_regexp_character_class_spec.rb
@@ -142,6 +142,19 @@
     end
   end
 
+  context 'with an interpolated unnecessary-character-class regexp' do
+    it 'registers an offense and corrects' do
+      expect_offense(<<~'RUBY')
+        foo = /a#{/[b]/}c/
+                   ^^^ Redundant single-element character class, `[b]` can be replaced with `b`.
+      RUBY
+
+      expect_correction(<<~'RUBY')
+        foo = /a#{/b/}c/
+      RUBY
+    end
+  end
+
   context 'with a negated character class with a single element' do
     it 'does not register an offense' do
       expect_no_offenses('foo = /[^x]/')
@@ -178,6 +191,12 @@
     end
   end
 
+  context 'with an array index inside an interpolation' do
+    it 'does not register an offense' do
+      expect_no_offenses('foo = /a#{b[0]}c/')
+    end
+  end
+
   context 'with a character class containing a space' do
     context 'when not using free-spaced mode' do
       it 'registers an offense and corrects' do
@@ -192,17 +211,44 @@
       end
     end
 
-    context 'when using free-spaced mode' do
-      it 'does not register an offense with only /x' do
-        expect_no_offenses('foo = /[ ]/x')
+    context 'with an unnecessary-character-class after a comment' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~'RUBY')
+          foo = /
+            a # This comment shouldn't affect the position of the offense
+            [b]
+            ^^^ Redundant single-element character class, `[b]` can be replaced with `b`.
+          /x
+        RUBY
+
+        expect_correction(<<~'RUBY')
+          foo = /
+            a # This comment shouldn't affect the position of the offense
+            b
+          /x
+        RUBY
       end
+    end
 
-      it 'does not register an offense with /ix' do
-        expect_no_offenses('foo = /[ ]/ix')
+    context 'when using free-spaced mode' do
+      context 'with a commented single-element character class' do
+        it 'does not register an offense' do
+          expect_no_offenses('foo = /foo # [a]/x')
+        end
       end
 
-      it 'does not register an offense with /iux' do
-        expect_no_offenses('foo = /[ ]/iux')
+      context 'with a single space character class' do
+        it 'does not register an offense with only /x' do
+          expect_no_offenses('foo = /[ ]/x')
+        end
+
+        it 'does not register an offense with /ix' do
+          expect_no_offenses('foo = /[ ]/ix')
+        end
+
+        it 'does not register an offense with /iux' do
+          expect_no_offenses('foo = /[ ]/iux')
+        end
       end
     end
   end
diff --git a/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb b/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb
index 835ac863539..5a477164401 100644
--- a/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb
+++ b/spec/rubocop/cop/style/redundant_regexp_escape_spec.rb
@@ -91,6 +91,25 @@
       end
     end
 
+    context 'with an interpolated unnecessary-escape regexp' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~'RUBY')
+          foo = /a#{/\-/}c/
+                     ^^ Redundant escape inside regexp literal
+        RUBY
+
+        expect_correction(<<~'RUBY')
+          foo = /a#{/-/}c/
+        RUBY
+      end
+    end
+
+    context 'with an escape inside an interpolated string' do
+      it 'does not register an offense' do
+        expect_no_offenses('foo = /#{"\""}/')
+      end
+    end
+
     context 'with an escaped interpolation outside a character class' do
       it 'does not register an offense' do
         expect_no_offenses('foo = /\#\{[a-z_]+\}/')
@@ -361,6 +380,25 @@
         RUBY
       end
     end
+
+    context 'with a redundant escape after a line with comment' do
+      it 'registers an offense and corrects' do
+        expect_offense(<<~'RUBY')
+          foo = %r{
+            foo # this should not affect the position of the escape below
+            \-
+            ^^ Redundant escape inside regexp literal
+          }x
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo = %r{
+            foo # this should not affect the position of the escape below
+            -
+          }x
+        RUBY
+      end
+    end
   end
 
   context 'with a multi-line %r// regexp' do