From 2bb4490f64d7f84973da339a350f8e81135406f4 Mon Sep 17 00:00:00 2001 From: Akinori MUSHA Date: Fri, 23 Oct 2020 00:53:32 +0900 Subject: [PATCH] Prevent Lint/LiteralInInterpolation from removing necessary interpolation %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. --- CHANGELOG.md | 4 ++ .../cop/lint/literal_in_interpolation.rb | 20 +++++- .../cop/lint/literal_in_interpolation_spec.rb | 71 +++++++++++++++++++ 3 files changed, 92 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d94a53ce04..a65e954614a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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][]) diff --git a/lib/rubocop/cop/lint/literal_in_interpolation.rb b/lib/rubocop/cop/lint/literal_in_interpolation.rb index 6fad736bbf0..82bcd8f1874 100644 --- a/lib/rubocop/cop/lint/literal_in_interpolation.rb +++ b/lib/rubocop/cop/lint/literal_in_interpolation.rb @@ -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 @@ -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 diff --git a/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb b/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb index 2da6eba9b8e..53a6e715441 100644 --- a/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb +++ b/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb @@ -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"