[Fix rubocop#3300] Do not replace %q()s containing escaped non-back…

…slashes (rubocop#3315)
Neodelf · Oct 15, 2016 · 91f9757 · 91f9757
1 parent 5d26c82
commit 91f9757
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,6 +41,7 @@
 * [#3272](https://github.com/bbatsov/rubocop/issues/3272): Add escape character missing to LITERAL_REGEX. ([@pocke][])
 * [#3255](https://github.com/bbatsov/rubocop/issues/3255): Fix auto-correct for `Style/RaiseArgs` when constructing exception without arguments. ([@drenmi][])
 * [#3294](https://github.com/bbatsov/rubocop/pull/3294): Allow to use `Time.zone_default`. ([@Tei][])
+* [#3300](https://github.com/bbatsov/rubocop/issues/3300): Do not replace `%q()`s containing escaped non-backslashes. ([@owst][])
 
 ### Changes
 

diff --git a/lib/rubocop/cop/style/unneeded_percent_q.rb b/lib/rubocop/cop/style/unneeded_percent_q.rb
@@ -16,6 +16,7 @@ class UnneededPercentQ < Cop
         PERCENT_Q = '%q'.freeze
         PERCENT_CAPITAL_Q = '%Q'.freeze
         STRING_INTERPOLATION_REGEXP = /#\{.+}/
+        ESCAPED_NON_BACKSLASH = /\\[^\\]/
 
         def on_dstr(node)
           check(node)
@@ -35,12 +36,9 @@ def check(node)
           src = node.source
           return unless start_with_percent_q_variant?(src)
           return if src.include?(SINGLE_QUOTE) && src.include?(QUOTE)
-          if src.start_with?(PERCENT_Q) && src =~ STRING_INTERPOLATION_REGEXP
-            return
-          end
-          if src.start_with?(PERCENT_CAPITAL_Q) && acceptable_capital_q?(node)
-            return
-          end
+          return if src.start_with?(PERCENT_Q) && acceptable_q?(node)
+          return if src.start_with?(PERCENT_CAPITAL_Q) &&
+                    acceptable_capital_q?(node)
 
           add_offense(node, :expression)
         end
@@ -73,6 +71,14 @@ def start_with_percent_q_variant?(string)
           string.start_with?(PERCENT_Q, PERCENT_CAPITAL_Q)
         end
 
+        def acceptable_q?(node)
+          src = node.source
+
+          return true if src =~ STRING_INTERPOLATION_REGEXP
+
+          src.scan(/\\./).any? { |s| s =~ ESCAPED_NON_BACKSLASH }
+        end
+
         def acceptable_capital_q?(node)
           src = node.source
           src.include?(QUOTE) &&

diff --git a/spec/rubocop/cop/style/unneeded_percent_q_spec.rb b/spec/rubocop/cop/style/unneeded_percent_q_spec.rb
@@ -34,24 +34,22 @@
       expect(cop.messages).to be_empty
     end
 
-    it 'registers an offense for a string with single quotes and what appears' \
-       ' to be an escape' do
-      # There is no such thing as escapes in a %q() string
-      # So this can just as well be written with double quotes
-      inspect_source(cop, "%q('hi\\t')")
+    it 'registers an offfense for a string containing escaped backslashes' do
+      inspect_source(cop, '%q(\\\\foo\\\\)')
 
-      expect(cop.messages).to eq(['Use `%q` only for strings that contain ' \
-                                  'both single quotes and double quotes.'])
+      expect(cop.messages.length).to eq 1
     end
 
-    it 'registers an offense for a string with double quotes and what appears' \
-       ' to be an escape' do
-      # There is no such thing as escapes in a %q() string
-      # So this can just as well be written with single quotes
-      inspect_source(cop, '%q("hi\\t")')
+    it 'accepts a string with escaped non-backslash characters' do
+      inspect_source(cop, "%q(\\'foo\\')")
 
-      expect(cop.messages).to eq(['Use `%q` only for strings that contain ' \
-                                  'both single quotes and double quotes.'])
+      expect(cop.messages).to be_empty
+    end
+
+    it 'accepts a string with escaped backslash and non-backslash characters' do
+      inspect_source(cop, "%q(\\\\ \\'foo\\' \\\\)") # This is \\ \'foo\' \\
+
+      expect(cop.messages).to be_empty
     end
 
     it 'accepts regular expressions starting with %q' do