[Fix #8621] Helpful Infinite Loop message

CHANGELOG.md

@@ -37,6 +37,7 @@
 * [#8617](https://github.com/rubocop-hq/rubocop/issues/8617): Fix `Style/HashAsLastArrayItem` to not register an offense when all items in an array are hashes. ([@dvandersluis][])
 * [#8500](https://github.com/rubocop-hq/rubocop/issues/8500): Add `in?` to AllowedMethods for `Lint/SafeNavigationChain` cop. ([@tejasbubane][])
 * [#8629](https://github.com/rubocop-hq/rubocop/pull/8629): Fix the cache being reusable in CI by using crc32 to calculate file hashes rather than `mtime`, which changes each CI build. ([@dvandersluis][])
+* [#8621](https://github.com/rubocop-hq/rubocop/issues/8621): Add helpful Infinite Loop error message. ([@iSarCasm][])
 
 ## 0.90.0 (2020-09-01)
 
@@ -4862,3 +4863,4 @@
 [@jaimerave]: https://github.com/jaimerave
 [@Skipants]: https://github.com/Skipants
 [@sascha-wolf]: https://github.com/sascha-wolf
+[@iSarCasm]: https://github.com/iSarCasm

lib/rubocop/runner.rb

@@ -11,9 +11,12 @@ class Runner # rubocop:disable Metrics/ClassLength
     class InfiniteCorrectionLoop < RuntimeError
       attr_reader :offenses
 
-      def initialize(path, offenses)
-        super "Infinite loop detected in #{path}."
-        @offenses = offenses
+      def initialize(path, offenses_by_iteration, loop_start: -1)
+        @offenses = offenses_by_iteration.flatten.uniq
+        root_cause = offenses_by_iteration[loop_start..-1]
+                     .map { |x| x.map(&:cop_name).uniq.join(', ') }
+                     .join(' -> ')
+        super "Infinite loop detected in #{path} and caused by #{root_cause}"
       end
     end
 
@@ -236,18 +239,21 @@ def save_in_cache(cache, offenses)
 
     def do_inspection_loop(file)
       processed_source = get_processed_source(file)
-      offenses = []
+      # This variable is 2d array used to track corrected offenses after each
+      # inspection iteration. This is used to output meaningful infinite loop
+      # error message.
+      offenses_by_iteration = []
 
       # When running with --auto-correct, we need to inspect the file (which
       # includes writing a corrected version of it) until no more corrections
       # are made. This is because automatic corrections can introduce new
       # offenses. In the normal case the loop is only executed once.
-      iterate_until_no_changes(processed_source, offenses) do
+      iterate_until_no_changes(processed_source, offenses_by_iteration) do
         # The offenses that couldn't be corrected will be found again so we
         # only keep the corrected ones in order to avoid duplicate reporting.
-        offenses.select!(&:corrected?)
+        !offenses_by_iteration.empty? && offenses_by_iteration.last.select!(&:corrected?)
         new_offenses, updated_source_file = inspect_file(processed_source)
-        offenses.concat(new_offenses).uniq!
+        offenses_by_iteration.push(new_offenses)
 
         # We have to reprocess the source to pickup the changes. Since the
         # change could (theoretically) introduce parsing errors, we break the
@@ -257,10 +263,12 @@ def do_inspection_loop(file)
         processed_source = get_processed_source(file)
       end
 
+      # Return summary of corrected offenses after all iterations
+      offenses = offenses_by_iteration.flatten.uniq
       [processed_source, offenses]
     end
 
-    def iterate_until_no_changes(source, offenses)
+    def iterate_until_no_changes(source, offenses_by_iteration)
       # Keep track of the state of the source. If a cop modifies the source
       # and another cop undoes it producing identical source we have an
       # infinite loop.
@@ -272,10 +280,10 @@ def iterate_until_no_changes(source, offenses)
       iterations = 0
 
       loop do
-        check_for_infinite_loop(source, offenses)
+        check_for_infinite_loop(source, offenses_by_iteration)
 
         if (iterations += 1) > MAX_ITERATIONS
-          raise InfiniteCorrectionLoop.new(source.path, offenses)
+          raise InfiniteCorrectionLoop.new(source.path, offenses_by_iteration)
         end
 
         source = yield
@@ -285,11 +293,15 @@ def iterate_until_no_changes(source, offenses)
 
     # Check whether a run created source identical to a previous run, which
     # means that we definitely have an infinite loop.
-    def check_for_infinite_loop(processed_source, offenses)
+    def check_for_infinite_loop(processed_source, offenses_by_iteration)
       checksum = processed_source.checksum
 
-      if @processed_sources.include?(checksum)
-        raise InfiniteCorrectionLoop.new(processed_source.path, offenses)
+      if (loop_start_index = @processed_sources.index(checksum))
+        raise InfiniteCorrectionLoop.new(
+          processed_source.path,
+          offenses_by_iteration,
+          loop_start: loop_start_index
+        )
       end
 
       @processed_sources << checksum

spec/rubocop/runner_spec.rb

@@ -189,23 +189,7 @@ def INVALID_CODE; end
   end
 
   describe '#run with cops autocorrecting each-other' do
-    subject(:runner) do
-      runner_class = Class.new(RuboCop::Runner) do
-        def mobilized_cop_classes(_config)
-          RuboCop::Cop::Registry.new(
-            [
-              RuboCop::Cop::Test::ClassMustBeAModuleCop,
-              RuboCop::Cop::Test::ModuleMustBeAClassCop
-            ]
-          )
-        end
-      end
-      runner_class.new(options, RuboCop::ConfigStore.new)
-    end
-
-    before do
-      create_file('example.rb', source)
-    end
+    let(:source_file_path) { create_file('example.rb', source) }
 
     let(:options) do
       {
@@ -214,17 +198,129 @@ def mobilized_cop_classes(_config)
       }
     end
 
-    context 'if there is an offense in an inspected file' do
-      let(:source) { <<~RUBY }
-        # frozen_string_literal: true
-        class Klass
+    context 'with two conflicting cops' do
+      subject(:runner) do
+        runner_class = Class.new(RuboCop::Runner) do
+          def mobilized_cop_classes(_config)
+            RuboCop::Cop::Registry.new(
+              [
+                RuboCop::Cop::Test::ClassMustBeAModuleCop,
+                RuboCop::Cop::Test::ModuleMustBeAClassCop
+              ]
+            )
+          end
         end
-      RUBY
+        runner_class.new(options, RuboCop::ConfigStore.new)
+      end
 
-      it 'aborts because of an infinite loop' do
-        expect do
-          runner.run([])
-        end.to raise_error RuboCop::Runner::InfiniteCorrectionLoop
+      context 'if there is an offense in an inspected file' do
+        let(:source) { <<~RUBY }
+          # frozen_string_literal: true
+          class Klass
+          end
+        RUBY
+
+        it 'aborts because of an infinite loop' do
+          expect do
+            runner.run([])
+          end.to raise_error(
+            RuboCop::Runner::InfiniteCorrectionLoop,
+            "Infinite loop detected in #{source_file_path} and caused by " \
+              'Test/ClassMustBeAModuleCop -> Test/ModuleMustBeAClassCop'
+          )
+        end
+      end
+
+      context 'if there are multiple offenses in an inspected file' do
+        let(:source) { <<~RUBY }
+          # frozen_string_literal: true
+          class Klass
+          end
+          class AnotherKlass
+          end
+        RUBY
+
+        it 'aborts because of an infinite loop' do
+          expect do
+            runner.run([])
+          end.to raise_error(
+            RuboCop::Runner::InfiniteCorrectionLoop,
+            "Infinite loop detected in #{source_file_path} and caused by " \
+              'Test/ClassMustBeAModuleCop -> Test/ModuleMustBeAClassCop'
+          )
+        end
+      end
+    end
+
+    context 'with two pairs of conflicting cops' do
+      subject(:runner) do
+        runner_class = Class.new(RuboCop::Runner) do
+          def mobilized_cop_classes(_config)
+            RuboCop::Cop::Registry.new(
+              [
+                RuboCop::Cop::Test::ClassMustBeAModuleCop,
+                RuboCop::Cop::Test::ModuleMustBeAClassCop,
+                RuboCop::Cop::Test::AtoB,
+                RuboCop::Cop::Test::BtoA
+              ]
+            )
+          end
+        end
+        runner_class.new(options, RuboCop::ConfigStore.new)
+      end
+
+      context 'if there is an offense in an inspected file' do
+        let(:source) { <<~RUBY }
+          # frozen_string_literal: true
+          class A_A
+          end
+        RUBY
+
+        it 'aborts because of an infinite loop' do
+          expect do
+            runner.run([])
+          end.to raise_error(
+            RuboCop::Runner::InfiniteCorrectionLoop,
+            "Infinite loop detected in #{source_file_path} and caused by " \
+              'Test/ClassMustBeAModuleCop, Test/AtoB ' \
+              '-> Test/ModuleMustBeAClassCop, Test/BtoA'
+          )
+        end
+      end
+
+      context 'with three cop cycle' do
+        subject(:runner) do
+          runner_class = Class.new(RuboCop::Runner) do
+            def mobilized_cop_classes(_config)
+              RuboCop::Cop::Registry.new(
+                [
+                  RuboCop::Cop::Test::AtoB,
+                  RuboCop::Cop::Test::BtoC,
+                  RuboCop::Cop::Test::CtoA
+                ]
+              )
+            end
+          end
+          runner_class.new(options, RuboCop::ConfigStore.new)
+        end
+
+        context 'if there is an offense in an inspected file' do
+          let(:source) { <<~RUBY }
+            # frozen_string_literal: true
+            class A
+            end
+          RUBY
+
+          it 'aborts because of an infinite loop' do
+            expect do
+              runner.run([])
+            end.to raise_error(
+              RuboCop::Runner::InfiniteCorrectionLoop,
+              "Infinite loop detected in #{source_file_path} and caused by " \
+                'Test/AtoB -> Test/BtoC -> Test/CtoA'
+            )
+          end
+        end
       end
     end
   end

spec/support/cops/a_to_b_cop.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Test
+      # This cop is only used to test infinite loop detection
+      class AtoB < RuboCop::Cop::Base
+        extend AutoCorrector
+
+        def on_class(node)
+          return unless /A/.match?(node.loc.name.source)
+
+          add_offense(node.loc.name, message: 'A to B') do |corrector|
+            autocorrect(corrector, node)
+          end
+        end
+        alias on_module on_class
+
+        private
+
+        def autocorrect(corrector, node)
+          corrector.replace(node.loc.name, node.loc.name.source.tr('A', 'B'))
+        end
+      end
+    end
+  end
+end

spec/support/cops/b_to_a_cop.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Test
+      # This cop is only used to test infinite loop detection
+      class BtoA < RuboCop::Cop::Base
+        extend AutoCorrector
+
+        def on_class(node)
+          return unless /B/.match?(node.loc.name.source)
+
+          add_offense(node.loc.name, message: 'B to A') do |corrector|
+            autocorrect(corrector, node)
+          end
+        end
+        alias on_module on_class
+
+        private
+
+        def autocorrect(corrector, node)
+          corrector.replace(node.loc.name, node.loc.name.source.tr('B', 'A'))
+        end
+      end
+    end
+  end
+end

spec/support/cops/b_to_c_cop.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Test
+      # This cop is only used to test infinite loop detection
+      class BtoC < RuboCop::Cop::Base
+        extend AutoCorrector
+
+        def on_class(node)
+          return unless /B/.match?(node.loc.name.source)
+
+          add_offense(node.loc.name, message: 'B to C') do |corrector|
+            autocorrect(corrector, node)
+          end
+        end
+        alias on_module on_class
+
+        private
+
+        def autocorrect(corrector, node)
+          corrector.replace(node.loc.name, node.loc.name.source.tr('B', 'C'))
+        end
+      end
+    end
+  end
+end

spec/support/cops/c_to_a_cop.rb

@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Test
+      # This cop is only used to test infinite loop detection
+      class CtoA < RuboCop::Cop::Base
+        extend AutoCorrector
+
+        def on_class(node)
+          return unless /C/.match?(node.loc.name.source)
+
+          add_offense(node.loc.name, message: 'C to A') do |corrector|
+            autocorrect(corrector, node)
+          end
+        end
+        alias on_module on_class
+
+        private
+
+        def autocorrect(corrector, node)
+          corrector.replace(node.loc.name, node.loc.name.source.tr('C', 'A'))
+        end
+      end
+    end
+  end
+end

spec/support/file_helper.rb

@@ -17,6 +17,8 @@ def create_file(file_path, content)
         file.puts content.join("\n")
       end
     end
+
+    file_path
   end
 
   def create_empty_file(file_path)