Incorrect auto-correction for Style/StringLiterals cop

[konieczkow]

Hi there, thanks to all the people working on Rubocop.

I noticed a weird behaviour when applying an autocorrection to the following code.

a = {
  b: "{\"[\\\"*\\\"]\""
}

This gets correctly marked as violating Style/StringLiterals but when I run rubocop with -a (which I assume means it will correct all the cops as long as it is safe to do so) I get this as the output:

a = {
  b: '{"["*"]"'
}

It seems to me that the autocorrection is too greedy and swollows too many escape characters?

---

Expected behavior

I would expect rubocop to either correctly escape the code to:

a = {
  b: {"[\"*\"]"
}

or it should say that the cop is unsafe and can be used with -A, instead of -a.

Actual behavior

Autocorrecting:

a = {
  b: "{\"[\\\"*\\\"]\""
}

produces:

a = {
  b: '{"["*"]"'
}

Steps to reproduce the problem

Create an empty ruby file, add the following piece of code to it:

a = {
  b: "{\"[\\\"*\\\"]\""
}

run:

rubocop -a [name-of-the-file]

check the file again.

RuboCop version

$ rubocop -V
1.37.1 (using Parser 3.1.2.1, rubocop-ast 1.23.0, running on ruby 2.7.6) [x86_64-linux]

[konieczkow]

Had a look at the source code and it seems to me that the problem is somewhere here:

rubocop/lib/rubocop/cop/util.rb

Line 125 in e2f5f20

"'#{string.gsub('\"', '"').gsub('\\') { '\\\\' }}'"

Switching the order of the gsubs fixes my issue:

original_string = "{\"[\\\"*\\\"]\"".freeze

irb(main):038:0> original_string.gsub('\"', '"').gsub('\\') { '\\\\' }
=> "{\"[\"*\"]\""
irb(main):039:0> original_string.gsub('\\') { '\\\\' }.gsub('\"', '"')
=> "{\"[\\\"*\\\"]\""

but not sure if it doesn't break anything else. Will clone the project and run the test suite to see.

[konieczkow]

Running the specs - this breaks a few examples in spec/rubocop/cop/style/quoted_symbols_spec.rb.

[tdeo]

@konieczkow if you want to open a PR for this it seems to me the issue is actually that QuotedSymbols calls to_string_literal not with the intended argument style.

Indeed, StringLiterals through StringLiteralCorrector calls it with the result of DStrNode.str_content, which takes care of handling some escaping:

rubocop/lib/rubocop/cop/correctors/string_literal_corrector.rb

Lines 13 to 15 in 7007fb1

	str = node.str_content
	if style == :single_quotes
	corrector.replace(node, to_string_literal(str))

While QuotedSymbols calls it directly with the source of the node which retains escape-backslashes as characters:

rubocop/lib/rubocop/cop/style/quoted_symbols.rb

Lines 72 to 77 in 7007fb1

	str = if hash_colon_key?(node)
	# strip quotes
	correct_quotes(node.source[1..-2])
	else
	# strip leading `:` and quotes
	":#{correct_quotes(node.source[2..-2])}"

It seems that on top of your fix, changing those lines to using eval to replicate the behavior of str_content actually makes the tests pass for Style/QuotedSymbols:

# Using eval to replicate the behavior of `#str_content`
str = if hash_colon_key?(node)
                  correct_quotes(eval(node.source))
                else
                  # strip leading `:`
                  ":#{correct_quotes(eval(node.source[1..]))}"
                end

But I'm not sure whether using eval here would be accepted - maybe the proper solution is to implement str_content on SymNode that behaves similarly as for DstrNode. Maybe a rubocop maintainer could point in the preferred direction for implementation?