[Fix rubocop#13103] Make Lint/UselessAssignment autocorrection safe

Fixes rubocop#13103.

This PR makes `Lint/UselessAssignment` autocorrection safe.

changelog/change_make_lint_useless_assignment_autocorrection_safe.md

@@ -0,0 +1 @@
+* [#13103](https://github.com/rubocop/rubocop/issues/13103): Make `Lint/UselessAssignment` autocorrection safe. ([@koic][])

config/default.yml

@@ -2543,8 +2543,7 @@ Lint/UselessAssignment:
   Enabled: true
   AutoCorrect: contextual
   VersionAdded: '0.11'
-  VersionChanged: '1.61'
-  SafeAutoCorrect: false
+  VersionChanged: '<<next>>'
 
 Lint/UselessElseWithoutRescue:
   Description: 'Checks for useless `else` in `begin..end` without `rescue`.'

lib/rubocop/cop/lint/useless_assignment.rb

@@ -16,15 +16,14 @@ module Lint
       # reassignments and properly handles varied cases such as branch, loop,
       # rescue, ensure, etc.
       #
+      # This cop's autocorrection avoids cases like `a ||= 1` because removing assignment from
+      # operator assignment can cause NameError if this assignment has been used to declare
+      # a local variable. For example, replacing `a ||= 1` with `a || 1` may cause
+      # "undefined local variable or method `a' for main:Object (NameError)".
+      #
       # NOTE: Given the assignment `foo = 1, bar = 2`, removing unused variables
       # can lead to a syntax error, so this case is not autocorrected.
       #
-      # @safety
-      #   This cop's autocorrection is unsafe because removing assignment from
-      #   operator assignment can cause NameError if this assignment has been used to declare
-      #   local variable. For example, replacing `a ||= 1` to `a || 1` may cause
-      #   "undefined local variable or method `a' for main:Object (NameError)".
-      #
       # @example
       #
       #   # bad
@@ -53,24 +52,32 @@ def after_leaving_scope(scope, _variable_table)
           scope.variables.each_value { |variable| check_for_unused_assignments(variable) }
         end
 
-        # rubocop:disable Metrics/AbcSize
+        # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity
         def check_for_unused_assignments(variable)
           return if variable.should_be_unused?
 
           variable.assignments.reverse_each do |assignment|
-            next if assignment.used? || part_of_ignored_node?(assignment.node)
+            assignment_node = assignment.node
+            next if assignment.used? || part_of_ignored_node?(assignment_node)
 
             message = message_for_useless_assignment(assignment)
             range = offense_range(assignment)
 
             add_offense(range, message: message) do |corrector|
-              autocorrect(corrector, assignment) unless sequential_assignment?(assignment.node)
+              # In cases like `x = 1, y = 2`, where removing a variable would cause a syntax error,
+              # and where changing `x ||= 1` to `x = 1` would cause `NameError`,
+              # the autocorrect will be skipped, even if the variable is unused.
+              if sequential_assignment?(assignment_node) || assignment_node.parent&.or_asgn_type?
+                next
+              end
+
+              autocorrect(corrector, assignment)
             end
 
-            ignore_node(assignment.node) if chained_assignment?(assignment.node)
+            ignore_node(assignment_node) if chained_assignment?(assignment_node)
           end
         end
-        # rubocop:enable Metrics/AbcSize
+        # rubocop:enable Metrics/AbcSize, Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity
 
         def message_for_useless_assignment(assignment)
           variable = assignment.variable

spec/rubocop/cli/options_spec.rb

@@ -979,7 +979,7 @@ def on_send(node)
             1  Layout/SpaceAroundOperators [Safe Correctable]
             1  Layout/TrailingWhitespace [Safe Correctable]
             1  Lint/MissingCopEnableDirective
-            1  Lint/UselessAssignment [Unsafe Correctable]
+            1  Lint/UselessAssignment [Safe Correctable]
             1  Migration/DepartmentName [Safe Correctable]
             1  Style/FrozenStringLiteralComment [Unsafe Correctable]
             1  Style/NumericPredicate [Unsafe Correctable]
@@ -2016,11 +2016,10 @@ def f
           a = "Hello"
         RUBY
 
-        expect(cli.run(['--autocorrect', '--format', 'simple', target_file])).to eq(1)
+        expect(cli.run(['--autocorrect', '--format', 'simple', target_file])).to eq(0)
 
         expect($stdout.string.lines.to_a.last)
-          .to eq('1 file inspected, 2 offenses detected, 1 offense corrected, 1 more offense can ' \
-                 "be corrected with `rubocop -A`\n")
+          .to eq("1 file inspected, 3 offenses detected, 3 offenses corrected\n")
       end
     end
 
@@ -2073,8 +2072,7 @@ def f
 
         expect(cli.run(['--autocorrect', '--format', 'simple', target_file])).to eq(1)
         expect($stdout.string.lines.to_a.last).to eq(
-          '1 file inspected, 2 offenses detected, 1 more offense can be corrected with ' \
-          "`rubocop -A`\n"
+          "1 file inspected, 2 offenses detected, 1 offense corrected\n"
         )
       end
     end

spec/rubocop/cop/lint/useless_assignment_spec.rb

@@ -321,9 +321,7 @@ module x::Foo
         ^^^ Useless assignment to variable - `foo`. Use `||` instead of `||=`.
       RUBY
 
-      expect_correction(<<~RUBY)
-        foo || 1
-      RUBY
+      expect_no_corrections
     end
   end
 
@@ -1149,12 +1147,7 @@ def some_method
         end
       RUBY
 
-      expect_correction(<<~RUBY)
-        def some_method
-          foo = do_something_returns_object_or_nil
-          foo || 1
-        end
-      RUBY
+      expect_no_corrections
     end
   end
 
@@ -1169,13 +1162,7 @@ def some_method
         end
       RUBY
 
-      expect_correction(<<~RUBY)
-        def some_method
-          foo = do_something_returns_object_or_nil
-          foo || 1
-          some_return_value
-        end
-      RUBY
+      expect_no_corrections
     end
   end