Skip to content

Commit

Permalink
Merge pull request #276 from fatkodima/assert_same-with-object-id
Browse files Browse the repository at this point in the history
Enhance `AssertSame`/`RefuteSame` to check for `object_id` comparison
  • Loading branch information
koic authored Nov 28, 2023
2 parents 14ba599 + 71a94d9 commit d0c87f2
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 4 deletions.
1 change: 1 addition & 0 deletions changelog/change_assert_same_check_object_ids.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#276](https://github.com/rubocop/rubocop-minitest/pull/276): Enhance `AssertSame`/`RefuteSame` to check for `object_id` comparison. ([@fatkodima][])
48 changes: 46 additions & 2 deletions lib/rubocop/cop/minitest/assert_same.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(%<new_arguments>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
Expand Down
48 changes: 46 additions & 2 deletions lib/rubocop/cop/minitest/refute_same.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(%<new_arguments>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
Expand Down
19 changes: 19 additions & 0 deletions test/rubocop/cop/minitest/assert_same_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions test/rubocop/cop/minitest/refute_same_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d0c87f2

Please sign in to comment.