[Fix rubocop#7709] wrap ranges in parenthese when correcting Redundan…

…tCondition
rrosenblum · Feb 21, 2020 · d1a19c5 · d1a19c5
1 parent 5986381
commit d1a19c5
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Bug fixes
 
 * [#7719](https://github.com/rubocop-hq/rubocop/issues/7719): Fix `Style/NestedParenthesizedCalls` cop for newline. ([@tejasbubane][])
+* [#7709](https://github.com/rubocop-hq/rubocop/issues/7709): Fix correction of `Style/RedundantCondition` when the else branch contains a range. ([@rrosenblum][])
 
 ## 0.80.0 (2020-02-18)
 

diff --git a/lib/rubocop/cop/style/redundant_condition.rb b/lib/rubocop/cop/style/redundant_condition.rb
@@ -46,7 +46,7 @@ def on_if(node)
         def autocorrect(node)
           lambda do |corrector|
             if node.ternary?
-              corrector.replace(range_of_offense(node), '||')
+              correct_ternary(corrector, node)
             elsif node.modifier_form? || !node.else_branch
               corrector.replace(node.source_range, node.if_branch.source)
             else
@@ -90,9 +90,13 @@ def use_if_branch?(else_branch)
         end
 
         def else_source(else_branch)
-          wrap_else =
-            else_branch.basic_conditional? && else_branch.modifier_form?
-          wrap_else ? "(#{else_branch.source})" : else_branch.source
+          if else_branch.basic_conditional? &&
+             else_branch.modifier_form? ||
+             else_branch.range_type?
+            "(#{else_branch.source})"
+          else
+            else_branch.source
+          end
         end
 
         def make_ternary_form(node)
@@ -106,6 +110,15 @@ def make_ternary_form(node)
             ternary_form
           end
         end
+
+        def correct_ternary(corrector, node)
+          corrector.replace(range_of_offense(node), '||')
+
+          return unless node.else_branch.range_type?
+
+          corrector.insert_before(node.else_branch.loc.expression, '(')
+          corrector.insert_after(node.else_branch.loc.expression, ')')
+        end
       end
     end
   end

diff --git a/spec/rubocop/cop/style/redundant_condition_spec.rb b/spec/rubocop/cop/style/redundant_condition_spec.rb
@@ -96,6 +96,22 @@
           RUBY
         end
       end
+
+      it 'registers an offense and corrects when the else branch ' \
+        'contains an irange' do
+        expect_offense(<<~RUBY)
+          if foo
+          ^^^^^^ Use double pipes `||` instead.
+            foo
+          else
+            1..2
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          foo || (1..2)
+        RUBY
+      end
     end
 
     describe '#autocorrection' do
@@ -201,6 +217,30 @@
             ^^^^^ Use double pipes `||` instead.
         RUBY
       end
+
+      it 'registers an offense and corrects when the else branch ' \
+        'contains an irange' do
+        expect_offense(<<~RUBY)
+          time_period = updated_during ? updated_during : 2.days.ago..Time.now
+                                       ^^^^^^^^^^^^^^^^^^ Use double pipes `||` instead.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          time_period = updated_during || (2.days.ago..Time.now)
+        RUBY
+      end
+
+      it 'registers an offense and corrects when the else branch ' \
+        'contains an erange' do
+        expect_offense(<<~RUBY)
+          time_period = updated_during ? updated_during : 2.days.ago...Time.now
+                                       ^^^^^^^^^^^^^^^^^^ Use double pipes `||` instead.
+        RUBY
+
+        expect_correction(<<~RUBY)
+          time_period = updated_during || (2.days.ago...Time.now)
+        RUBY
+      end
     end
 
     describe '#autocorrection' do