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|