diff --git a/CHANGELOG.md b/CHANGELOG.md index bc3e9b8565c..312143af6f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * [#8232](https://github.com/rubocop-hq/rubocop/issues/8232): Fix a false positive for `Layout/EmptyLinesAroundAccessModifier` when `end` immediately after access modifier. ([@koic][]) * [#7777](https://github.com/rubocop-hq/rubocop/issues/7777): Fix crash for `Layout/MultilineArrayBraceLayout` when comment is present after last element. ([@shekhar-patil][]) * [#7776](https://github.com/rubocop-hq/rubocop/issues/7776): Fix crash for `Layout/MultilineMethodCallBraceLayout` when comment is present before closing braces. ([@shekhar-patil][]) +* [#8282](https://github.com/rubocop-hq/rubocop/issues/8282): Fix `Style/IfUnlessModifier` bad precedence detection. ([@tejasbubane][]) ## 0.87.1 (2020-07-07) diff --git a/lib/rubocop/cop/style/if_unless_modifier.rb b/lib/rubocop/cop/style/if_unless_modifier.rb index 96a7b3d9dda..72a1c3e1f7c 100644 --- a/lib/rubocop/cop/style/if_unless_modifier.rb +++ b/lib/rubocop/cop/style/if_unless_modifier.rb @@ -41,9 +41,6 @@ class IfUnlessModifier < Cop MSG_USE_NORMAL = 'Modifier form of `%s` makes the line too long.' - ASSIGNMENT_TYPES = %i[lvasgn casgn cvasgn - gvasgn ivasgn masgn].freeze - def on_if(node) msg = if single_line_as_modifier?(node) MSG_USE_MODIFIER unless named_capture_in_condition?(node) @@ -155,8 +152,9 @@ def parenthesize?(node) # Parenthesize corrected expression if changing to modifier-if form # would change the meaning of the parent expression # (due to the low operator precedence of modifier-if) - return false if node.parent.nil? - return true if ASSIGNMENT_TYPES.include?(node.parent.type) + parent = node.parent + return false if parent.nil? + return true if parent.assignment? || parent.operator_keyword? node.parent.send_type? && !node.parent.parenthesized? end diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index a9f5ffa0826..5bcb14da619 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -445,6 +445,32 @@ def f expect(corrected).to eq "a + (1 if b)\n" end + it 'adds parens in autocorrect when if-end used with `||` operator' do + expect_offense(<<~RUBY) + a || if b + ^^ Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. + 1 + end + RUBY + + expect_correction(<<~RUBY) + a || (1 if b) + RUBY + end + + it 'adds parens in autocorrect when if-end used with `&&` operator' do + expect_offense(<<~RUBY) + a && if b + ^^ Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. + 1 + end + RUBY + + expect_correction(<<~RUBY) + a && (1 if b) + RUBY + end + it 'accepts if-end when used as LHS of binary arithmetic' do expect_no_offenses(<<~RUBY) if test