Merge pull request #232 from fatkodima/lifecycle_hooks_order-cop

Add new `Minitest/LifecycleHooksOrder` cop

changelog/new_minitest_lifecycle_hooks_order_cop.md

@@ -0,0 +1 @@
+* [#214](https://github.com/rubocop/rubocop-minitest/issues/214): Add new `Minitest/LifecycleHooksOrder` cop. ([@fatkodima][])

config/default.yml

@@ -158,6 +158,12 @@ Minitest/GlobalExpectations:
   VersionAdded: '0.7'
   VersionChanged: '0.26'
 
+Minitest/LifecycleHooksOrder:
+  Description: 'Checks that lifecycle hooks are declared in the order in which they will be executed.'
+  StyleGuide: 'https://minitest.rubystyle.guide/#hooks-ordering'
+  Enabled: pending
+  VersionAdded: '<<next>>'
+
 Minitest/LiteralAsActualArgument:
   Description: 'This cop enforces correct order of `expected` and `actual` arguments for `assert_equal`.'
   StyleGuide: 'https://minitest.rubystyle.guide/#assert-equal-arguments-order'

lib/rubocop/cop/minitest/lifecycle_hooks_order.rb

@@ -0,0 +1,100 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Minitest
+      # Checks that lifecycle hooks are declared in the order in which they will be executed.
+      #
+      # @example
+      #   # bad
+      #   class FooTest < Minitest::Test
+      #     def teardown; end
+      #     def setup; end
+      #   end
+      #
+      #   # good
+      #   class FooTest < Minitest::Test
+      #     def setup; end
+      #     def teardown; end
+      #   end
+      #
+      #   # bad (after test cases)
+      #   class FooTest < Minitest::Test
+      #     def test_something
+      #       assert foo
+      #     end
+      #     def setup; end
+      #     def teardown; end
+      #   end
+      #
+      #   # good
+      #   class FooTest < Minitest::Test
+      #     def setup; end
+      #     def teardown; end
+      #     def test_something
+      #       assert foo
+      #     end
+      #   end
+      #
+      #   # good (after non test case methods)
+      #   class FooTest < Minitest::Test
+      #     def do_something; end
+      #     def setup; end
+      #     def teardown; end
+      #   end
+      #
+      class LifecycleHooksOrder < Base
+        include MinitestExplorationHelpers
+        include RangeHelp
+        extend AutoCorrector
+
+        MSG = '`%<current>s` is supposed to appear before `%<previous>s`.'
+
+        # Regular method's position should be last.
+        REGULAR_METHOD_POSITION = LIFECYCLE_HOOK_METHODS_IN_ORDER.size + 1
+        HOOKS_ORDER_MAP = Hash.new do |hash, hook|
+          hash[hook] = LIFECYCLE_HOOK_METHODS_IN_ORDER.index(hook) || REGULAR_METHOD_POSITION
+        end
+
+        # rubocop:disable Metrics/MethodLength
+        def on_class(class_node)
+          return unless test_class?(class_node)
+
+          previous_index = -1
+          previous_hook_node = nil
+
+          hooks_and_test_cases(class_node).each do |node|
+            hook = node.method_name
+            index = HOOKS_ORDER_MAP[hook]
+
+            if index < previous_index
+              message = format(MSG, current: hook, previous: previous_hook_node.method_name)
+              add_offense(node, message: message) do |corrector|
+                autocorrect(corrector, previous_hook_node, node)
+              end
+            end
+            previous_index = index
+            previous_hook_node = node
+          end
+        end
+        # rubocop:enable Metrics/MethodLength
+
+        private
+
+        def hooks_and_test_cases(class_node)
+          class_def_nodes(class_node).select do |node|
+            lifecycle_hook_method?(node) || test_case?(node)
+          end
+        end
+
+        def autocorrect(corrector, previous_node, node)
+          previous_node_range = range_with_comments_and_lines(previous_node)
+          node_range = range_with_comments_and_lines(node)
+
+          corrector.insert_before(previous_node_range, node_range.source)
+          corrector.remove(node_range)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/minitest_cops.rb

@@ -30,6 +30,7 @@
 require_relative 'minitest/empty_line_before_assertion_methods'
 require_relative 'minitest/test_file_name'
 require_relative 'minitest/global_expectations'
+require_relative 'minitest/lifecycle_hooks_order'
 require_relative 'minitest/literal_as_actual_argument'
 require_relative 'minitest/multiple_assertions'
 require_relative 'minitest/no_assertions'

lib/rubocop/cop/mixin/minitest_exploration_helpers.rb

@@ -12,14 +12,16 @@ module MinitestExplorationHelpers
 
       ASSERTION_PREFIXES = %w[assert refute].freeze
 
-      LIFECYCLE_HOOK_METHODS = %i[
+      LIFECYCLE_HOOK_METHODS_IN_ORDER = %i[
         before_setup
         setup
         after_setup
         before_teardown
         teardown
         after_teardown
-      ].to_set.freeze
+      ].freeze
+
+      LIFECYCLE_HOOK_METHODS = LIFECYCLE_HOOK_METHODS_IN_ORDER.to_set.freeze
 
       private
 

rubocop-minitest.gemspec

@@ -32,6 +32,6 @@ Gem::Specification.new do |spec|
   spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
   spec.require_paths = ['lib']
 
-  spec.add_runtime_dependency 'rubocop', '>= 0.90', '< 2.0'
+  spec.add_runtime_dependency 'rubocop', '>= 1.39', '< 2.0'
   spec.add_development_dependency 'minitest', '~> 5.11'
 end

test/rubocop/cop/minitest/lifecycle_hooks_order_test.rb

@@ -0,0 +1,146 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class LifecycleHooksOrderTest < Minitest::Test
+  # rubocop:disable Metrics/MethodLength
+  def test_registers_offense_when_hooks_are_not_correctly_ordered
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def after_teardown
+          more_cleanup
+        end
+
+        def before_setup; end
+        ^^^^^^^^^^^^^^^^^^^^^ `before_setup` is supposed to appear before `after_teardown`.
+
+        def test_something
+          assert_equal foo, bar
+        end
+
+        def teardown
+        ^^^^^^^^^^^^ `teardown` is supposed to appear before `test_something`.
+          cleanup
+        end
+
+        def setup
+        ^^^^^^^^^ `setup` is supposed to appear before `teardown`.
+          setup_something
+        end
+
+        def test_something_else; end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def before_setup; end
+        def setup
+          setup_something
+        end
+        def teardown
+          cleanup
+        end
+        def after_teardown
+          more_cleanup
+        end
+
+
+        def test_something
+          assert_equal foo, bar
+        end
+
+
+
+        def test_something_else; end
+      end
+    RUBY
+  end
+  # rubocop:enable Metrics/MethodLength
+
+  def test_registers_offense_when_hooks_are_not_before_test_cases
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_something
+          assert_equal foo, bar
+        end
+
+        def setup; end
+        ^^^^^^^^^^^^^^ `setup` is supposed to appear before `test_something`.
+        def teardown; end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def setup; end
+        def teardown; end
+        def test_something
+          assert_equal foo, bar
+        end
+
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_hooks_after_non_test_cases
+    assert_no_offenses(<<~RUBY)
+      class FooTest < Minitest::Test
+        def do_something; end
+        def setup; end
+        def teardown; end
+      end
+    RUBY
+  end
+
+  def test_correctly_autocorrects_when_there_is_preceding_comment
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        # after_teardown comment
+        def after_teardown
+          more_cleanup
+        end
+
+        # before_setup comment
+        def before_setup; end
+        ^^^^^^^^^^^^^^^^^^^^^ `before_setup` is supposed to appear before `after_teardown`.
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        # before_setup comment
+        def before_setup; end
+        # after_teardown comment
+        def after_teardown
+          more_cleanup
+        end
+
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_correctly_ordered
+    assert_no_offenses(<<~RUBY)
+      class FooTest < Minitest::Test
+        def setup; end
+        def teardown; end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_not_in_test_class
+    assert_no_offenses(<<~RUBY)
+      class FooTest
+        def teardown; end
+        def setup; end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_no_callbacks
+    assert_no_offenses(<<~RUBY)
+      class FooTest < Minitest::Test; end
+    RUBY
+  end
+end