diff --git a/changelog/change_assert_same_check_object_ids.md b/changelog/change_assert_same_check_object_ids.md new file mode 100644 index 0000000..1af94ca --- /dev/null +++ b/changelog/change_assert_same_check_object_ids.md @@ -0,0 +1 @@ +* [#276](https://github.com/rubocop/rubocop-minitest/pull/276): Enhance `AssertSame`/`RefuteSame` to check for `object_id` comparison. ([@fatkodima][]) diff --git a/lib/rubocop/cop/minitest/assert_same.rb b/lib/rubocop/cop/minitest/assert_same.rb index da71292..9f7604d 100644 --- a/lib/rubocop/cop/minitest/assert_same.rb +++ b/lib/rubocop/cop/minitest/assert_same.rb @@ -12,14 +12,58 @@ module Minitest # @example # # bad # assert(expected.equal?(actual)) + # assert_equal(expected.object_id, actual.object_id) # # # good # assert_same(expected, actual) # class AssertSame < Base - extend MinitestCopRule + extend AutoCorrector - define_rule :assert, target_method: :equal?, preferred_method: :assert_same + MSG = 'Prefer using `assert_same(%s)`.' + RESTRICT_ON_SEND = %i[assert assert_equal].freeze + + def_node_matcher :assert_with_equal?, <<~PATTERN + (send nil? :assert + $(send $_ :equal? $_) + $_?) + PATTERN + + def_node_matcher :assert_equal_with_object_id?, <<~PATTERN + (send nil? :assert_equal + (send $_ :object_id) + (send $_ :object_id) + $_?) + PATTERN + + # rubocop:disable Metrics/AbcSize + def on_send(node) + if (equal_node, expected_node, actual_node, message_node = assert_with_equal?(node)) + add_offense(node, message: message(expected_node, actual_node, message_node.first)) do |corrector| + corrector.replace(node.loc.selector, 'assert_same') + corrector.replace(equal_node, "#{expected_node.source}, #{actual_node.source}") + end + elsif (expected_node, actual_node, message_node = assert_equal_with_object_id?(node)) + add_offense(node, message: message(expected_node, actual_node, message_node.first)) do |corrector| + corrector.replace(node.loc.selector, 'assert_same') + remove_method_call(expected_node.parent, corrector) + remove_method_call(actual_node.parent, corrector) + end + end + end + # rubocop:enable Metrics/AbcSize + + private + + def message(expected_node, actual_node, message_node) + arguments = [expected_node, actual_node, message_node].compact.map(&:source).join(', ') + format(MSG, new_arguments: arguments) + end + + def remove_method_call(send_node, corrector) + range = send_node.loc.dot.join(send_node.loc.selector) + corrector.remove(range) + end end end end diff --git a/lib/rubocop/cop/minitest/refute_same.rb b/lib/rubocop/cop/minitest/refute_same.rb index b7f6b41..7e6ae23 100644 --- a/lib/rubocop/cop/minitest/refute_same.rb +++ b/lib/rubocop/cop/minitest/refute_same.rb @@ -12,14 +12,58 @@ module Minitest # @example # # bad # refute(expected.equal?(actual)) + # refute_equal(expected.object_id, actual.object_id) # # # good # refute_same(expected, actual) # class RefuteSame < Base - extend MinitestCopRule + extend AutoCorrector - define_rule :refute, target_method: :equal?, preferred_method: :refute_same + MSG = 'Prefer using `refute_same(%s)`.' + RESTRICT_ON_SEND = %i[refute refute_equal].freeze + + def_node_matcher :refute_with_equal?, <<~PATTERN + (send nil? :refute + $(send $_ :equal? $_) + $_?) + PATTERN + + def_node_matcher :refute_equal_with_object_id?, <<~PATTERN + (send nil? :refute_equal + (send $_ :object_id) + (send $_ :object_id) + $_?) + PATTERN + + # rubocop:disable Metrics/AbcSize + def on_send(node) + if (equal_node, expected_node, actual_node, message_node = refute_with_equal?(node)) + add_offense(node, message: message(expected_node, actual_node, message_node.first)) do |corrector| + corrector.replace(node.loc.selector, 'refute_same') + corrector.replace(equal_node, "#{expected_node.source}, #{actual_node.source}") + end + elsif (expected_node, actual_node, message_node = refute_equal_with_object_id?(node)) + add_offense(node, message: message(expected_node, actual_node, message_node.first)) do |corrector| + corrector.replace(node.loc.selector, 'refute_same') + remove_method_call(expected_node.parent, corrector) + remove_method_call(actual_node.parent, corrector) + end + end + end + # rubocop:enable Metrics/AbcSize + + private + + def message(expected_node, actual_node, message_node) + arguments = [expected_node, actual_node, message_node].compact.map(&:source).join(', ') + format(MSG, new_arguments: arguments) + end + + def remove_method_call(send_node, corrector) + range = send_node.loc.dot.join(send_node.loc.selector) + corrector.remove(range) + end end end end diff --git a/test/rubocop/cop/minitest/assert_same_test.rb b/test/rubocop/cop/minitest/assert_same_test.rb index a011ee0..3badd56 100644 --- a/test/rubocop/cop/minitest/assert_same_test.rb +++ b/test/rubocop/cop/minitest/assert_same_test.rb @@ -85,6 +85,25 @@ def test_do_something RUBY end + def test_registers_offense_when_using_assert_equal_with_object_id + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_equal(expected.object_id, actual.object_id) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_same(expected, actual)`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + assert_same(expected, actual) + end + end + RUBY + end + def test_does_not_register_offense_when_using_assert_same assert_no_offenses(<<~RUBY) class FooTest < Minitest::Test diff --git a/test/rubocop/cop/minitest/refute_same_test.rb b/test/rubocop/cop/minitest/refute_same_test.rb index 7257379..326b31f 100644 --- a/test/rubocop/cop/minitest/refute_same_test.rb +++ b/test/rubocop/cop/minitest/refute_same_test.rb @@ -85,6 +85,25 @@ def test_do_something RUBY end + def test_registers_offense_when_using_refute_equal_with_object_id + assert_offense(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute_equal(expected.object_id, actual.object_id) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_same(expected, actual)`. + end + end + RUBY + + assert_correction(<<~RUBY) + class FooTest < Minitest::Test + def test_do_something + refute_same(expected, actual) + end + end + RUBY + end + def test_does_not_register_offense_when_using_assert_same assert_no_offenses(<<~RUBY) class FooTest < Minitest::Test