Skip to content

Commit

Permalink
Enhance AssertSame/RefuteSame to check for object_id comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Nov 25, 2023
1 parent 0b26645 commit 09dde90
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 9 deletions.
5 changes: 5 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ Style/FormatStringToken:
Exclude:
- test/**/*

Style/DocumentDynamicEvalDefinition:
Exclude:
- 'lib/rubocop/cop/mixin/minitest_cop_rule.rb'
- 'lib/rubocop/cop/mixin/assert_refute_same_helper.rb'

Metrics/ClassLength:
Exclude:
- test/**/*
Expand Down
5 changes: 0 additions & 5 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,3 @@ Metrics/MethodLength:
Max: 14
Exclude:
- 'test/rubocop/cop/minitest/global_expectations_test.rb'

# Offense count: 1
Style/DocumentDynamicEvalDefinition:
Exclude:
- 'lib/rubocop/cop/mixin/minitest_cop_rule.rb'
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][])
5 changes: 3 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,15 @@ 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 AssertRefuteSameHelper

define_rule :assert, target_method: :equal?, preferred_method: :assert_same
define_rule :assert
end
end
end
Expand Down
5 changes: 3 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,15 @@ 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 AssertRefuteSameHelper

define_rule :refute, target_method: :equal?, preferred_method: :refute_same
define_rule :refute
end
end
end
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/minitest_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
require_relative 'mixin/minitest_exploration_helpers'
require_relative 'mixin/nil_assertion_handleable'
require_relative 'mixin/predicate_assertion_handleable'
require_relative 'mixin/assert_refute_same_helper'
require_relative 'minitest/assert_empty'
require_relative 'minitest/assert_empty_literal'
require_relative 'minitest/assert_equal'
Expand Down
57 changes: 57 additions & 0 deletions lib/rubocop/cop/mixin/assert_refute_same_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# frozen_string_literal: true

module RuboCop
module Cop
# Provide a method to define offense rule for Minitest cops.
module AssertRefuteSameHelper
def define_rule(assertion_method)
class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
extend AutoCorrector
MSG = 'Prefer using `#{assertion_method}_same(%<new_arguments>s)`.'
RESTRICT_ON_SEND = %i[#{assertion_method} #{assertion_method}_equal]
def_node_matcher :assertion_with_equal?, <<~PATTERN
(send nil? :#{assertion_method}
$(send $_ :equal? $_)
$_?)
PATTERN
def_node_matcher :equality_with_object_id?, <<~PATTERN
(send nil? :#{assertion_method}_equal
(send $_ :object_id)
(send $_ :object_id)
$_?)
PATTERN
def on_send(node)
if (equal_node, expected_node, actual_node, message_node = assertion_with_equal?(node))
add_offense(node, message: message(expected_node, actual_node, message_node.first)) do |corrector|
corrector.replace(node.loc.selector, "#{assertion_method}_same")
corrector.replace(equal_node, "\#{expected_node.source}, \#{actual_node.source}")
end
elsif (expected_node, actual_node, message_node = equality_with_object_id?(node))
add_offense(node, message: message(expected_node, actual_node, message_node.first)) do |corrector|
corrector.replace(node.loc.selector, "#{assertion_method}_same")
remove_method_call(expected_node.parent, corrector)
remove_method_call(actual_node.parent, corrector)
end
end
end
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
RUBY
end
end
end
end
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 09dde90

Please sign in to comment.