From 837899cfd1e92a5bf39bd665bf54499df3a49e84 Mon Sep 17 00:00:00 2001
From: Daniel Vandersluis <daniel.vandersluis@gmail.com>
Date: Thu, 8 Oct 2020 11:01:40 -0400
Subject: [PATCH] [Fix #8781] Change how comments are determined for
 `Style/SafeNavigation` autocorrection.

Previously, every comment within the source range of the `if` being auto-corrected was captured and moved above the rewritten line. This resulted in comment duplication, as some comments belonged to internal blocks within the if. This change only considers line ranges between internal if blocks as entry points for comments that are allowed to be moved.
---
 CHANGELOG.md                                  |  1 +
 lib/rubocop/cop/style/safe_navigation.rb      | 20 +++++++++++---
 .../rubocop/cop/style/safe_navigation_spec.rb | 26 +++++++++++++++++++
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index df36b728eb0..7fef55e2c5d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Bug fixes
 
 * [#8892](https://github.com/rubocop-hq/rubocop/issues/8892): Fix an error for `Style/StringConcatenation` when correcting nested concatenable parts. ([@fatkodima][])
+* [#8781](https://github.com/rubocop-hq/rubocop/issues/8781): Fix handling of comments in `Style/SafeNavigation` autocorrection. ([@dvandersluis][])
 
 ### Changes
 
diff --git a/lib/rubocop/cop/style/safe_navigation.rb b/lib/rubocop/cop/style/safe_navigation.rb
index 99e44feab5c..7043e3b5959 100644
--- a/lib/rubocop/cop/style/safe_navigation.rb
+++ b/lib/rubocop/cop/style/safe_navigation.rb
@@ -142,10 +142,22 @@ def handle_comments(corrector, node, method_call)
         end
 
         def comments(node)
-          processed_source.each_comment_in_lines(
-            node.loc.first_line...
-            node.loc.last_line
-          ).to_a
+          relevant_comment_ranges(node).each.with_object([]) do |range, comments|
+            comments.concat(processed_source.each_comment_in_lines(range).to_a)
+          end
+        end
+
+        def relevant_comment_ranges(node)
+          # Get source lines ranges inside the if node that aren't inside an inner node
+          # Comments inside an inner node should remain attached to that node, and not
+          # moved.
+          begin_pos = node.loc.first_line
+          end_pos = node.loc.last_line
+
+          node.child_nodes.each.with_object([]) do |child, ranges|
+            ranges << (begin_pos...child.loc.first_line)
+            begin_pos = child.loc.last_line
+          end << (begin_pos...end_pos)
         end
 
         def allowed_if_condition?(node)
diff --git a/spec/rubocop/cop/style/safe_navigation_spec.rb b/spec/rubocop/cop/style/safe_navigation_spec.rb
index cc10dee6876..920e972f6b8 100644
--- a/spec/rubocop/cop/style/safe_navigation_spec.rb
+++ b/spec/rubocop/cop/style/safe_navigation_spec.rb
@@ -160,6 +160,32 @@
     RUBY
   end
 
+  # See https://github.com/rubocop-hq/rubocop/issues/8781
+  it 'does not move comments that are inside an inner block' do
+    expect_offense(<<~RUBY)
+      # Comment 1
+      if x
+      ^^^^ Use safe navigation (`&.`) instead of checking if an object exists before calling the method.
+        # Comment 2
+        x.each do
+          # Comment 3
+          # Comment 4
+        end
+        # Comment 5
+      end
+    RUBY
+
+    expect_correction(<<~RUBY)
+      # Comment 1
+      # Comment 2
+      # Comment 5
+      x&.each do
+          # Comment 3
+          # Comment 4
+        end
+    RUBY
+  end
+
   shared_examples 'all variable types' do |variable|
     context 'modifier if' do
       shared_examples 'safe guarding logical break keywords' do |keyword|