Merge pull request #3189 from deivid-rodriguez/string_literals_for_no…

…n_ascii_strings

[Fix #3017][Fix #3056] `Style/StringLiterals` now works with non-ascii strings

CHANGELOG.md

@@ -43,6 +43,8 @@
 * [#3217](https://github.com/bbatsov/rubocop/pull/3217): Fix output of ellipses for multi-line offense ranges in HTML formatter. ([@jonas054][])
 * [#3207](https://github.com/bbatsov/rubocop/issues/3207): Accept modifier forms of `while`/`until` in `Style/InfiniteLoop`. ([@jonas054][])
 * [#3202](https://github.com/bbatsov/rubocop/issues/3202): Fix `Style/EmptyElse` registering wrong offenses and thus making RuboCop crash. ([@deivid-rodriguez][])
+* [#3017](https://github.com/bbatsov/rubocop/issues/3017): Fix `Style/StringLiterals` to register offenses on non-ascii strings. ([@deivid-rodriguez][])
+* [#3056](https://github.com/bbatsov/rubocop/issues/3056): Fix `Style/StringLiterals` to register offenses on non-ascii strings. ([@deivid-rodriguez][])
 
 ### Changes
 
@@ -122,7 +124,7 @@
 
 ### Bug fixes
 
-* [#2948](https://github.com/bbatsov/rubocop/issues/2948): `Style/SpaceAroundKeyword` should allow yield[n] and super[n]. ([@laurelfan][])
+* [#2948](https://github.com/bbatsov/rubocop/issues/2948): `Style/SpaceAroundKeyword` should allow `yield[n]` and `super[n]`. ([@laurelfan][])
 * [#2950](https://github.com/bbatsov/rubocop/issues/2950): Fix auto-correcting cases in which precedence has changed in `Style/OneLineConditional`. ([@lumeet][])
 * [#2947](https://github.com/bbatsov/rubocop/issues/2947): Fix auto-correcting `if-then` in `Style/Next`. ([@lumeet][])
 * [#2904](https://github.com/bbatsov/rubocop/issues/2904): `Style/RedundantParentheses` doesn't flag `-(1.method)` or `+(1.method)`, since removing the parentheses would change the meaning of these expressions. ([@alexdowad][])
@@ -154,7 +156,7 @@
 
 ### Bug fixes
 
-* Add `require 'time'` to `remote_config.rb` to avoid "undefined method `rfc2822'". ([@necojackarc][])
+* Add `require 'time'` to `remote_config.rb` to avoid "undefined method \`rfc2822'". ([@necojackarc][])
 * Replace `Rake::TaskManager#last_comment` with `Rake::TaskManager#last_description` for Rake 11 compatibility. ([@tbrisker][])
 * Fix false positive in `Style/TrailingCommaInArguments` & `Style/TrailingCommaInLiteral` cops with consistent_comma style. ([@meganemura][])
 * [#2861](https://github.com/bbatsov/rubocop/pull/2861): Fix false positive in `Style/SpaceAroundKeyword` for `rescue(...`. ([@rrosenblum][])
@@ -386,7 +388,7 @@
 * [#2436](https://github.com/bbatsov/rubocop/issues/2436): `Style/SpaceInsideHashLiteralBraces` doesn't mangle a hash literal which is not surrounded by curly braces, but has another hash literal which does as its first key. ([@alexdowad][])
 * [#2483](https://github.com/bbatsov/rubocop/issues/2483): `Style/Attr` differentiate between attr_accessor and attr_reader. ([@weh][])
 * `Style/ConditionalAssignment` doesn't crash if it finds a `case` with an empty branch. ([@lumeet][])
-* [#2506](https://github.com/bbatsov/rubocop/issues/2506): `Lint/FormatParameterMismatch` understands %{} and %<> interpolations. ([@alexdowad][])
+* [#2506](https://github.com/bbatsov/rubocop/issues/2506): `Lint/FormatParameterMismatch` understands `%{}` and `%<>` interpolations. ([@alexdowad][])
 * [#2145](https://github.com/bbatsov/rubocop/issues/2145): `Lint/ParenthesesAsGroupedExpression` ignores calls with multiple arguments, since they are not ambiguous. ([@alexdowad][])
 * [#2484](https://github.com/bbatsov/rubocop/issues/2484): Remove two vulnerabilities in cache handling. ([@jonas054][])
 * [#2517](https://github.com/bbatsov/rubocop/issues/2517): `Lint/UselessAccessModifier` doesn't think that an access modifier applied to `attr_writer` is useless. ([@alexdowad][])

lib/rubocop/cop/mixin/string_literals_help.rb

@@ -11,7 +11,7 @@ def wrong_quotes?(node)
         src = node.source
         return false if src.start_with?('%', '?')
         if style == :single_quotes
-          src !~ /'/ && !double_quotes_acceptable?(node.str_content)
+          !double_quotes_required?(src)
         else
           src !~ /" | \\ | \#(@|\{)/x
         end

lib/rubocop/cop/style/symbol_array.rb

@@ -63,7 +63,7 @@ def symbols_contain_spaces?(node)
         def autocorrect(node)
           syms = node.children.map { |c| c.children[0].to_s }
           corrected = if style == :percent
-                        escape = syms.any? { |s| double_quotes_required?(s) }
+                        escape = syms.any? { |s| needs_escaping?(s) }
                         syms = syms.map { |s| escape_string(s) } if escape
                         syms = syms.map { |s| s.gsub(/\)/, '\\)') }
                         if escape
@@ -80,10 +80,6 @@ def autocorrect(node)
             corrector.replace(node.source_range, corrected)
           end
         end
-
-        def escape_string(string)
-          string.inspect[1..-2].tap { |s| s.gsub!(/\\"/, '"') }
-        end
       end
     end
   end

lib/rubocop/cop/style/word_array.rb

@@ -27,7 +27,7 @@ def on_array(node)
         def autocorrect(node)
           words = node.children
           if style == :percent
-            escape = words.any? { |w| double_quotes_required?(w.children[0]) }
+            escape = words.any? { |w| needs_escaping?(w.children[0]) }
             char = escape ? 'W' : 'w'
             contents = autocorrect_words(words, escape, node.loc.line)
             lambda do |corrector|
@@ -107,10 +107,6 @@ def autocorrect_words(word_nodes, escape, base_line_number)
           end.join(' ')
         end
 
-        def escape_string(string)
-          string.inspect[1..-2].tap { |s| s.gsub!(/\\"/, '"') }
-        end
-
         def style_detected(style, ary_size)
           cfg = config_to_allow_offenses
           return if cfg['Enabled'] == false

lib/rubocop/cop/util.rb

@@ -212,32 +212,36 @@ def double_quotes_required?(string)
 
         # Regex matches IF there is a ' or there is a \\ in the string that is
         # not preceded/followed by another \\ (e.g. "\\x34") but not "\\\\".
-        string.inspect =~ /'|(?<! \\) \\{2}* \\ (?![\\"])/x
+        string =~ /'|(?<! \\) \\{2}* \\ (?![\\"])/x
       end
 
       # If double quoted string literals are found in Ruby code, and they are
       # not the preferred style, should they be flagged?
       def double_quotes_acceptable?(string)
         # If a string literal contains hard-to-type characters which would
         # not appear on a "normal" keyboard, then double-quotes are acceptable
-        double_quotes_required?(string) ||
+        needs_escaping?(string) ||
           string.codepoints.any? { |cp| cp < 32 || cp > 126 }
       end
 
+      def needs_escaping?(string)
+        double_quotes_required?(escape_string(string))
+      end
+
+      def escape_string(string)
+        string.inspect[1..-2].tap { |s| s.gsub!(/\\"/, '"') }
+      end
+
       def to_string_literal(string)
-        if double_quotes_required?(string)
+        if needs_escaping?(string)
           string.inspect
         else
           "'#{string.gsub('\\') { '\\\\' }}'"
         end
       end
 
       def to_symbol_literal(string)
-        if string =~ /\s/ || double_quotes_required?(string)
-          ":#{to_string_literal(string)}"
-        else
-          ":#{string}"
-        end
+        ":#{string.to_sym}"
       end
 
       def interpret_string_escapes(string)

spec/rubocop/cop/style/string_literals_spec.rb

@@ -63,16 +63,44 @@
       expect(cop.offenses).to be_empty
     end
 
-    it 'accepts double quotes when they are needed' do
-      src = ['a = "\n"',
-             'b = "#{encode_severity}:' \
-             '#{sprintf(\'%3d\', line_number)}: #{m}"',
-             'c = "\'"',
-             'd = "#@test"',
-             'e = "#$test"',
-             'f = "\e"',
-             'g = "#@@test"']
-      inspect_source(cop, src)
+    it 'accepts double quotes when new line is used' do
+      inspect_source(cop, '"\n"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when interpolating & quotes in multiple lines' do
+      inspect_source(cop, '"#{encode_severity}:' \
+                          '#{sprintf(\'%3d\', line_number)}: #{m}"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when single quotes are used' do
+      inspect_source(cop, '"\'"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when interpolating an instance variable' do
+      inspect_source(cop, '"#@test"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when interpolating a global variable' do
+      inspect_source(cop, '"#$test"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when interpolating a class variable' do
+      inspect_source(cop, '"#@@test"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when control characters are used' do
+      inspect_source(cop, '"\e"')
+      expect(cop.offenses).to be_empty
+    end
+
+    it 'accepts double quotes when unicode control sequence is used' do
+      inspect_source(cop, '"Espa\u00f1a"')
       expect(cop.offenses).to be_empty
     end
 
@@ -139,14 +167,14 @@
                                   'special symbols.'])
     end
 
-    it 'registers an offense for hello... using hex escapes' do
-      inspect_source(cop, '"\\x68\\x65\\x6c\\x6c\\x6f"')
+    it 'registers an offense for words with non-ascii chars' do
+      inspect_source(cop, '"España"')
       expect(cop.offenses.size).to eq(1)
     end
 
-    it 'autocorrects hello... using hex escapes' do
-      new_source = autocorrect_source(cop, '"\\x68\\x65\\x6c\\x6c\\x6f"')
-      expect(new_source).to eq("'hello'")
+    it 'autocorrects words with non-ascii chars' do
+      new_source = autocorrect_source(cop, '"España"')
+      expect(new_source).to eq("'España'")
     end
   end