Skip to content

Commit

Permalink
[Fix rubocop#9750] Fix an incorrect auto-correct for `Style/SoleNeste…
Browse files Browse the repository at this point in the history
…dConditional`

Fixes rubocop#9750.

This PR fixes an incorrect auto-correct for `Style/SoleNestedConditional`
when when using nested `if` within `unless foo == bar`.

This patch leaves auto-correction from `(!foo == bar)` to `foo != bar` to
`Style/InverseMethods` cop thus simplify its role and implementation.

And this PR adds a cli test to prevent unintended auto-correction when using
`Style/IfUnlessModifier` together for the patch.
  • Loading branch information
koic committed Apr 30, 2021
1 parent 74377b0 commit 426e30f
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 5 deletions.
@@ -0,0 +1 @@
* [#9750](https://github.com/rubocop/rubocop/issues/9750): Fix an incorrect auto-correct for `Style/SoleNestedConditional` when when using nested `if` within `unless foo == bar`. ([@koic][])
19 changes: 14 additions & 5 deletions lib/rubocop/cop/style/sole_nested_conditional.rb
Expand Up @@ -80,10 +80,7 @@ def offending_branch?(branch)
def autocorrect(corrector, node, if_branch)
corrector.wrap(node.condition, '(', ')') if node.condition.or_type?

if node.unless?
corrector.replace(node.loc.keyword, 'if')
corrector.insert_before(node.condition, '!')
end
correct_from_unless_to_if(corrector, node) if node.unless?

and_operator = if_branch.unless? ? ' && !' : ' && '
if if_branch.modifier_form?
Expand All @@ -94,6 +91,17 @@ def autocorrect(corrector, node, if_branch)
end
end

def correct_from_unless_to_if(corrector, node)
corrector.replace(node.loc.keyword, 'if')

condition = node.condition
if condition.send_type? && condition.comparison_method? && !condition.parenthesized?
corrector.wrap(node.condition, '!(', ')')
else
corrector.insert_before(node.condition, '!')
end
end

def correct_for_guard_condition_style(corrector, node, if_branch, and_operator)
outer_condition = node.condition
correct_outer_condition(corrector, outer_condition)
Expand Down Expand Up @@ -136,7 +144,8 @@ def correct_outer_condition(corrector, condition)
end

def requrie_parentheses?(condition)
condition.send_type? && !condition.arguments.empty? && !condition.parenthesized?
condition.send_type? && !condition.arguments.empty? && !condition.parenthesized? &&
!condition.comparison_method?
end

def arguments_range(node)
Expand Down
20 changes: 20 additions & 0 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -304,6 +304,26 @@ def foo
RUBY
end

it 'corrects `Style/SoleNestedConditional` with `Style/InverseMethods` and `Style/IfUnlessModifier`' do
source = <<~RUBY
unless foo.to_s == 'foo'
if condition
return foo
end
end
RUBY
create_file('example.rb', source)
expect(cli.run(
[
'--auto-correct-all',
'--only', 'Style/SoleNestedConditional,Style/InverseMethods,Style/IfUnlessModifier'
]
)).to eq(0)
expect(File.read('example.rb')).to eq(<<~RUBY)
return foo if foo.to_s != 'foo' && condition
RUBY
end

describe 'trailing comma cops' do
let(:source) do
<<~RUBY
Expand Down
18 changes: 18 additions & 0 deletions spec/rubocop/cop/style/sole_nested_conditional_spec.rb
Expand Up @@ -54,6 +54,24 @@
RUBY
end

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

# NOTE: `Style/InverseMethods` cop auto-corrects from `(!foo == bar)` to `foo != bar`.
expect_correction(<<~RUBY)
if !(foo == bar) && baz
do_something
end
RUBY
end

it 'registers an offense and corrects when using nested `unless` within `unless`' do
expect_offense(<<~RUBY)
unless foo
Expand Down

0 comments on commit 426e30f

Please sign in to comment.