[Fix #10024] Fix an incorrect auto-correct for Style/RedundantSelfAssignmentBranch

changelog/fix_incorrect_autocorrect_for_redundant_self_assignment_branch.md

@@ -0,0 +1 @@
+* [#10024](https://github.com/rubocop/rubocop/issues/10024): Fix an incorrect auto-correct for `Style/RedundantSelfAssignmentBranch` when using multiline `if` / `else` conditional assignment. ([@koic][])

lib/rubocop/cop/style/redundant_self_assignment_branch.rb

@@ -35,11 +35,11 @@ class RedundantSelfAssignmentBranch < Base
 
         def on_lvasgn(node)
           variable, expression = *node
-          return unless expression&.if_type?
-          return unless expression.ternary? || expression.else?
+          return unless use_if_and_else_branch?(expression)
 
           if_branch = expression.if_branch
           else_branch = expression.else_branch
+          return if inconvertible_to_modifier?(if_branch, else_branch)
 
           if self_assign?(variable, if_branch)
             register_offense(expression, if_branch, else_branch, 'unless')
@@ -50,35 +50,31 @@ def on_lvasgn(node)
 
         private
 
-        def self_assign?(variable, branch)
-          variable.to_s == branch&.source
+        def use_if_and_else_branch?(expression)
+          return false unless expression&.if_type?
+
+          !expression.ternary? || !expression.else?
         end
 
-        def register_offense(if_node, offense_branch, opposite_branch, keyword)
-          add_offense(offense_branch) do |corrector|
-            if if_node.ternary?
-              replacement = "#{opposite_branch.source} #{keyword} #{if_node.condition.source}"
-              corrector.replace(if_node, replacement)
-            else
-              if_node_loc = if_node.loc
+        def inconvertible_to_modifier?(if_branch, else_branch)
+          multiple_statements?(if_branch) || multiple_statements?(else_branch) ||
+            else_branch.respond_to?(:elsif?) && else_branch.elsif?
+        end
 
-              range = range_by_whole_lines(offense_branch.source_range, include_final_newline: true)
-              corrector.remove(range)
-              range = range_by_whole_lines(if_node_loc.else, include_final_newline: true)
-              corrector.remove(range)
+        def multiple_statements?(branch)
+          branch && branch.children.compact.count > 1
+        end
 
-              autocorrect_if_condition(corrector, if_node, if_node_loc, keyword)
-            end
-          end
+        def self_assign?(variable, branch)
+          variable.to_s == branch&.source
         end
 
-        def autocorrect_if_condition(corrector, if_node, if_node_loc, keyword)
-          else_branch = if_node.else_branch
+        def register_offense(if_node, offense_branch, opposite_branch, keyword)
+          add_offense(offense_branch) do |corrector|
+            assignment_value = opposite_branch ? opposite_branch.source : 'nil'
+            replacement = "#{assignment_value} #{keyword} #{if_node.condition.source}"
 
-          if else_branch.respond_to?(:elsif?) && else_branch.elsif?
-            corrector.replace(if_node.condition, else_branch.condition.source)
-          else
-            corrector.replace(if_node_loc.keyword, keyword)
+            corrector.replace(if_node, replacement)
           end
         end
       end

spec/rubocop/cop/style/redundant_self_assignment_branch_spec.rb

@@ -34,9 +34,7 @@
     RUBY
 
     expect_correction(<<~RUBY)
-      foo = if condition
-              bar
-            end
+      foo = bar if condition
     RUBY
   end
 
@@ -51,48 +49,30 @@
     RUBY
 
     expect_correction(<<~RUBY)
-      foo = unless condition
-              bar
-            end
+      foo = bar unless condition
     RUBY
   end
 
-  it 'registers and corrects an offense when self-assigning redundant else branch and multiline if branch' do
-    expect_offense(<<~RUBY)
+  it 'does not register an offense when self-assigning redundant else branch and multiline if branch' do
+    expect_no_offenses(<<~RUBY)
       foo = if condition
               bar
               baz
             else
               foo
-              ^^^ Remove the self-assignment branch.
-            end
-    RUBY
-
-    expect_correction(<<~RUBY)
-      foo = if condition
-              bar
-              baz
             end
     RUBY
   end
 
-  it 'registers and corrects an offense when self-assigning redundant else branch and multiline else branch' do
-    expect_offense(<<~RUBY)
+  it 'does not register an offense when self-assigning redundant else branch and multiline else branch' do
+    expect_no_offenses(<<~RUBY)
       foo = if condition
               foo
-              ^^^ Remove the self-assignment branch.
             else
               bar
               baz
             end
     RUBY
-
-    expect_correction(<<~RUBY)
-      foo = unless condition
-              bar
-              baz
-            end
-    RUBY
   end
 
   it 'registers and corrects an offense when self-assigning redundant else branch and empty if branch' do
@@ -105,8 +85,7 @@
     RUBY
 
     expect_correction(<<~RUBY)
-      foo = if condition
-            end
+      foo = nil if condition
     RUBY
   end
 
@@ -120,8 +99,7 @@
     RUBY
 
     expect_correction(<<~RUBY)
-      foo = unless condition
-            end
+      foo = nil unless condition
     RUBY
   end
 
@@ -170,30 +148,19 @@
     RUBY
   end
 
-  it 'registers and corrects an offense when using `elsif` and self-assigning the value of `then` branch' do
-    expect_offense(<<~RUBY)
+  it 'does not register an offense when using `elsif` and self-assigning the value of `then` branch' do
+    expect_no_offenses(<<~RUBY)
       foo = if condition
         foo
-        ^^^ Remove the self-assignment branch.
       elsif another_condtion
         bar
       else
         baz
       end
     RUBY
-
-    expect_correction(<<~RUBY)
-      foo = if another_condtion
-        bar
-      else
-        baz
-      end
-    RUBY
   end
 
-  # It may be possible to extend it to register an offense in future.
-  # auto-correction test patterns should be considered and implemented.
-  it 'registers and corrects an offense when using `elsif` and self-assigning the value of `elsif` branch' do
+  it 'does not register an offense when using `elsif` and self-assigning the value of `elsif` branch' do
     expect_no_offenses(<<~RUBY)
       foo = if condition
               bar
@@ -205,9 +172,7 @@
     RUBY
   end
 
-  # It may be possible to extend it to register an offense in future.
-  # auto-correction test patterns should be considered and implemented.
-  it 'registers and corrects an offense when using `elsif` and self-assigning the value of `else` branch' do
+  it 'does not register an offense when using `elsif` and self-assigning the value of `else` branch' do
     expect_no_offenses(<<~RUBY)
       foo = if condition
               bar