Skip to content

Commit

Permalink
[Fix #9152] Fix an incorrect auto-correct for `Style/SoleNestedCondit…
Browse files Browse the repository at this point in the history
…ional`

Fixes #9152.

This PR fixes an incorrect auto-correct for `Style/SoleNestedConditional`
when nested `||` operator modifier condition.
  • Loading branch information
koic authored and bbatsov committed Dec 3, 2020
1 parent 4336f72 commit 1b9df89
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#9152](https://github.com/rubocop-hq/rubocop/issues/9152): Fix an incorrect auto-correct for `Style/SoleNestedConditional` when nested `||` operator modifier condition. ([@koic][])
18 changes: 14 additions & 4 deletions lib/rubocop/cop/style/sole_nested_conditional.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ def autocorrect(corrector, node, if_branch)
corrector.insert_before(node.condition, '!')
end

corrector.wrap(node.condition, '(', ')') if node.condition.or_type?

and_operator = if_branch.unless? ? ' && !' : ' && '
if if_branch.modifier_form?
correct_for_gurad_condition_style(corrector, node, if_branch, and_operator)
Expand All @@ -79,11 +81,10 @@ def autocorrect(corrector, node, if_branch)
end

def correct_for_gurad_condition_style(corrector, node, if_branch, and_operator)
corrector.insert_after(node.condition, "#{and_operator}#{if_branch.condition.source}")
condition = if_branch.condition
corrector.insert_after(node.condition, replacement_condition(and_operator, condition))

range = range_between(
if_branch.loc.keyword.begin_pos, if_branch.condition.source_range.end_pos
)
range = range_between(if_branch.loc.keyword.begin_pos, condition.source_range.end_pos)
corrector.remove(range_with_surrounding_space(range: range, newlines: false))
corrector.remove(if_branch.loc.keyword)
end
Expand All @@ -94,6 +95,7 @@ def correct_for_basic_condition_style(corrector, node, if_branch, and_operator)
)
corrector.replace(range, and_operator)
corrector.remove(range_by_whole_lines(node.loc.end, include_final_newline: true))
corrector.wrap(if_branch.condition, '(', ')') if if_branch.condition.or_type?
end

def correct_for_comment(corrector, node, if_branch)
Expand All @@ -103,6 +105,14 @@ def correct_for_comment(corrector, node, if_branch)
corrector.insert_before(node.loc.keyword, comment_text) unless comments.empty?
end

def replacement_condition(and_operator, condition)
if condition.or_type?
"#{and_operator}(#{condition.source})"
else
"#{and_operator}#{condition.source}"
end
end

def allow_modifier?
cop_config['AllowModifier']
end
Expand Down
65 changes: 65 additions & 0 deletions spec/rubocop/cop/style/sole_nested_conditional_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,71 @@
RUBY
end

it 'registers an offense and corrects when nested `||` operator condition' do
expect_offense(<<~RUBY)
if foo
if bar || baz
^^ Consider merging nested conditions into outer `if` conditions.
do_something
end
end
RUBY

expect_correction(<<~RUBY)
if foo && (bar || baz)
do_something
end
RUBY
end

it 'registers an offense and corrects when nested `||` operator modifier condition' do
expect_offense(<<~RUBY)
if foo
do_something if bar || baz
^^ Consider merging nested conditions into outer `if` conditions.
end
RUBY

expect_correction(<<~RUBY)
if foo && (bar || baz)
do_something
end
RUBY
end

it 'registers an offense and corrects when using `||` in the outer condition' do
expect_offense(<<~RUBY)
if foo || bar
if baz || qux
^^ Consider merging nested conditions into outer `if` conditions.
do_something
end
end
RUBY

expect_correction(<<~RUBY)
if (foo || bar) && (baz || qux)
do_something
end
RUBY
end

it 'registers an offense and corrects when using `||` in the outer condition' \
'and nested modifier condition ' do
expect_offense(<<~RUBY)
if foo || bar
do_something if baz || qux
^^ Consider merging nested conditions into outer `if` conditions.
end
RUBY

expect_correction(<<~RUBY)
if (foo || bar) && (baz || qux)
do_something
end
RUBY
end

it 'registers an offense and corrects for multiple nested conditionals' do
expect_offense(<<~RUBY)
if foo
Expand Down

0 comments on commit 1b9df89

Please sign in to comment.