Skip to content

Commit

Permalink
[Fix #10148] Fix Style/QuotedSymbols handling escaped characters in…
Browse files Browse the repository at this point in the history
…correctly.

Two cases are improved here:
1. Escaped double quotes (`\"`) within a double-quoted string are handled correctly (replaced with an unescaped double quote) when converting to a single-quoted string.
2. Escaped backslashes (`\\`) are treated properly and not doubled for both single- and double-quoted strings.
  • Loading branch information
dvandersluis authored and bbatsov committed Oct 2, 2021
1 parent eba1d4d commit a7ef72a
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 7 deletions.
1 change: 1 addition & 0 deletions changelog/fix_fix_stylequotedsymbols_handling_escaped.md
@@ -0,0 +1 @@
* [#10148](https://github.com/rubocop/rubocop/issues/10148): Fix `Style/QuotedSymbols` handling escaped characters incorrectly. ([@dvandersluis][])
6 changes: 5 additions & 1 deletion lib/rubocop/cop/mixin/string_literals_help.rb
Expand Up @@ -13,7 +13,11 @@ def wrong_quotes?(src_or_node)
if style == :single_quotes
!double_quotes_required?(src)
else
!/" | \\[^'\\] | \#[@{$]/x.match?(src)
# The string needs single quotes if:
# 1. It contains a double quote
# 2. It contains text that would become an escape sequence with double quotes
# 3. It contains text that would become an interpolation with double quotes
!/" | (?<!\\)\\[abcefMnrtuUx0-7] | \#[@{$]/x.match?(src)

This comment has been minimized.

Copy link
@Bo98

Bo98 Oct 5, 2021

This is behaviour change that confused me for a while.

'\s' for example is now flagged with the message "Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping." despite needing extra backslashes for escaping when in a double-quoted string ("\\s").

This comment has been minimized.

Copy link
@koic

koic Oct 5, 2021

Member

I think that is a regression. I've opened #10166 to fix it.

end
end
end
Expand Down
13 changes: 8 additions & 5 deletions lib/rubocop/cop/style/quoted_symbols.rb
Expand Up @@ -76,11 +76,14 @@ def hash_colon_key?(node)
end

def correct_quotes(str)
if style == :single_quotes
to_string_literal(str)
else
str.inspect
end
correction = if style == :single_quotes
to_string_literal(str)
else
str.gsub("\\'", "'").inspect
end

# The conversion process doubles escaped slashes, so they have to be reverted
correction.gsub('\\\\', '\\')
end

def style
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/util.rb
Expand Up @@ -113,7 +113,8 @@ def to_string_literal(string)
if needs_escaping?(string) && compatible_external_encoding_for?(string)
string.inspect
else
"'#{string.gsub('\\') { '\\\\' }}'"
# In a single-quoted strings, double quotes don't need to be escaped
"'#{string.gsub('\"', '"').gsub('\\') { '\\\\' }}'"
end
end

Expand Down
56 changes: 56 additions & 0 deletions spec/rubocop/cop/style/quoted_symbols_spec.rb
Expand Up @@ -113,6 +113,34 @@
RUBY
end

it 'registers an offense and corrects for an escaped quote within double quotes' do
expect_offense(<<~'RUBY')
:"my\"quote"
^^^^^^^^^^^^ Prefer single-quoted symbols when you don't need string interpolation or special symbols.
RUBY

expect_correction(<<~RUBY)
:'my"quote'
RUBY
end

it 'registers an offense and corrects escape characters properly' do
expect_offense(<<~'RUBY')
:"foo\\bar"
^^^^^^^^^^^ Prefer single-quoted symbols when you don't need string interpolation or special symbols.
RUBY

expect_correction(<<~'RUBY')
:'foo\\bar'
RUBY
end

it 'accepts single quoted symbol with an escaped quote' do
expect_no_offenses(<<~'RUBY')
:'o\'clock'
RUBY
end

context 'hash with hashrocket style' do
it 'accepts properly quoted symbols' do
expect_no_offenses(<<~RUBY)
Expand Down Expand Up @@ -213,6 +241,34 @@
RUBY
end

it 'registers an offense and corrects for an escaped quote within single quotes' do
expect_offense(<<~'RUBY')
:'o\'clock'
^^^^^^^^^^^ Prefer double-quoted symbols unless you need single quotes to avoid extra backslashes for escaping.
RUBY

expect_correction(<<~'RUBY')
:"o'clock"
RUBY
end

it 'registers an offense and corrects escape characters properly' do
expect_offense(<<~'RUBY')
:'foo\\bar'
^^^^^^^^^^^ Prefer double-quoted symbols unless you need single quotes to avoid extra backslashes for escaping.
RUBY

expect_correction(<<~'RUBY')
:"foo\\bar"
RUBY
end

it 'accepts double quoted symbol with an escaped quote' do
expect_no_offenses(<<~'RUBY')
:"my\"quote"
RUBY
end

context 'hash with hashrocket style' do
it 'accepts properly quoted symbols' do
expect_no_offenses(<<~RUBY)
Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/style/string_literals_spec.rb
Expand Up @@ -157,6 +157,17 @@
RUBY
end

it 'registers an offense for "\\"' do
expect_offense(<<~'RUBY')
"\\"
^^^^ Prefer single-quoted strings when you don't need string interpolation or special symbols.
RUBY

expect_correction(<<~'RUBY')
'\\'
RUBY
end

it 'registers an offense for words with non-ascii chars' do
expect_offense(<<~RUBY)
"España"
Expand Down

0 comments on commit a7ef72a

Please sign in to comment.