Skip to content

Commit

Permalink
Prevent Lint/LiteralInInterpolation from removing necessary interpola…
Browse files Browse the repository at this point in the history
…tion

%W and %I split the content into words before expansion treating each
interpolation as a word component.

For example, the following array literal evaluates to
`["-H", "Cookie: a=1; b=2", "https://example.com/"]`.

```ruby
args = %W[-H #{"Cookie: a=1; b=2"} https://example.com/]
```

However, Lint/LiteralInInterpolation will wrongly auto-correct it to
this:

```ruby
args = %W[-H Cookie: a=1; b=2 https://example.com/]
```

Which gives a completely different set of elements:
`["-H", "Cookie:", "a=1;", "b=2", "https://example.com/"]`.

This fix teaches the cop not to expand an interpolation when the
expanded value contains a space character.
  • Loading branch information
knu committed Oct 29, 2020
1 parent 5905396 commit 2bb4490
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -20,6 +20,10 @@
* [#8933](https://github.com/rubocop-hq/rubocop/pull/8933): Fix an error for `Layout/EmptyLinesAroundAccessModifier` when the first line is a comment. ([@matthieugendreau][])
* [#8954](https://github.com/rubocop-hq/rubocop/pull/8954): Fix autocorrection for Style/RedundantRegexpCharacterClass with %r. ([@ysakasin][])

### Bug fixes

* [#8921](https://github.com/rubocop-hq/rubocop/pull/8921): Prevent `Lint/LiteralInInterpolation` from removing necessary interpolation in `%W[]` and `%I[]` literals. ([@knu][])

### Changes

* [#8920](https://github.com/rubocop-hq/rubocop/pull/8920): Remove Capybara's `save_screenshot` from `Lint/Debugger`. ([@ybiquitous][])
Expand Down
20 changes: 17 additions & 3 deletions lib/rubocop/cop/lint/literal_in_interpolation.rb
Expand Up @@ -31,12 +31,18 @@ def on_interpolation(begin_node)
return if special_keyword?(final_node)
return unless prints_as_self?(final_node)

# %W and %I split the content into words before expansion
# treating each interpolation as a word component, so
# interpolation should not be removed if the expanded value
# contains a space character.
expanded_value = autocorrected_value(final_node)
return if in_array_percent_literal?(begin_node) &&
/\s/.match?(expanded_value)

add_offense(final_node) do |corrector|
return if final_node.dstr_type? # nested, fixed in next iteration

value = autocorrected_value(final_node)

corrector.replace(final_node.parent, value)
corrector.replace(final_node.parent, expanded_value)
end
end

Expand Down Expand Up @@ -92,6 +98,14 @@ def prints_as_self?(node)
(COMPOSITE.include?(node.type) &&
node.children.all? { |child| prints_as_self?(child) })
end

def in_array_percent_literal?(node)
parent = node.parent
return false unless parent.dstr_type? || parent.dsym_type?

grandparent = parent.parent
grandparent&.array_type? && grandparent&.percent_literal?
end
end
end
end
Expand Down
71 changes: 71 additions & 0 deletions spec/rubocop/cop/lint/literal_in_interpolation_spec.rb
Expand Up @@ -113,6 +113,77 @@
it_behaves_like('literal interpolation', '%i[s1 s2]', '[\"s1\", \"s2\"]')
it_behaves_like('literal interpolation', '%i[ s1 s2 ]', '[\"s1\", \"s2\"]')

shared_examples 'literal interpolation in words literal' do |prefix|
let(:word) { 'interpolation' }

it "accepts interpolation of a string literal with space in #{prefix}[]" do
expect_no_offenses(<<~RUBY)
#{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]
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]
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]
RUBY
end

it "removes interpolation of a string literal without space in #{prefix}[]" do
expect_offense(<<~'RUBY', prefix: prefix, literal: word.inspect)
%{prefix}[this #{%{literal}} is not significant]
_{prefix} ^{literal} Literal interpolation detected.
RUBY
expect_correction(<<~RUBY)
#{prefix}[this #{word} is not significant]
RUBY
end

it "removes interpolation of a symbol literal without space in #{prefix}[]" do
expect_offense(<<~'RUBY', prefix: prefix, literal: word.to_sym.inspect)
%{prefix}[this #{%{literal}} is not significant]
_{prefix} ^{literal} Literal interpolation detected.
RUBY
expect_correction(<<~RUBY)
#{prefix}[this #{word} is not significant]
RUBY
end

it "removes interpolation of an array containing a string literal without space in #{prefix}[]" do
expect_offense(<<~'RUBY', prefix: prefix, literal: [word].inspect)
%{prefix}[this #{%{literal}} is not significant]
_{prefix} ^{literal} Literal interpolation detected.
RUBY
expect_correction(<<~RUBY)
#{prefix}[this #{[word].inspect.gsub(/"/, '\"')} is not significant]
RUBY
end

it "removes interpolation of an array containing a symbol literal without space in #{prefix}[]" do
expect_offense(<<~'RUBY', prefix: prefix, literal: [word.to_sym].inspect)
%{prefix}[this #{%{literal}} is not significant]
_{prefix} ^{literal} Literal interpolation detected.
RUBY
expect_correction(<<~RUBY)
#{prefix}[this #{[word.to_sym].inspect} is not significant]
RUBY
end
end

it_behaves_like('literal interpolation in words literal', '%W')
it_behaves_like('literal interpolation in words literal', '%I')

it 'handles nested interpolations when auto-correction' do
expect_offense(<<~'RUBY')
"this is #{"#{1}"} silly"
Expand Down

0 comments on commit 2bb4490

Please sign in to comment.