diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a971202336d..37b91b0d5945 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/rubocop/cop/mixin/percent_literal.rb b/lib/rubocop/cop/mixin/percent_literal.rb index 8c7b4d360745..b38a29a1d8fd 100644 --- a/lib/rubocop/cop/mixin/percent_literal.rb +++ b/lib/rubocop/cop/mixin/percent_literal.rb @@ -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) @@ -35,14 +37,18 @@ 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 @@ -50,10 +56,45 @@ def autocorrect_words(word_nodes, escape, base_line_number) 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 diff --git a/lib/rubocop/cop/style/percent_literal_delimiters.rb b/lib/rubocop/cop/style/percent_literal_delimiters.rb index 356651d1f801..e1abf6cdf2c9 100644 --- a/lib/rubocop/cop/style/percent_literal_delimiters.rb +++ b/lib/rubocop/cop/style/percent_literal_delimiters.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/style/symbol_array_spec.rb b/spec/rubocop/cop/style/symbol_array_spec.rb index fd0fbb674aaa..a5c00110e181 100644 --- a/spec/rubocop/cop/style/symbol_array_spec.rb +++ b/spec/rubocop/cop/style/symbol_array_spec.rb @@ -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' } } @@ -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 @@ -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 diff --git a/spec/rubocop/cop/style/word_array_spec.rb b/spec/rubocop/cop/style/word_array_spec.rb index 51ff93f1d8a3..034fc8024f69 100644 --- a/spec/rubocop/cop/style/word_array_spec.rb +++ b/spec/rubocop/cop/style/word_array_spec.rb @@ -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, @@ -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