From 426e30fc299b4195f08aef65ea2d7e018ca1aab5 Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Fri, 30 Apr 2021 12:27:36 +0900
Subject: [PATCH] [Fix #9750] Fix an incorrect auto-correct for
 `Style/SoleNestedConditional`

Fixes #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.
---
 ...t_for_style_sole_nested_conditional_cop.md |  1 +
 .../cop/style/sole_nested_conditional.rb      | 19 +++++++++++++-----
 spec/rubocop/cli/autocorrect_spec.rb          | 20 +++++++++++++++++++
 .../cop/style/sole_nested_conditional_spec.rb | 18 +++++++++++++++++
 4 files changed, 53 insertions(+), 5 deletions(-)
 create mode 100644 changelog/fix_an_incorrect_autocorrect_for_style_sole_nested_conditional_cop.md

diff --git a/changelog/fix_an_incorrect_autocorrect_for_style_sole_nested_conditional_cop.md b/changelog/fix_an_incorrect_autocorrect_for_style_sole_nested_conditional_cop.md
new file mode 100644
index 00000000000..0c4ea5b1273
--- /dev/null
+++ b/changelog/fix_an_incorrect_autocorrect_for_style_sole_nested_conditional_cop.md
@@ -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][])
diff --git a/lib/rubocop/cop/style/sole_nested_conditional.rb b/lib/rubocop/cop/style/sole_nested_conditional.rb
index 22822c22dfc..fe40a9eea35 100644
--- a/lib/rubocop/cop/style/sole_nested_conditional.rb
+++ b/lib/rubocop/cop/style/sole_nested_conditional.rb
@@ -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?
@@ -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)
@@ -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)
diff --git a/spec/rubocop/cli/autocorrect_spec.rb b/spec/rubocop/cli/autocorrect_spec.rb
index 2ddb77aa0c7..685a5ac52b2 100644
--- a/spec/rubocop/cli/autocorrect_spec.rb
+++ b/spec/rubocop/cli/autocorrect_spec.rb
@@ -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
diff --git a/spec/rubocop/cop/style/sole_nested_conditional_spec.rb b/spec/rubocop/cop/style/sole_nested_conditional_spec.rb
index a223f2c6ba1..ea7caa360fa 100644
--- a/spec/rubocop/cop/style/sole_nested_conditional_spec.rb
+++ b/spec/rubocop/cop/style/sole_nested_conditional_spec.rb
@@ -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