Skip to content

Commit

Permalink
[Fix rubocop#4199] Fix incorrect auto correction in `Style/SymbolArra…
Browse files Browse the repository at this point in the history
…y` cop

See rubocop#4199

If `PreferredDelimiters` option is specified, `Style/SymbolArray` and `Style/WordArray` cops auto correct to not expected code.
The option is ignored.

For example

```ruby
 # PreferredDelimiters is []

 # Before
 [:a, :b, :c]
 ['a', 'b', 'c']

 # Corrected
 %i(a b c) # Not %i[]
 %w(a b c) # Not %w[]
```

This change fixes the problem.
  • Loading branch information
pocke committed Mar 30, 2017
1 parent cf8c802 commit 6ea36db
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* [#4172](https://github.com/bbatsov/rubocop/pull/4172): Fix false positives in `Style/MixinGrouping` cop. ([@drenmi][])
* [#4185](https://github.com/bbatsov/rubocop/pull/4185): Make `Lint/NestedMethodDefinition` aware of `#*_exec` class of methods. ([@drenmi][])
* [#4197](https://github.com/bbatsov/rubocop/pull/4197): Fix false positive in `Style/RedundantSelf` cop with parallel assignment. ([@drenmi][])
* [#4199](https://github.com/bbatsov/rubocop/issues/4199): Fix incorrect auto correction in `Style/SymbolArray` and `Style/WordArray` cop. ([@pocke][])

## 0.48.0 (2017-03-26)

Expand Down
49 changes: 45 additions & 4 deletions lib/rubocop/cop/mixin/percent_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module RuboCop
module Cop
# Common functionality for handling percent literals.
module PercentLiteral
PERCENT_LITERAL_TYPES = %w(% %i %I %q %Q %r %s %w %W %x).freeze

private

def percent_literal?(node)
Expand Down Expand Up @@ -35,25 +37,64 @@ def correct_percent(node, char)
words = node.children
escape = words.any? { |w| needs_escaping?(w.children[0]) }
char = char.upcase if escape
contents = autocorrect_words(words, escape, node.loc.line)
delimiters = preferred_delimiters_for("%#{char}")
contents = autocorrect_words(words, escape, node.loc.line, delimiters)

lambda do |corrector|
corrector.replace(node.source_range, "%#{char}(#{contents})")
corrector.replace(
node.source_range,
"%#{char}#{delimiters[0]}#{contents}#{delimiters[1]}"
)
end
end

def autocorrect_words(word_nodes, escape, base_line_number)
def autocorrect_words(word_nodes, escape, base_line_number, delimiters)
previous_node_line_number = base_line_number
word_nodes.map do |node|
number_of_line_breaks = node.loc.line - previous_node_line_number
line_breaks = "\n" * number_of_line_breaks
previous_node_line_number = node.loc.line
content = node.children.first.to_s
content = escape ? escape_string(content) : content
content.gsub!(/\)/, '\\)')
delimiters.each do |delimiter|
content.gsub!(delimiter, "\\#{delimiter}")
end
line_breaks + content
end.join(' ')
end

def ensure_valid_preferred_delimiters
invalid = preferred_delimiters_config.keys -
(PERCENT_LITERAL_TYPES + %w(default))
return if invalid.empty?

raise ArgumentError,
"Invalid preferred delimiter config key: #{invalid.join(', ')}"
end

def preferred_delimiters
@preferred_delimiters ||=
begin
ensure_valid_preferred_delimiters

if preferred_delimiters_config.key?('default')
Hash[PERCENT_LITERAL_TYPES.map do |type|
[type, preferred_delimiters_config[type] ||
preferred_delimiters_config['default']]
end]
else
preferred_delimiters_config
end
end
end

def preferred_delimiters_config
@config.for_cop('Style/PercentLiteralDelimiters')['PreferredDelimiters']
end

def preferred_delimiters_for(type)
preferred_delimiters[type].split(//)
end
end
end
end
31 changes: 0 additions & 31 deletions lib/rubocop/cop/style/percent_literal_delimiters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ module Style
class PercentLiteralDelimiters < Cop
include PercentLiteral

PERCENT_LITERAL_TYPES = %w(% %i %I %q %Q %r %s %w %W %x).freeze

def on_array(node)
process(node, '%w', '%W', '%i', '%I')
end
Expand Down Expand Up @@ -78,35 +76,6 @@ def on_percent_literal(node)
add_offense(node, :expression)
end

def preferred_delimiters
@preferred_delimiters ||=
begin
ensure_valid_preferred_delimiters

if cop_config['PreferredDelimiters'].key?('default')
Hash[PERCENT_LITERAL_TYPES.map do |type|
[type, cop_config['PreferredDelimiters'][type] ||
cop_config['PreferredDelimiters']['default']]
end]
else
cop_config['PreferredDelimiters']
end
end
end

def ensure_valid_preferred_delimiters
invalid = cop_config['PreferredDelimiters'].keys -
(PERCENT_LITERAL_TYPES + %w(default))
return if invalid.empty?

raise ArgumentError,
"Invalid preferred delimiter config key: #{invalid.join(', ')}"
end

def preferred_delimiters_for(type)
preferred_delimiters[type].split(//)
end

def uses_preferred_delimiter?(node, type)
preferred_delimiters_for(type)[0] == begin_source(node)[-1]
end
Expand Down
33 changes: 31 additions & 2 deletions spec/rubocop/cop/style/symbol_array_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,16 @@
describe RuboCop::Cop::Style::SymbolArray, :config do
subject(:cop) { described_class.new(config) }

let(:other_cops) do
{
'Style/PercentLiteralDelimiters' => {
'PreferredDelimiters' => {
'default' => '()'
}
}
}
end

context 'when EnforcedStyle is percent' do
let(:cop_config) { { 'EnforcedStyle' => 'percent' } }

Expand All @@ -28,8 +38,9 @@
end

it "doesn't break when a symbol contains )" do
new_source = autocorrect_source(cop, '[:one, :")", :three]')
expect(new_source).to eq('%i(one \\) three)')
source = '[:one, :")", :three, :"(", :"]", :"["]'
new_source = autocorrect_source(cop, source)
expect(new_source).to eq('%i(one \\) three \\( ] [)')
end

it 'does not register an offense for array with non-syms' do
Expand Down Expand Up @@ -58,6 +69,24 @@
expect(cop.offenses).to be_empty
end
end

context 'when PreferredDelimiters is specified' do
let(:other_cops) do
{
'Style/PercentLiteralDelimiters' => {
'PreferredDelimiters' => {
'default' => '[]'
}
}
}
end

it 'autocorrects an array with delimiters' do
source = '[:one, :")", :three, :"(", :"]", :"["]'
new_source = autocorrect_source(cop, source)
expect(new_source).to eq('%i[one ) three ( \\] \\[]')
end
end
end

context 'when EnforcedStyle is array' do
Expand Down
31 changes: 29 additions & 2 deletions spec/rubocop/cop/style/word_array_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@
described_class.largest_brackets = -Float::INFINITY
end

let(:other_cops) do
{
'Style/PercentLiteralDelimiters' => {
'PreferredDelimiters' => {
'default' => '()'
}
}
}
end

context 'when EnforcedStyle is percent' do
let(:cop_config) do
{ 'MinSize' => 0,
Expand Down Expand Up @@ -255,8 +265,25 @@
end

it "doesn't break when words contain delimiters" do
new_source = autocorrect_source(cop, "[')', ']']")
expect(new_source).to eq('%w(\\) ])')
new_source = autocorrect_source(cop, "[')', ']', '(']")
expect(new_source).to eq('%w(\\) ] \\()')
end

context 'when PreferredDelimiters is specified' do
let(:other_cops) do
{
'Style/PercentLiteralDelimiters' => {
'PreferredDelimiters' => {
'default' => '[]'
}
}
}
end

it 'autocorrects an array with delimiters' do
new_source = autocorrect_source(cop, "[')', ']', '(', '[']")
expect(new_source).to eq('%w[) \\] ( \\[]')
end
end
end
end

0 comments on commit 6ea36db

Please sign in to comment.