Skip to content

Commit

Permalink
[Fix rubocop#2294] Allow regex with options in StringReplacement
Browse files Browse the repository at this point in the history
  • Loading branch information
rrosenblum committed Oct 8, 2015
1 parent 9b13f84 commit 4217327
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
* [#2284](https://github.com/bbatsov/rubocop/issues/2284): Fix result cache being shared between ruby versions. ([@miquella][])
* [#2285](https://github.com/bbatsov/rubocop/issues/2285): Fix `ConfigurableNaming#class_emitter_method?` error when handling singleton class methods. ([@palkan][])
* [#2295](https://github.com/bbatsov/rubocop/issues/2295): Fix Performance/Detect autocorrect to handle rogue newlines. ([@palkan][])
* [#2294](https://github.com/bbatsov/rubocop/issues/2294): Do not register an offense in `Performance/StringReplacement` for regex with options. ([@rrosenblum][])

## 0.34.2 (21/09/2015)

Expand Down
20 changes: 13 additions & 7 deletions lib/rubocop/cop/performance/string_replacement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,17 @@ class StringReplacement < Cop
def on_send(node)
_string, method, first_param, second_param = *node
return unless GSUB_METHODS.include?(method)
return unless second_param && second_param.str_type?
return unless string?(second_param)
return unless DETERMINISTIC_TYPES.include?(first_param.type)

first_source = first_source(first_param)
first_source, options = first_source(first_param)
second_source, = *second_param

return if first_source.nil?

if regex?(first_param)
return unless first_source =~ DETERMINISTIC_REGEX
return if options
end

return if first_source.length != 1
Expand All @@ -53,7 +54,7 @@ def on_send(node)

def autocorrect(node)
_string, method, first_param, second_param = *node
first_source = first_source(first_param)
first_source, = first_source(first_param)
second_source, = *second_param
replacement_method = replacement_method(method,
first_source,
Expand All @@ -74,17 +75,21 @@ def autocorrect(node)

private

def string?(node)
node && node.str_type?
end

def first_source(first_param)
case first_param.type
when :regexp, :send
return nil unless regex?(first_param)

source, = extract_source(first_param)
source, options = extract_source(first_param)
when :str
source, = *first_param
end

source
[source, options]
end

def extract_source(node)
Expand All @@ -97,9 +102,10 @@ def extract_source(node)
end

def source_from_regex_literal(node)
regex, = *node
regex, options = *node
source, = *regex
source
options, = *options
[source, options]
end

def source_from_regex_constructor(node)
Expand Down
12 changes: 12 additions & 0 deletions spec/rubocop/cop/performance/string_replacement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,18 @@
expect(cop.messages).to be_empty
end

it 'allows regex literals with options' do
inspect_source(cop, "'abc'.#{method}(/a/i, '1')")

expect(cop.messages).to be_empty
end

it 'allows regex with options' do
inspect_source(cop, "'abc'.#{method}(Regexp.new(/a/i), '1')")

expect(cop.messages).to be_empty
end

it 'allows empty string pattern' do
inspect_source(cop, "'abc'.gsub('', '1')")

Expand Down

0 comments on commit 4217327

Please sign in to comment.