Skip to content

Commit

Permalink
Merge pull request rubocop#8287 from tejasbubane/fix-8282
Browse files Browse the repository at this point in the history
Fix `Style/IfUnlessModifier` bad precedence detection
  • Loading branch information
koic committed Jul 9, 2020
2 parents 2f184b5 + e5a2e26 commit e80b278
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down
8 changes: 3 additions & 5 deletions lib/rubocop/cop/style/if_unless_modifier.rb
Expand Up @@ -41,9 +41,6 @@ class IfUnlessModifier < Cop
MSG_USE_NORMAL =
'Modifier form of `%<keyword>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)
Expand Down Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions spec/rubocop/cop/style/if_unless_modifier_spec.rb
Expand Up @@ -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
Expand Down

0 comments on commit e80b278

Please sign in to comment.