From a028c031681df73661906cea18008b76b729ac88 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Thu, 19 Jan 2023 20:19:08 +0200 Subject: [PATCH] Add new `Minitest/LifecycleHooksOrder` cop --- .../new_minitest_lifecycle_hooks_order_cop.md | 1 + config/default.yml | 6 + .../cop/minitest/lifecycle_hooks_order.rb | 100 ++++++++++++ lib/rubocop/cop/minitest_cops.rb | 1 + .../cop/mixin/minitest_exploration_helpers.rb | 6 +- rubocop-minitest.gemspec | 2 +- .../minitest/lifecycle_hooks_order_test.rb | 146 ++++++++++++++++++ 7 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 changelog/new_minitest_lifecycle_hooks_order_cop.md create mode 100644 lib/rubocop/cop/minitest/lifecycle_hooks_order.rb create mode 100644 test/rubocop/cop/minitest/lifecycle_hooks_order_test.rb diff --git a/changelog/new_minitest_lifecycle_hooks_order_cop.md b/changelog/new_minitest_lifecycle_hooks_order_cop.md new file mode 100644 index 00000000..080f69b6 --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index 97fd7734..0839a689 100644 --- a/config/default.yml +++ b/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: '<>' + 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' diff --git a/lib/rubocop/cop/minitest/lifecycle_hooks_order.rb b/lib/rubocop/cop/minitest/lifecycle_hooks_order.rb new file mode 100644 index 00000000..8a07a5c2 --- /dev/null +++ b/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 = '`%s` is supposed to appear before `%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 diff --git a/lib/rubocop/cop/minitest_cops.rb b/lib/rubocop/cop/minitest_cops.rb index 8a5ae09a..ae63a8ff 100644 --- a/lib/rubocop/cop/minitest_cops.rb +++ b/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' diff --git a/lib/rubocop/cop/mixin/minitest_exploration_helpers.rb b/lib/rubocop/cop/mixin/minitest_exploration_helpers.rb index d0a640b0..9656ad9d 100644 --- a/lib/rubocop/cop/mixin/minitest_exploration_helpers.rb +++ b/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 diff --git a/rubocop-minitest.gemspec b/rubocop-minitest.gemspec index 7736b5bc..408eba49 100644 --- a/rubocop-minitest.gemspec +++ b/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 diff --git a/test/rubocop/cop/minitest/lifecycle_hooks_order_test.rb b/test/rubocop/cop/minitest/lifecycle_hooks_order_test.rb new file mode 100644 index 00000000..133a373b --- /dev/null +++ b/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