[Fix #5628] Rewrite SpaceInsideStringInterpolation

Rewrite in terms of `SurroundingSpace` and `SpaceCorrector`. In style `space` there was an overlap in functionality with cop `ExtraSpace`: This cop no longer removes excess whitespace.
rubocop · Jul 24, 2019 · faf8d7e · faf8d7e
1 parent 7d190a3
commit faf8d7e
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 94 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 * [#7217](https://github.com/rubocop-hq/rubocop/pull/7217): Make `Style/TrailingMethodEndStatement` work on more than the first `def`. ([@buehmann][])
 * [#7190](https://github.com/rubocop-hq/rubocop/issues/7190): Support lower case drive letters on Windows. ([@jonas054][])
+* [#5628](https://github.com/rubocop-hq/rubocop/issues/5628): Fix an error of `Layout/SpaceInsideStringInterpolation` on interpolations with multiple statements. ([@buehmann][])
 
 ### Changes
 

diff --git a/lib/rubocop/cop/layout/space_inside_string_interpolation.rb b/lib/rubocop/cop/layout/space_inside_string_interpolation.rb
@@ -19,61 +19,48 @@ module Layout
       #   # good
       #      var = "This is the #{ space } example"
       class SpaceInsideStringInterpolation < Cop
+        include SurroundingSpace
         include ConfigurableEnforcedStyle
         include RangeHelp
 
         NO_SPACE_MSG = 'Space inside string interpolation detected.'
-        SPACE_MSG = 'Missing space around string interpolation detected.'
+        SPACE_MSG = 'Missing space inside string interpolation detected.'
 
         def on_dstr(node)
-          each_style_violation(node) do |final_node|
-            add_offense(final_node)
+          node.each_child_node(:begin) do |begin_node|
+            on_interpolation(begin_node)
           end
         end
 
-        def autocorrect(node)
-          new_source = style == :no_space ? node.source : " #{node.source} "
-          lambda do |corrector|
-            corrector.replace(
-              range_with_surrounding_space(range: node.source_range),
-              new_source
-            )
+        def on_interpolation(begin_node)
+          delims = delimiters(begin_node)
+          return if empty_brackets?(*delims)
+
+          if style == :no_space
+            no_space_offenses(begin_node, *delims, NO_SPACE_MSG)
+          else
+            space_offenses(begin_node, *delims, SPACE_MSG)
           end
         end
 
-        private
-
-        def each_style_violation(node)
-          node.each_child_node(:begin) do |begin_node|
-            final_node = begin_node.children.last
-            next unless final_node
+        def autocorrect(begin_node)
+          lambda do |corrector|
+            delims = delimiters(begin_node)
 
-            if style == :no_space && space_on_any_side?(final_node)
-              yield final_node
-            elsif style == :space && !space_on_each_side?(final_node)
-              yield final_node
+            if style == :no_space
+              SpaceCorrector.remove_space(processed_source, corrector, *delims)
+            else
+              SpaceCorrector.add_space(processed_source, corrector, *delims)
             end
           end
         end
 
-        def message(_node)
-          style == :no_space ? NO_SPACE_MSG : SPACE_MSG
-        end
-
-        def space_on_any_side?(node)
-          interp = node.source_range
-          interp_with_surrounding_space =
-            range_with_surrounding_space(range: interp)
-
-          interp_with_surrounding_space != interp
-        end
-
-        def space_on_each_side?(node)
-          interp = node.source_range
-          interp_with_surrounding_space =
-            range_with_surrounding_space(range: interp)
+        private
 
-          interp_with_surrounding_space.source == " #{interp.source} "
+        def delimiters(begin_node)
+          left = processed_source.tokens[index_of_first_token(begin_node)]
+          right = processed_source.tokens[index_of_last_token(begin_node)]
+          [left, right]
         end
       end
     end

diff --git a/spec/rubocop/cop/layout/space_inside_string_interpolation_spec.rb b/spec/rubocop/cop/layout/space_inside_string_interpolation_spec.rb
@@ -3,66 +3,79 @@
 RSpec.describe RuboCop::Cop::Layout::SpaceInsideStringInterpolation, :config do
   subject(:cop) { described_class.new(config) }
 
-  let(:irregular_source) do
-    <<~'RUBY'.chomp
-      "#{ var}"
-      "#{var }"
-      "#{   var   }"
-      "#{var	}"
-      "#{	var	}"
-      "#{	var}"
-      "#{ 	 var 	 	}"
-    RUBY
-  end
-
-  shared_examples 'ill-formatted string interpolations' do
-    let(:source_length) { source.count("\n") + 1 }
-
-    it 'registers an offense for any irregular spacing inside the braces' do
-      inspect_source(source)
-      expect(cop.messages).to eq([expected_message] * source_length)
-    end
-
-    it 'auto-corrects spacing within a string interpolation' do
-      new_source = autocorrect_source(source)
-      expected_source = ([corrected_source] * source_length).join("\n")
-      expect(new_source).to eq(expected_source)
-    end
-  end
-
   context 'when EnforcedStyle is no_space' do
     let(:cop_config) { { 'EnforcedStyle' => 'no_space' } }
-    let(:expected_message) do
-      'Space inside string interpolation detected.'
-    end
 
-    context 'for always ill-formatted string interpolations' do
-      let(:source) { irregular_source }
-      let(:corrected_source) { '"#{var}"' }
+    context 'for ill-formatted string interpolations' do
+      it 'registers offenses and autocorrects' do
+        expect_offense(<<-'RUBY'.strip_indent)
+          "#{ var}"
+             ^ Space inside string interpolation detected.
+          "#{var }"
+                ^ Space inside string interpolation detected.
+          "#{   var   }"
+             ^^^ Space inside string interpolation detected.
+                   ^^^ Space inside string interpolation detected.
+          "#{var	}"
+                ^ Space inside string interpolation detected.
+          "#{	var	}"
+             ^ Space inside string interpolation detected.
+                 ^ Space inside string interpolation detected.
+          "#{	var}"
+             ^ Space inside string interpolation detected.
+          "#{ 	 var 	 	}"
+             ^^^ Space inside string interpolation detected.
+                   ^^^^ Space inside string interpolation detected.
+        RUBY
 
-      it_behaves_like 'ill-formatted string interpolations'
+        expect_correction(<<-'RUBY'.strip_indent)
+          "#{var}"
+          "#{var}"
+          "#{var}"
+          "#{var}"
+          "#{var}"
+          "#{var}"
+          "#{var}"
+        RUBY
+      end
     end
 
     context 'for "space" style formatted string interpolations' do
-      let(:source) { '"#{ var }"' }
-      let(:corrected_source) { '"#{var}"' }
+      it 'registers offenses and autocorrects' do
+        expect_offense(<<-'RUBY'.strip_indent)
+          "#{ var }"
+             ^ Space inside string interpolation detected.
+                 ^ Space inside string interpolation detected.
+        RUBY
 
-      it_behaves_like 'ill-formatted string interpolations'
+        expect_correction(<<-'RUBY'.strip_indent)
+          "#{var}"
+        RUBY
+      end
+    end
+
+    it 'does not touch spaces inside the interpolated expression' do
+      expect_offense(<<-'RUBY'.strip_indent)
+        "#{ a; b }"
+           ^ Space inside string interpolation detected.
+                ^ Space inside string interpolation detected.
+      RUBY
+
+      expect_correction(<<-'RUBY'.strip_indent)
+        "#{a; b}"
+      RUBY
     end
 
     context 'for well-formatted string interpolations' do
       let(:source) do
-        <<~'RUBY'.chomp
+        <<-'RUBY'.strip_indent
           "Variable is    #{var}      "
           "  Variable is  #{var}"
         RUBY
       end
 
       it 'does not register an offense for excess literal spacing' do
-        expect_no_offenses(<<~'RUBY')
-          "Variable is    #{var}      "
-          "  Variable is  #{var}"
-        RUBY
+        expect_no_offenses(source)
       end
 
       it 'does not correct valid string interpolations' do
@@ -78,37 +91,60 @@
 
   context 'when EnforcedStyle is space' do
     let(:cop_config) { { 'EnforcedStyle' => 'space' } }
-    let(:expected_message) do
-      'Missing space around string interpolation detected.'
-    end
 
-    context 'for always ill-formatted string interpolations' do
-      let(:source) { irregular_source }
-      let(:corrected_source) { '"#{ var }"' }
+    context 'for ill-formatted string interpolations' do
+      it 'registers offenses and autocorrects' do
+        expect_offense(<<-'RUBY'.strip_indent)
+          "#{ var}"
+                 ^ Missing space inside string interpolation detected.
+          "#{var }"
+           ^^ Missing space inside string interpolation detected.
+          "#{   var   }"
+          "#{var	}"
+           ^^ Missing space inside string interpolation detected.
+          "#{	var	}"
+          "#{	var}"
+                 ^ Missing space inside string interpolation detected.
+          "#{ 	 var 	 	}"
+        RUBY
 
-      it_behaves_like 'ill-formatted string interpolations'
+        # Extra space is handled by ExtraSpace cop.
+        expect_correction(<<-'RUBY'.strip_indent)
+          "#{ var }"
+          "#{ var }"
+          "#{   var   }"
+          "#{ var	}"
+          "#{	var	}"
+          "#{	var }"
+          "#{ 	 var 	 	}"
+        RUBY
+      end
     end
 
     context 'for "no_space" style formatted string interpolations' do
-      let(:source) { '"#{var}"' }
-      let(:corrected_source) { '"#{ var }"' }
+      it 'registers offenses and autocorrects' do
+        expect_offense(<<-'RUBY'.strip_indent)
+          "#{var}"
+           ^^ Missing space inside string interpolation detected.
+                ^ Missing space inside string interpolation detected.
+        RUBY
 
-      it_behaves_like 'ill-formatted string interpolations'
+        expect_correction(<<-'RUBY'.strip_indent)
+          "#{ var }"
+        RUBY
+      end
     end
 
     context 'for well-formatted string interpolations' do
       let(:source) do
-        <<~'RUBY'.chomp
+        <<-'RUBY'.strip_indent
           "Variable is    #{ var }      "
           "  Variable is  #{ var }"
         RUBY
       end
 
       it 'does not register an offense for excess literal spacing' do
-        expect_no_offenses(<<~'RUBY')
-          "Variable is    #{ var }      "
-          "  Variable is  #{ var }"
-        RUBY
+        expect_no_offenses(source)
       end
 
       it 'does not correct valid string interpolations' do