Skip to content

Commit

Permalink
[Fix rubocop#2154] Fix auto-correct of StringReplacement with a backs…
Browse files Browse the repository at this point in the history
…lash
  • Loading branch information
rrosenblum committed Aug 20, 2015
1 parent 543679a commit 2abc2fc
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* [#2149](https://github.com/bbatsov/rubocop/issues/2149): Do not register an offense in `Rails/Date` when `Date#to_time` is called with a time zone argument. ([@maxjacobson][])
* Do not register a `Rails/TimeZone` offense when using Time.new safely. ([@maxjacobson][])
* [#2124](https://github.com/bbatsov/rubocop/issues/2124): Fix bug in `Style/EmptyLineBetweenDefs` when there are only comments between method definitions. ([@lumeet][])
* [#2154](https://github.com/bbatsov/rubocop/issues/2154): `Performance/StringReplacement` can auto-correct replacements with backslash in them. ([@rrosenblum][])

## 0.33.0 (05/08/2015)

Expand Down
72 changes: 45 additions & 27 deletions lib/rubocop/cop/performance/string_replacement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@ module Performance
# 'a b c'.delete(' ')
class StringReplacement < Cop
MSG = 'Use `%s` instead of `%s`.'
DETERMINISTIC_REGEX = /^[\w\s\-,."']+$/
REGEXP_CONSTRUCTOR_METHODS = [:new, :compile]
GSUB_METHODS = [:gsub, :gsub!]
DETERMINISTIC_TYPES = [:regexp, :str, :send]
DETERMINISTIC_REGEX = /^[\w\s\-,."']+$/.freeze
REGEXP_CONSTRUCTOR_METHODS = [:new, :compile].freeze
GSUB_METHODS = [:gsub, :gsub!].freeze
DETERMINISTIC_TYPES = [:regexp, :str, :send].freeze
DELETE = 'delete'.freeze
TR = 'tr'.freeze
BANG = '!'.freeze
SINGLE_QUOTE = "'".freeze
CLOSING_PAREN = ')'.freeze

def on_send(node)
_string, method, first_param, second_param = *node
Expand Down Expand Up @@ -51,19 +56,21 @@ def autocorrect(node)
_string, method, first_param, second_param = *node
first_source = first_source(first_param)
second_source, = *second_param
replacement_method = replacement_method(first_source, second_source)
replacement_method = replacement_method(method,
first_source,
second_source)

range = Parser::Source::Range.new(node.loc.expression.source_buffer,
node.loc.selector.begin_pos,
first_param.loc.expression.end_pos)

lambda do |corrector|
replacement =
if second_source.empty? && first_source.length == 1
"#{replacement_method}#{'!' if bang_method?(method)}" \
"(#{escape(first_source)})"
else
"#{replacement_method}#{'!' if bang_method?(method)}" \
"(#{escape(first_source)}, #{escape(second_source)})"
end

corrector.replace(range(node), replacement)
corrector.replace(range,
"#{replacement_method}(#{escape(first_source)}")

if second_source.empty? && first_source.length == 1
remove_second_param(corrector, node, first_param)
end
end
end

Expand Down Expand Up @@ -123,24 +130,26 @@ def range(node)
node.loc.expression.end_pos)
end

def replacement_method(first_source, second_source)
if second_source.empty? && first_source.length == 1
'delete'
else
'tr'
end
def replacement_method(method, first_source, second_source)
replacement = if second_source.empty? && first_source.length == 1
DELETE
else
TR
end

"#{replacement}#{BANG if bang_method?(method)}"
end

def message(method, first_source, second_source)
replacement_method = replacement_method(first_source, second_source)
replacement_method = replacement_method(method,
first_source,
second_source)

format(MSG,
"#{replacement_method}#{'!' if bang_method?(method)}",
method)
format(MSG, replacement_method, method)
end

def bang_method?(method)
method.to_s.end_with?('!')
method.to_s.end_with?(BANG)
end

def escape(string)
Expand All @@ -152,9 +161,18 @@ def escape(string)
end

def require_double_quotes?(string)
/'/ =~ string.inspect ||
string.inspect.include?(SINGLE_QUOTE) ||
StringHelp::ESCAPED_CHAR_REGEXP =~ string.inspect
end

def remove_second_param(corrector, node, first_param)
end_range =
Parser::Source::Range.new(node.loc.expression.source_buffer,
first_param.loc.expression.end_pos,
node.loc.expression.end_pos)

corrector.replace(end_range, CLOSING_PAREN)
end
end
end
end
Expand Down
10 changes: 8 additions & 2 deletions spec/rubocop/cop/performance/string_replacement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,16 @@
expect(new_source).to eq("'abc'.tr('a', '1')")
end

it 'when the replacement contains an escape character' do
it 'when the replacement contains an escape new line character' do
new_source = autocorrect_source(cop, "'abc'.gsub('a', '\n')")

expect(new_source).to eq("'abc'.tr('a', \"\\n\")")
expect(new_source).to eq("'abc'.tr('a', '\n')")
end

it 'when the replacement contains an escape backslash character' do
new_source = autocorrect_source(cop, "\"\".gsub('/', '\\\\')")

expect(new_source).to eq("\"\".tr('/', '\\\\')")
end

it 'when the pattern contains an escape character' do
Expand Down

0 comments on commit 2abc2fc

Please sign in to comment.