Merge pull request #10609 from nobuyo/fix-autocorrect-for-style-redundant-condition-with-parentheses

[Fix #10607] Fix autocorrect for `Style/RedundantCondition` with parenthesized method call

Author: koic

changelog/fix_autocorrect_for_style_redundant_condition_with_parenthesized_method_call.md

@@ -0,0 +1 @@
+* [#10607](https://github.com/rubocop/rubocop/issues/10607): Fix autocorrect for `Style/RedundantCondition` when there are parenthesized method calls in each branch. ([@nobuyo][])

lib/rubocop/cop/style/redundant_condition.rb

@@ -44,9 +44,9 @@ def on_if(node)
           message = message(node)
 
           add_offense(range_of_offense(node), message: message) do |corrector|
-            if node.ternary?
+            if node.ternary? && !branches_have_method?(node)
               correct_ternary(corrector, node)
-            elsif node.modifier_form? || !node.else_branch
+            elsif redudant_condition?(node)
               corrector.replace(node, node.if_branch.source)
             else
               corrected = make_ternary_form(node)
@@ -59,7 +59,7 @@ def on_if(node)
         private
 
         def message(node)
-          if node.modifier_form? || !node.else_branch
+          if redudant_condition?(node)
             REDUNDANT_CONDITION
           else
             MSG
@@ -68,6 +68,7 @@ def message(node)
 
         def range_of_offense(node)
           return node.loc.expression unless node.ternary?
+          return node.loc.expression if node.ternary? && branches_have_method?(node)
 
           range_between(node.loc.question.begin_pos, node.loc.colon.end_pos)
         end
@@ -81,6 +82,10 @@ def offense?(node)
             (node.ternary? || !else_branch.instance_of?(AST::Node) || else_branch.single_line?)
         end
 
+        def redudant_condition?(node)
+          node.modifier_form? || !node.else_branch
+        end
+
         def use_if_branch?(else_branch)
           else_branch&.if_type?
         end
@@ -89,6 +94,10 @@ def use_hash_key_assignment?(else_branch)
           else_branch&.send_type? && else_branch&.method?(:[]=)
         end
 
+        def use_hash_key_access?(node)
+          node.send_type? && node.method?(:[])
+        end
+
         def synonymous_condition_and_branch?(node)
           condition, if_branch, _else_branch = *node
           # e.g.
@@ -113,7 +122,8 @@ def synonymous_condition_and_branch?(node)
           #   else
           #     test.value = another_value?
           #   end
-          branches_have_method?(node) && condition == if_branch.first_argument
+          branches_have_method?(node) && condition == if_branch.first_argument &&
+            !use_hash_key_access?(if_branch)
         end
 
         def branches_have_assignment?(node)
@@ -140,6 +150,14 @@ def branches_have_method?(node)
             if_branch.method?(else_branch.method_name)
         end
 
+        def if_source(if_branch)
+          if branches_have_method?(if_branch.parent) && if_branch.parenthesized?
+            if_branch.source.delete_suffix(')')
+          else
+            if_branch.source
+          end
+        end
+
         def else_source(else_branch)
           if branches_have_method?(else_branch.parent)
             else_source_if_has_method(else_branch)
@@ -176,7 +194,8 @@ def else_source_if_has_assignment(else_branch)
 
         def make_ternary_form(node)
           _condition, if_branch, else_branch = *node
-          ternary_form = [if_branch.source, else_source(else_branch)].join(' || ')
+          ternary_form = [if_source(if_branch), else_source(else_branch)].join(' || ')
+          ternary_form += ')' if branches_have_method?(node) && if_branch.parenthesized?
 
           if node.parent&.send_type?
             "(#{ternary_form})"

spec/rubocop/cop/style/redundant_condition_spec.rb

@@ -322,6 +322,21 @@ def test
         RUBY
       end
 
+      it 'registers an offense and corrects when the branches contains parenthesized method call' do
+        expect_offense(<<~RUBY)
+          if foo
+          ^^^^^^ Use double pipes `||` instead.
+            bar(foo)
+          else
+            bar(1..2)
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          bar(foo || (1..2))
+        RUBY
+      end
+
       it 'does not register offenses when using `nil?` and the branches contains assignment' do
         expect_no_offenses(<<~RUBY)
           if foo.nil?
@@ -351,6 +366,16 @@ def test
           end
         RUBY
       end
+
+      it 'does not register offenses when the branches contains hash key access' do
+        expect_no_offenses(<<~RUBY)
+          if foo
+            bar[foo]
+          else
+            bar[1]
+          end
+        RUBY
+      end
     end
   end
 
@@ -426,6 +451,28 @@ def test
         RUBY
       end
 
+      it 'registers an offense and corrects with ternary expression and the branches contains parenthesized method call' do
+        expect_offense(<<~RUBY)
+          foo ? bar(foo) : bar(quux)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use double pipes `||` instead.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          bar(foo || quux)
+        RUBY
+      end
+
+      it 'registers an offense and corrects with ternary expression and the branches contains chained parenthesized method call' do
+        expect_offense(<<~RUBY)
+          foo ? foo(foo).bar(foo) : foo(foo).bar(quux)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use double pipes `||` instead.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo(foo).bar(foo || quux)
+        RUBY
+      end
+
       it 'registers an offense and corrects when the else branch contains `rescue`' do
         expect_offense(<<~RUBY)
           if a