Merge pull request #14424 from martinemde/martinemde/fix-incorrect-autocorrect-style-safe-navigation

koic · web-flow · commit 5535cd33a076 · 2025-08-06T11:34:21.000+09:00
Fix Style/SafeNavigation cop to preserve existing safe navigation in check
diff --git a/changelog/fix_fix_safe_navigation_cop_to_preserve_safe_20250805124624.md b/changelog/fix_fix_safe_navigation_cop_to_preserve_safe_20250805124624.md
@@ -0,0 +1 @@
+* [#14424](https://github.com/rubocop/rubocop/pull/14424): Fix SafeNavigation cop to preserve existing safe navigation in fixed code. ([@martinemde][])
diff --git a/lib/rubocop/cop/style/safe_navigation.rb b/lib/rubocop/cop/style/safe_navigation.rb
@@ -142,6 +142,7 @@ class SafeNavigation < Base # rubocop:disable Metrics/ClassLength
         # @!method strip_begin(node)
         def_node_matcher :strip_begin, '{ (begin $!begin) $!(begin) }'
 
+        # rubocop:disable Metrics/AbcSize
         def on_if(node)
           return if allowed_if_condition?(node)
 
@@ -155,9 +156,11 @@ def on_if(node)
           removal_ranges = [begin_range(node, body), end_range(node, body)]
 
           report_offense(node, method_chain, method_call, *removal_ranges) do |corrector|
+            corrector.replace(receiver, checked_variable.source) if checked_variable.csend_type?
             corrector.insert_before(method_call.loc.dot, '&') unless method_call.safe_navigation?
           end
         end
+        # rubocop:enable Metrics/AbcSize
 
         def on_and(node) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength
           collect_and_clauses(node).each do |(lhs, lhs_operator_range), (rhs, _rhs_operator_range)|
@@ -335,8 +338,14 @@ def matching_nodes?(left, right)
 
         def matching_call_nodes?(left, right)
           return false unless left && right.respond_to?(:call_type?)
+          return false unless left.call_type? && right.call_type?
 
-          left.call_type? && right.call_type? && left.children == right.children
+          # Compare receiver and method name, but ignore the difference between
+          # safe navigation method call (`&.`) and dot method call (`.`).
+          left_receiver, left_method = left.children.take(2)
+          right_receiver, right_method = right.children.take(2)
+
+          left_method == right_method && matching_nodes?(left_receiver, right_receiver)
         end
 
         def chain_length(method_chain, method)
diff --git a/spec/rubocop/cop/style/safe_navigation_spec.rb b/spec/rubocop/cop/style/safe_navigation_spec.rb
@@ -1437,6 +1437,41 @@ def foobar
               #{variable}&.one&.two(baz) { |e| e.qux }
             RUBY
           end
+
+          it 'corrects a ternary expression with safe navigation object check followed by a chained method call' do
+            expect_offense(<<~RUBY, variable: variable)
+              %{variable}&.bar ? %{variable}.bar.baz : nil
+              ^{variable}^^^^^^^^^{variable}^^^^^^^^^^^^^^ Use safe navigation (`&.`) instead [...]
+            RUBY
+
+            expect_correction(<<~RUBY)
+              #{variable}&.bar&.baz
+            RUBY
+          end
+
+          it 'corrects an object check with safe navigation followed by a chained method call' do
+            expect_offense(<<~RUBY, variable: variable)
+              %{variable}&.bar && %{variable}.bar.baz
+              ^{variable}^^^^^^^^^^{variable}^^^^^^^^ Use safe navigation (`&.`) instead [...]
+            RUBY
+
+            expect_correction(<<~RUBY)
+              #{variable}&.bar&.baz
+            RUBY
+          end
+
+          it 'corrects a check with safe navigation followed by a chained method call' do
+            expect_offense(<<~RUBY, variable: variable)
+              if %{variable}&.bar
+              ^^^^{variable}^^^^^ Use safe navigation (`&.`) instead [...]
+                %{variable}.bar.baz
+              end
+            RUBY
+
+            expect_correction(<<~RUBY)
+              #{variable}&.bar&.baz
+            RUBY
+          end
         end
       end
 
