[Fix #10436] Fix an error for Style/SoleNestedConditional raises exception when inspecting if ... end if ...

changelog/fix_an_error_style_sole_nested_conditional.md

@@ -0,0 +1 @@
+* [#10447](https://github.com/rubocop/rubocop/pull/10447): Fix an error for `Style/SoleNestedConditional` raises exception when inspecting `if ... end if ...`. ([@ydah][])

lib/rubocop/cop/style/sole_nested_conditional.rb

@@ -15,6 +15,11 @@ module Style
       #     end
       #   end
       #
+      #   # bad
+      #   if condition_b
+      #     do_something
+      #   end if condition_a
+      #
       #   # good
       #   if condition_a && condition_b
       #     do_something
@@ -26,12 +31,21 @@ module Style
       #     do_something if condition_b
       #   end
       #
+      #   # bad
+      #   if condition_b
+      #     do_something
+      #   end if condition_a
+      #
       # @example AllowModifier: true
       #   # good
       #   if condition_a
       #     do_something if condition_b
       #   end
       #
+      #   # good
+      #   if condition_b
+      #     do_something
+      #   end if condition_a
       class SoleNestedConditional < Base
         include RangeHelp
         extend AutoCorrector
@@ -47,7 +61,7 @@ def on_if(node)
 
           if_branch = node.if_branch
           return if use_variable_assignment_in_condition?(node.condition, if_branch)
-          return unless offending_branch?(if_branch)
+          return unless offending_branch?(node, if_branch)
 
           message = format(MSG, conditional_type: node.keyword)
           add_offense(if_branch.loc.keyword, message: message) do |corrector|
@@ -72,20 +86,28 @@ def assigned_variables(condition)
           end
         end
 
-        def offending_branch?(branch)
+        def offending_branch?(node, branch)
           return false unless branch
 
           branch.if_type? &&
             !branch.else? &&
             !branch.ternary? &&
-            !(branch.modifier_form? && allow_modifier?)
+            !((node.modifier_form? || branch.modifier_form?) && allow_modifier?)
         end
 
         def autocorrect(corrector, node, if_branch)
           if node.condition.or_type? || node.condition.assignment?
             corrector.wrap(node.condition, '(', ')')
           end
 
+          if outer_condition_modify_form?(node, if_branch)
+            autocorrect_outer_condition_modify_form(corrector, node, if_branch)
+          else
+            autocorrect_outer_condition_basic(corrector, node, if_branch)
+          end
+        end
+
+        def autocorrect_outer_condition_basic(corrector, node, if_branch)
           correct_from_unless_to_if(corrector, node) if node.unless?
 
           and_operator = if_branch.unless? ? ' && !' : ' && '
@@ -97,11 +119,17 @@ def autocorrect(corrector, node, if_branch)
           end
         end
 
-        def correct_from_unless_to_if(corrector, node)
+        def autocorrect_outer_condition_modify_form(corrector, node, if_branch)
+          correct_from_unless_to_if(corrector, if_branch, is_modify_form: true) if if_branch.unless?
+          correct_for_outer_condition_modify_form_style(corrector, node, if_branch)
+        end
+
+        def correct_from_unless_to_if(corrector, node, is_modify_form: false)
           corrector.replace(node.loc.keyword, 'if')
 
           condition = node.condition
-          if condition.send_type? && condition.comparison_method? && !condition.parenthesized?
+          if (condition.send_type? && condition.comparison_method? && !condition.parenthesized?) ||
+             (is_modify_form && wrap_condition?(condition))
             corrector.wrap(node.condition, '!(', ')')
           else
             corrector.insert_before(node.condition, '!')
@@ -113,7 +141,7 @@ def correct_for_guard_condition_style(corrector, node, if_branch, and_operator)
           correct_outer_condition(corrector, outer_condition)
 
           condition = if_branch.condition
-          corrector.insert_after(outer_condition, replacement_condition(and_operator, condition))
+          corrector.insert_after(outer_condition, "#{and_operator}#{replace_condition(condition)}")
 
           range = range_between(if_branch.loc.keyword.begin_pos, condition.source_range.end_pos)
           corrector.remove(range_with_surrounding_space(range: range, newlines: false))
@@ -129,6 +157,16 @@ def correct_for_basic_condition_style(corrector, node, if_branch, and_operator)
           corrector.wrap(if_branch.condition, '(', ')') if wrap_condition?(if_branch.condition)
         end
 
+        def correct_for_outer_condition_modify_form_style(corrector, node, if_branch)
+          condition = if_branch.condition
+          corrector.insert_before(condition,
+                                  "#{'!' if node.unless?}#{replace_condition(node.condition)} && ")
+
+          corrector.remove(node.condition.loc.expression)
+          corrector.remove(range_with_surrounding_space(range: node.loc.keyword, newlines: false))
+          corrector.replace(if_branch.loc.keyword, 'if')
+        end
+
         def correct_for_comment(corrector, node, if_branch)
           return if config.for_cop('Style/IfUnlessModifier')['Enabled']
 
@@ -165,17 +203,17 @@ def wrap_condition?(node)
             (node.send_type? && node.arguments.any? && !node.parenthesized?)
         end
 
-        def replacement_condition(and_operator, condition)
-          if wrap_condition?(condition)
-            "#{and_operator}(#{condition.source})"
-          else
-            "#{and_operator}#{condition.source}"
-          end
+        def replace_condition(condition)
+          wrap_condition?(condition) ? "(#{condition.source})" : condition.source
         end
 
         def allow_modifier?
           cop_config['AllowModifier']
         end
+
+        def outer_condition_modify_form?(node, if_branch)
+          node.condition.loc.expression.begin_pos > if_branch.condition.loc.expression.begin_pos
+        end
       end
     end
   end

spec/rubocop/cop/style/sole_nested_conditional_spec.rb

@@ -409,6 +409,126 @@ def foo
     RUBY
   end
 
+  it 'registers an offense and corrects when `if` foo do_something end `if` bar' do
+    expect_offense(<<~RUBY)
+      if foo
+      ^^ Consider merging nested conditions into outer `if` conditions.
+        do_something
+      end if bar
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if bar && foo
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `if` foo do_something end `unless` bar' do
+    expect_offense(<<~RUBY)
+      if foo
+      ^^ Consider merging nested conditions into outer `unless` conditions.
+        do_something
+      end unless bar
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if !bar && foo
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `unless` foo do_something end `if` bar' do
+    expect_offense(<<~RUBY)
+      unless foo
+      ^^^^^^ Consider merging nested conditions into outer `if` conditions.
+        do_something
+      end if bar
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if bar && !foo
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `if` foo do_something end `if` bar && baz' do
+    expect_offense(<<~RUBY)
+      if foo
+      ^^ Consider merging nested conditions into outer `if` conditions.
+        do_something
+      end if bar && baz
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if (bar && baz) && foo
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `if` foo && bar do_something end `if` baz' do
+    expect_offense(<<~RUBY)
+      if foo && bar
+      ^^ Consider merging nested conditions into outer `if` conditions.
+        do_something
+      end if baz
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if baz && foo && bar
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `if` foo do_something end `unless` bar && baz' do
+    expect_offense(<<~RUBY)
+      if foo
+      ^^ Consider merging nested conditions into outer `unless` conditions.
+        do_something
+      end unless bar && baz
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if !(bar && baz) && foo
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `if` foo && bar do_something end `unless` baz' do
+    expect_offense(<<~RUBY)
+      if foo && bar
+      ^^ Consider merging nested conditions into outer `unless` conditions.
+        do_something
+      end unless baz
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if !baz && foo && bar
+        do_something
+      end
+    RUBY
+  end
+
+  it 'registers an offense and corrects when `unless` foo && bar do_something end `if` baz' do
+    expect_offense(<<~RUBY)
+      unless foo && bar
+      ^^^^^^ Consider merging nested conditions into outer `if` conditions.
+        do_something
+      end if baz
+    RUBY
+
+    expect_correction(<<~RUBY)
+      if baz && !(foo && bar)
+        do_something
+      end
+    RUBY
+  end
+
   it 'does not register an offense when using nested ternary within conditional' do
     expect_no_offenses(<<~RUBY)
       if foo
@@ -557,7 +677,7 @@ def foo
       expect_no_offenses(<<~RUBY)
         if foo
           do_something if bar
-        end
+        end if baz
       RUBY
     end
   end