Skip to content

Commit

Permalink
[Fix #10439] Add Style/RedundantStringEscape (#11002)
Browse files Browse the repository at this point in the history
  • Loading branch information
owst committed Sep 29, 2022
1 parent c97046f commit 378eea5
Show file tree
Hide file tree
Showing 12 changed files with 580 additions and 31 deletions.
1 change: 1 addition & 0 deletions changelog/new_add_styleredundantstringescape.md
@@ -0,0 +1 @@
* [#10439](https://github.com/rubocop/rubocop/issues/10439): Add new `Style/RedundantStringEscape` cop. ([@owst][])
5 changes: 5 additions & 0 deletions config/default.yml
Expand Up @@ -4794,6 +4794,11 @@ Style/RedundantSortBy:
Enabled: true
VersionAdded: '0.36'

Style/RedundantStringEscape:
Description: 'Checks for redundant escapes in string literals.'
Enabled: pending
VersionAdded: '<<next>>'

Style/RegexpLiteral:
Description: 'Use / or %r around regular expressions.'
StyleGuide: '#percent-r'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop.rb
Expand Up @@ -612,6 +612,7 @@
require_relative 'rubocop/cop/style/redundant_self'
require_relative 'rubocop/cop/style/redundant_sort'
require_relative 'rubocop/cop/style/redundant_sort_by'
require_relative 'rubocop/cop/style/redundant_string_escape'
require_relative 'rubocop/cop/style/regexp_literal'
require_relative 'rubocop/cop/style/rescue_modifier'
require_relative 'rubocop/cop/style/rescue_standard_error'
Expand Down
173 changes: 173 additions & 0 deletions lib/rubocop/cop/style/redundant_string_escape.rb
@@ -0,0 +1,173 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Style
# Checks for redundant escapes in string literals.
#
# @example
# # bad - no need to escape # without following {/$/@
# "\#foo"
#
# # bad - no need to escape single quotes inside double quoted string
# "\'foo\'"
#
# # bad - heredocs are also checked for unnecessary escapes
# <<~STR
# \#foo \"foo\"
# STR
#
# # good
# "#foo"
#
# # good
# "\#{no_interpolation}"
#
# # good
# "'foo'"
#
# # good
# "foo\
# bar"
#
# # good
# <<~STR
# #foo "foo"
# STR
class RedundantStringEscape < Base
include MatchRange
include RangeHelp
extend AutoCorrector

MSG = 'Redundant escape of %<char>s inside string literal.'

def on_str(node)
return if node.parent&.regexp_type? || node.parent&.xstr_type?

str_contents_range = str_contents_range(node)
return unless str_contents_range.source.include?('\\')

each_match_range(str_contents_range, /(\\.)/) do |range|
next if allowed_escape?(node, range.resize(3))

add_offense(range) do |corrector|
corrector.remove_leading(range, 1)
end
end
end

private

def message(range)
format(MSG, char: range.source.chars.last)
end

def str_contents_range(node)
if heredoc?(node)
node.loc.heredoc_body
elsif begin_loc_present?(node)
contents_range(node)
else
node.loc.expression
end
end

def begin_loc_present?(node)
# e.g. a __FILE__ literal has no begin loc so we can't query if it's nil
node.loc.to_hash.key?(:begin) && !node.loc.begin.nil?
end

def allowed_escape?(node, range)
escaped = range.source[(1..-1)]

# Inside a single-quoted string, escapes (except \\ and \') do not have special meaning,
# and so are not redundant, as they are a literal backslash.
return true if interpolation_not_enabled?(node)

# Strictly speaking a few single-letter chars are currently unnecessary to "escape", e.g.
# d, but enumerating them is rather difficult, and their behavior could change over time
# with different versions of Ruby so that e.g. /\d/ != /d/
return true if /[\n\\[[:alnum:]]]/.match?(escaped[0])

return true if escaped[0] == ' ' && percent_array_literal?(node)

# Allow #{foo}, #$foo, #@foo, and #@@foo for escaping local, global, instance and class
# variable interpolations inside
return true if /\A#[{$@]/.match?(escaped)
return true if delimiter?(node, escaped[0])

false
end

def interpolation_not_enabled?(node)
single_quoted?(node) ||
percent_w_literal?(node) ||
percent_q_literal?(node) ||
heredoc_with_disabled_interpolation?(node)
end

def single_quoted?(node)
delimiter?(node, "'")
end

def percent_q_literal?(node)
if literal_in_interpolated_or_multiline_string?(node)
percent_q_literal?(node.parent)
else
node.source.start_with?('%q')
end
end

def array_literal?(node, prefix)
if literal_in_interpolated_or_multiline_string?(node)
array_literal?(node.parent, prefix)
else
node.parent&.array_type? && node.parent.source.start_with?(prefix)
end
end

def percent_w_literal?(node)
array_literal?(node, '%w')
end

def percent_w_upper_literal?(node)
array_literal?(node, '%W')
end

def percent_array_literal?(node)
(percent_w_literal?(node) || percent_w_upper_literal?(node))
end

def heredoc_with_disabled_interpolation?(node)
if heredoc?(node)
node.loc.expression.source.end_with?("'")
elsif node.parent&.dstr_type?
heredoc_with_disabled_interpolation?(node.parent)
else
false
end
end

def heredoc?(node)
(node.str_type? || node.dstr_type?) && node.heredoc?
end

def delimiter?(node, char)
return false if heredoc?(node)

if literal_in_interpolated_or_multiline_string?(node) || percent_array_literal?(node)
return delimiter?(node.parent, char)
end

delimiters = [node.loc.begin.source[-1], node.loc.end.source[0]]

delimiters.include?(char)
end

def literal_in_interpolated_or_multiline_string?(node)
node.str_type? && !begin_loc_present?(node) && node.parent&.dstr_type?
end
end
end
end
end
6 changes: 3 additions & 3 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -986,7 +986,7 @@ def method
require 'spec_helper'
describe ArticlesController do
render_views
describe "GET \'index\'" do
describe "GET 'index'" do
it "returns http success" do
end
describe "admin user" do
Expand All @@ -1004,7 +1004,7 @@ def method
require 'spec_helper'
describe ArticlesController do
render_views
describe \"GET 'index'\" do
describe "GET 'index'" do
it 'returns http success' do
end
describe 'admin user' do
Expand Down Expand Up @@ -1670,7 +1670,7 @@ def self.some_method(foo, bar: 1)
expect(File.read('example.rb')).to eq(<<~RUBY)
# frozen_string_literal: true
puts \"Hello\", 123_456
puts "Hello", 123_456
RUBY
end

Expand Down
8 changes: 4 additions & 4 deletions spec/rubocop/cop/layout/indentation_style_spec.rb
Expand Up @@ -80,12 +80,12 @@

it 'registers and corrects an offense for a line with tab in a string indented with tab' do
expect_offense(<<~RUBY)
\t(x = \"\t\")
\t(x = "\t")
^ Tab detected in indentation.
RUBY

expect_correction(<<-RUBY.strip_margin('|'))
| (x = \"\t\")
| (x = "\t")
RUBY
end

Expand Down Expand Up @@ -184,12 +184,12 @@

it 'registers and corrects an offense for a line with tab in a string indented with space' do
expect_offense(<<~RUBY)
(x = \"\t\")
(x = "\t")
^^ Space detected in indentation.
RUBY

expect_correction(<<~RUBY)
\t(x = \"\t\")
\t(x = "\t")
RUBY
end

Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cop/layout/line_length_spec.rb
Expand Up @@ -387,8 +387,8 @@ def method_definition_that_is_just_under_the_line_length_limit(foo) # rubocop:di
context 'and the source contains non-directive #s as non-comment' do
it 'registers an offense for the line' do
expect_offense(<<-RUBY)
LARGE_DATA_STRING_PATTERN = %r{\A([A-Za-z0-9\+\/#]*\={0,2})#([A-Za-z0-9\+\/#]*\={0,2})#([A-Za-z0-9\+\/#]*\={0,2})\z} # rubocop:disable Style/ClassVars
#{' ' * 68}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Line is too long. [117/80]
LARGE_DATA_STRING_PATTERN = %r{\\A([A-Za-z0-9+/#]*={0,2})#([A-Za-z0-9+/#]*={0,2})#([A-Za-z0-9+/#]*={0,2})\\z} # rubocop:disable Style/ClassVars
#{' ' * 68}^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Line is too long. [119/80]
RUBY
end
end
Expand Down
10 changes: 5 additions & 5 deletions spec/rubocop/cop/lint/literal_in_interpolation_spec.rb
Expand Up @@ -120,25 +120,25 @@

it "accepts interpolation of a string literal with space in #{prefix}[]" do
expect_no_offenses(<<~RUBY)
#{prefix}[\#{\"this interpolation\"} is significant]
#{prefix}[\#{"this interpolation"} is significant]
RUBY
end

it "accepts interpolation of a symbol literal with space in #{prefix}[]" do
expect_no_offenses(<<~RUBY)
#{prefix}[\#{:\"this interpolation\"} is significant]
#{prefix}[\#{:"this interpolation"} is significant]
RUBY
end

it "accepts interpolation of an array literal containing a string with space in #{prefix}[]" do
expect_no_offenses(<<~RUBY)
#{prefix}[\#{[\"this interpolation\"]} is significant]
#{prefix}[\#{["this interpolation"]} is significant]
RUBY
end

it "accepts interpolation of an array literal containing a symbol with space in #{prefix}[]" do
expect_no_offenses(<<~RUBY)
#{prefix}[\#{[:\"this interpolation\"]} is significant]
#{prefix}[\#{[:"this interpolation"]} is significant]
RUBY
end

Expand Down Expand Up @@ -320,7 +320,7 @@
RUBY

expect_correction(<<~RUBY)
\`this is the #{expected}\`
`this is the #{expected}`
RUBY
end

Expand Down

0 comments on commit 378eea5

Please sign in to comment.