Skip to content

Commit

Permalink
[Fixes #8273] Fix false positive for Style/WhileUntilModifier.
Browse files Browse the repository at this point in the history
Refactor that condition
  • Loading branch information
marcandre committed Jul 9, 2020
1 parent c97671b commit 9f4382c
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 20 deletions.
6 changes: 3 additions & 3 deletions lib/rubocop/cop/mixin/statement_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ def single_line_as_modifier?(node)
end

def non_eligible_node?(node)
node.nonempty_line_count > 3 ||
!node.modifier_form? &&
processed_source.commented?(node.loc.end)
node.modifier_form? ||
node.nonempty_line_count > 3 ||
processed_source.commented?(node.loc.end)
end

def non_eligible_body?(body)
Expand Down
14 changes: 8 additions & 6 deletions lib/rubocop/cop/style/if_unless_modifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class IfUnlessModifier < Cop
gvasgn ivasgn masgn].freeze

def on_if(node)
msg = if eligible_node?(node)
msg = if single_line_as_modifier?(node)
MSG_USE_MODIFIER unless named_capture_in_condition?(node)
elsif too_long_due_to_modifier?(node)
MSG_USE_NORMAL
Expand Down Expand Up @@ -125,13 +125,15 @@ def named_capture_in_condition?(node)
node.condition.match_with_lvasgn_type?
end

def eligible_node?(node)
!non_eligible_if?(node) && !node.chained? &&
!node.nested_conditional? && single_line_as_modifier?(node)
def non_eligible_node?(node)
non_simple_if_unless?(node) ||
node.chained? ||
node.nested_conditional? ||
super
end

def non_eligible_if?(node)
node.ternary? || node.modifier_form? || node.elsif? || node.else?
def non_simple_if_unless?(node)
node.ternary? || node.elsif? || node.else?
end

def another_statement_on_same_line?(node)
Expand Down
11 changes: 0 additions & 11 deletions spec/rubocop/cop/style/while_until_modifier_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,4 @@
RSpec.describe RuboCop::Cop::Style::WhileUntilModifier, :config do
it_behaves_like 'condition modifier cop', :while
it_behaves_like 'condition modifier cop', :until

# Regression: https://github.com/rubocop-hq/rubocop/issues/4006
context 'when the modifier condition is multiline' do
it 'registers an offense' do
expect_offense(<<~RUBY)
foo while bar ||
^^^^^ Favor modifier `while` usage when having a single-line body.
baz
RUBY
end
end
end
10 changes: 10 additions & 0 deletions spec/support/condition_modifier_cop.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,15 @@
x = 0 #{keyword} true
RUBY
end

# See: https://github.com/rubocop-hq/rubocop/issues/8273
context 'accepts multiline condition in modifier form' do
it 'registers an offense' do
expect_no_offenses(<<~RUBY)
foo #{keyword} bar ||
baz
RUBY
end
end
end
end

0 comments on commit 9f4382c

Please sign in to comment.