Merge pull request #1115 from rubocop-hq/improve-move-node-corrections

Take into account comments when moving nodes

Author: bquorning

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Master (Unreleased)
 
+* Fix `HooksBeforeExamples`, `LeadingSubject`, `LetBeforeExamples` and `ScatteredLet` autocorrection to take into account inline comments and comments immediately before the moved node. ([@Darhazer][])
+
 ## 2.1.0 (2020-12-17)
 
 * Fix `RSpec/FilePath` false positive for relative file path runs with long namespaces. ([@ahukkanen][])

lib/rubocop-rspec.rb

@@ -15,6 +15,7 @@
 require_relative 'rubocop/cop/rspec/mixin/top_level_group'
 require_relative 'rubocop/cop/rspec/mixin/variable'
 require_relative 'rubocop/cop/rspec/mixin/final_end_location'
+require_relative 'rubocop/cop/rspec/mixin/comments_help'
 require_relative 'rubocop/cop/rspec/mixin/empty_line_separation'
 
 require_relative 'rubocop/rspec/concept'

lib/rubocop/cop/rspec/mixin/comments_help.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Help methods for working with nodes containing comments.
+      module CommentsHelp
+        include FinalEndLocation
+
+        def source_range_with_comment(node)
+          begin_pos = begin_pos_with_comment(node).begin_pos
+          end_pos = end_line_position(node).end_pos
+
+          Parser::Source::Range.new(buffer, begin_pos, end_pos)
+        end
+
+        def begin_pos_with_comment(node)
+          first_comment = processed_source.ast_with_comments[node].first
+
+          start_line_position(first_comment || node)
+        end
+
+        def start_line_position(node)
+          buffer.line_range(node.loc.line)
+        end
+
+        def end_line_position(node)
+          end_line = buffer.line_for_position(final_end_location(node).end_pos)
+          buffer.line_range(end_line)
+        end
+
+        def buffer
+          processed_source.buffer
+        end
+      end
+    end
+  end
+end

lib/rubocop/rspec/corrector/move_node.rb

@@ -6,6 +6,7 @@ module Corrector
       # Helper methods to move a node
       class MoveNode
         include RuboCop::Cop::RangeHelp
+        include RuboCop::Cop::RSpec::CommentsHelp
         include RuboCop::Cop::RSpec::FinalEndLocation
 
         attr_reader :original, :corrector, :processed_source
@@ -17,20 +18,16 @@ def initialize(node, corrector, processed_source)
         end
 
         def move_before(other)
-          position = other.loc.expression
-          indent = ' ' * other.loc.column
-          newline_indent = "\n#{indent}"
+          position = start_line_position(other)
 
-          corrector.insert_before(position, source(original) + newline_indent)
+          corrector.insert_before(position, "#{source(original)}\n")
           corrector.remove(node_range_with_surrounding_space(original))
         end
 
         def move_after(other)
-          position = final_end_location(other)
-          indent = ' ' * other.loc.column
-          newline_indent = "\n#{indent}"
+          position = end_line_position(other)
 
-          corrector.insert_after(position, newline_indent + source(original))
+          corrector.insert_after(position, "\n#{source(original)}")
           corrector.remove(node_range_with_surrounding_space(original))
         end
 
@@ -41,7 +38,7 @@ def source(node)
         end
 
         def node_range(node)
-          node.loc.expression.with(end_pos: final_end_location(node).end_pos)
+          source_range_with_comment(node)
         end
 
         def node_range_with_surrounding_space(node)

spec/rubocop/cop/rspec/hooks_before_examples_spec.rb

@@ -94,6 +94,27 @@
     RUBY
   end
 
+  it 'works with comments' do
+    expect_offense(<<-RUBY)
+      RSpec.describe User do
+        it { is_expected.to be_after_before_hook } # h
+        # setup the system
+        # with multiline comment
+        before { setup } # some setup
+        ^^^^^^^^^^^^^^^^ Move `before` above the examples in the group.
+      end
+    RUBY
+
+    expect_correction(<<-RUBY)
+      RSpec.describe User do
+        # setup the system
+        # with multiline comment
+        before { setup } # some setup
+        it { is_expected.to be_after_before_hook } # h
+      end
+    RUBY
+  end
+
   it 'does not flag hooks before the examples' do
     expect_no_offenses(<<-RUBY)
       RSpec.describe User do

spec/rubocop/cop/rspec/scattered_let_spec.rb

@@ -49,6 +49,33 @@
     RUBY
   end
 
+  it 'works with comments' do
+    expect_offense(<<-RUBY)
+      RSpec.describe User do
+        let(:a) { a } # a comment
+        # example comment
+        it { expect(subject.foo).to eq(a) }
+        it { expect(subject.fu).to eq(b) } # inline example comment
+        # define the second letter
+        # with a multi-line description
+        let(:b) { b } # inline explanation as well
+        ^^^^^^^^^^^^^ Group all let/let! blocks in the example group together.
+      end
+    RUBY
+
+    expect_correction(<<-RUBY)
+      RSpec.describe User do
+        let(:a) { a } # a comment
+        # define the second letter
+        # with a multi-line description
+        let(:b) { b } # inline explanation as well
+        # example comment
+        it { expect(subject.foo).to eq(a) }
+        it { expect(subject.fu).to eq(b) } # inline example comment
+      end
+    RUBY
+  end
+
   it 'flags `let` at different nesting levels' do
     expect_offense(<<-RUBY)
       describe User do