From d4da6336d1c4cc8accede2fbf26f64fd9867e2c8 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Fri, 10 Jul 2020 23:01:55 +0900 Subject: [PATCH] [Fix #8299] Fix an incorrect auto-correct for `Style/RedundantCondition` Fixes #8299 This PR fixes an incorrect auto-correct for `Style/RedundantCondition` when using `raise`, `rescue`, or `and` without argument parentheses in `else`. --- CHANGELOG.md | 1 + lib/rubocop/cop/style/redundant_condition.rb | 18 +++++- .../cop/style/redundant_condition_spec.rb | 60 +++++++++++++++++++ 3 files changed, 76 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57d8ff82670..78062cab6b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * [#8324](https://github.com/rubocop-hq/rubocop/issues/8324): Fix crash for `Layout/SpaceAroundMethodCallOperator` when using `Proc#call` shorthand syntax. ([@fatkodima][]) * [#8323](https://github.com/rubocop-hq/rubocop/issues/8323): Fix a false positive for `Style/HashAsLastArrayItem` when hash is not a last array item. ([@fatkodima][]) +* [#8299](https://github.com/rubocop-hq/rubocop/issues/8299): Fix an incorrect auto-correct for `Style/RedundantCondition` when using `raise`, `rescue`, or `and` without argument parentheses in `else`. ([@koic][]) ## 0.88.0 (2020-07-13) diff --git a/lib/rubocop/cop/style/redundant_condition.rb b/lib/rubocop/cop/style/redundant_condition.rb index 0fdbc6027aa..065293b82e1 100644 --- a/lib/rubocop/cop/style/redundant_condition.rb +++ b/lib/rubocop/cop/style/redundant_condition.rb @@ -90,10 +90,10 @@ def use_if_branch?(else_branch) end def else_source(else_branch) - if else_branch.basic_conditional? && - else_branch.modifier_form? || - else_branch.range_type? + if require_parentheses?(else_branch) "(#{else_branch.source})" + elsif without_argument_parentheses_method?(else_branch) + "#{else_branch.method_name}(#{else_branch.arguments.map(&:source).join(', ')})" else else_branch.source end @@ -118,6 +118,18 @@ def correct_ternary(corrector, node) corrector.wrap(node.else_branch, '(', ')') end + + def require_parentheses?(node) + node.basic_conditional? && + node.modifier_form? || + node.range_type? || + node.rescue_type? || + node.respond_to?(:semantic_operator?) && node.semantic_operator? + end + + def without_argument_parentheses_method?(node) + node.send_type? && !node.arguments.empty? && !node.parenthesized? + end end end end diff --git a/spec/rubocop/cop/style/redundant_condition_spec.rb b/spec/rubocop/cop/style/redundant_condition_spec.rb index 457f421d063..989e4eb8333 100644 --- a/spec/rubocop/cop/style/redundant_condition_spec.rb +++ b/spec/rubocop/cop/style/redundant_condition_spec.rb @@ -42,6 +42,36 @@ RUBY end + it 'registers an offense and corrects when `raise` without argument parentheses in `else`' do + expect_offense(<<~RUBY) + if b + ^^^^ Use double pipes `||` instead. + b + else + raise 'foo' + end + RUBY + + expect_correction(<<~RUBY) + b || raise('foo') + RUBY + end + + it 'registers an offense and corrects when a method without argument parentheses in `else`' do + expect_offense(<<~RUBY) + if b + ^^^^ Use double pipes `||` instead. + b + else + do_something foo, bar, key: :value + end + RUBY + + expect_correction(<<~RUBY) + b || do_something(foo, bar, key: :value) + RUBY + end + it 'registers an offense and corrects complex one liners' do expect_offense(<<~RUBY) if b @@ -252,6 +282,36 @@ time_period = updated_during || (2.days.ago...Time.now) RUBY end + + it 'registers an offense and corrects when the else branch contains `rescue`' do + expect_offense(<<~RUBY) + if a + ^^^^ Use double pipes `||` instead. + a + else + b rescue c + end + RUBY + + expect_correction(<<~RUBY) + a || (b rescue c) + RUBY + end + + it 'registers an offense and corrects when the else branch contains `and`' do + expect_offense(<<~RUBY) + if a + ^^^^ Use double pipes `||` instead. + a + else + b and c + end + RUBY + + expect_correction(<<~RUBY) + a || (b and c) + RUBY + end end end