Avoid infinite loop when autocorrecting with Style/IfUnlessModifier

Style/IfUnlessModifier can autocorrect long lines which use an if/unless modifier to
'if...end' form. It can also autocorrect 'if...end' form to modifier form if the body
is only a single line.

Obviously, there is the potential for an infinite autocorrect loop here. The cop tried
to avoid that by checking whether the 'if...end' conditional would overflow the maximum
length of a line if converted to modifier form.

However, the check is based only on the size of the conditional code itself and not
*other* statements which may follow after 'end', like this:

    if condition
      do_this
    end; other_statement; yet_another

This (hideous) code could result from an autocorrect of something like this:

    do_this if condition; other_statement; yet_another

So the cop still can and does cause infinite loops. A simple solution is to avoid
flagging long lines which use if/unless modifiers *if* there are other statements
following on the same line. Of course, such long lines are still a problem; but we can
allow Layout/LineLength to flag them rather than Style/IfUnlessModifier.

CHANGELOG.md

@@ -20,6 +20,7 @@
 * [#7834](https://github.com/rubocop-hq/rubocop/issues/7834): Fix `Lint/UriRegexp` to register offense with array arguments. ([@tejasbubane][])
 * [#7841](https://github.com/rubocop-hq/rubocop/issues/7841): Fix an error for `Style/TrailingCommaInBlockArgs` when lambda literal (`->`) has multiple arguments. ([@koic][])
 * [#7842](https://github.com/rubocop-hq/rubocop/issues/7842): Fix a false positive for `Lint/RaiseException` when Exception without cbase specified under the namespace `Gem` by adding  `AllowedImplicitNamespaces` option. ([@koic][])
+* `Style/IfUnlessModifier` does not infinite-loop when autocorrecting long lines which use if/unless modifiers and have multiple statements separated by semicolons. ([@alexdowad][])
 
 ### Changes
 

lib/rubocop/ast/node.rb

@@ -116,7 +116,7 @@ def updated(type = nil, children = nil, properties = {})
       #
       # @return [Integer] the index of the receiver node in its siblings
       def sibling_index
-        parent.children.index { |sibling| sibling.equal?(self) }
+        parent&.children&.index { |sibling| sibling.equal?(self) }
       end
 
       # Common destructuring method. This can be used to normalize

lib/rubocop/cop/style/if_unless_modifier.rb

@@ -47,7 +47,7 @@ class IfUnlessModifier < Cop
         def on_if(node)
           msg = if eligible_node?(node)
                   MSG_USE_MODIFIER unless named_capture_in_condition?(node)
-                elsif node.modifier_form? && too_long_single_line?(node)
+                elsif too_long_due_to_modifier?(node)
                   MSG_USE_NORMAL
                 end
           return unless msg
@@ -68,6 +68,11 @@ def autocorrect(node)
 
         private
 
+        def too_long_due_to_modifier?(node)
+          node.modifier_form? && too_long_single_line?(node) &&
+            !another_statement_on_same_line?(node)
+        end
+
         def ignored_patterns
           config.for_cop('Layout/LineLength')['IgnoredPatterns'] || []
         end
@@ -129,6 +134,21 @@ def non_eligible_if?(node)
           node.ternary? || node.modifier_form? || node.elsif? || node.else?
         end
 
+        def another_statement_on_same_line?(node)
+          line_no = node.source_range.last_line
+
+          # traverse the AST upwards until we find a 'begin' node
+          # we want to look at the following child and see if it is on the
+          #   same line as this 'if' node
+          while node && !node.begin_type?
+            index = node.sibling_index
+            node  = node.parent
+          end
+
+          node && (sibling = node.children[index + 1]) &&
+            sibling.source_range.first_line == line_no
+        end
+
         def parenthesize?(node)
           # Parenthesize corrected expression if changing to modifier-if form
           # would change the meaning of the parent expression

spec/rubocop/cop/style/if_unless_modifier_spec.rb

@@ -185,6 +185,24 @@ def f
     end
   end
 
+  context 'modifier if that does not fit on one line, but is not the only' \
+          ' statement on the line' do
+    let(:spaces) { ' ' * 59 }
+    let(:source) { "puts '#{spaces}' if condition; some_method_call" }
+
+    # long lines which have multiple statements on the same line can be flagged
+    #   by Layout/LineLength, Style/Semicolon, etc.
+    # if they are handled by Style/IfUnlessModifier, there is a danger of
+    #   creating infinite autocorrect loops when autocorrecting
+    it 'accepts' do
+      expect_no_offenses(<<~RUBY)
+        def f
+          #{source}
+        end
+      RUBY
+    end
+  end
+
   context 'multiline if that fits on one line with comment on first line' do
     let(:source) do
       <<~RUBY