Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent Lint/LiteralInInterpolation from removing necessary interpolation #8921

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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