Skip to content

Commit fe0618e

Browse files
authored
Merge pull request #248 from koic/make_instance_of_cops_aware_of_equal
Make `Minitest/AssertInstanceOf` and `Minitest/RefuteInstanceOf` aware of `assert_equal` and `refute_equal`
2 parents e970d41 + 2d9e7ca commit fe0618e

File tree

7 files changed

+183
-13
lines changed

7 files changed

+183
-13
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#248](https://github.com/rubocop/rubocop-minitest/pull/248): Make `Minitest/AssertInstanceOf` and `Minitest/RefuteInstanceOf` aware of `assert_equal(Class, object.class)` and `refute_equal(Class, object.class)`. ([@koic][])

lib/rubocop/cop/minitest/assert_instance_of.rb

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,30 @@ module Minitest
1111
# assert(object.instance_of?(Class))
1212
# assert(object.instance_of?(Class), 'message')
1313
#
14+
# # bad
15+
# assert_equal(Class, object.class)
16+
# assert_equal(Class, object.class, 'message')
17+
#
1418
# # good
1519
# assert_instance_of(Class, object)
1620
# assert_instance_of(Class, object, 'message')
1721
#
1822
class AssertInstanceOf < Base
19-
extend MinitestCopRule
23+
include InstanceOfAssertionHandleable
24+
extend AutoCorrector
25+
26+
RESTRICT_ON_SEND = %i[assert assert_equal].freeze
27+
28+
def_node_matcher :instance_of_assertion?, <<~PATTERN
29+
{
30+
(send nil? :assert (send $_ :instance_of? $const) $_?)
31+
(send nil? :assert_equal $const (send $_ :class) $_?)
32+
}
33+
PATTERN
2034

21-
define_rule :assert, target_method: :instance_of?, inverse: true
35+
def on_send(node)
36+
investigate(node, :assert)
37+
end
2238
end
2339
end
2440
end

lib/rubocop/cop/minitest/refute_instance_of.rb

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,30 @@ module Minitest
1111
# refute(object.instance_of?(Class))
1212
# refute(object.instance_of?(Class), 'message')
1313
#
14+
# # bad
15+
# refute_equal(Class, object.class)
16+
# refute_equal(Class, object.class, 'message')
17+
#
1418
# # good
1519
# refute_instance_of(Class, object)
1620
# refute_instance_of(Class, object, 'message')
1721
#
1822
class RefuteInstanceOf < Base
19-
extend MinitestCopRule
23+
include InstanceOfAssertionHandleable
24+
extend AutoCorrector
25+
26+
RESTRICT_ON_SEND = %i[refute refute_equal].freeze
27+
28+
def_node_matcher :instance_of_assertion?, <<~PATTERN
29+
{
30+
(send nil? :refute (send $_ :instance_of? $const) $_?)
31+
(send nil? :refute_equal $const (send $_ :class) $_?)
32+
}
33+
PATTERN
2034

21-
define_rule :refute, target_method: :instance_of?, inverse: true
35+
def on_send(node)
36+
investigate(node, :refute)
37+
end
2238
end
2339
end
2440
end

lib/rubocop/cop/minitest_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require_relative 'mixin/argument_range_helper'
44
require_relative 'mixin/in_delta_mixin'
5+
require_relative 'mixin/instance_of_assertion_handleable'
56
require_relative 'mixin/minitest_cop_rule'
67
require_relative 'mixin/minitest_exploration_helpers'
78
require_relative 'mixin/nil_assertion_handleable'
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Minitest
6+
# Common functionality for `Minitest/AssertInstanceOf` and `Minitest/RefuteInstanceOf` cops.
7+
# @api private
8+
module InstanceOfAssertionHandleable
9+
include ArgumentRangeHelper
10+
11+
MSG = 'Prefer using `%<prefer>s`.'
12+
13+
private
14+
15+
def investigate(node, assertion_type)
16+
return unless (first_capture, second_capture, message = instance_of_assertion?(node))
17+
18+
required_arguments = build_required_arguments(node, assertion_type, first_capture, second_capture)
19+
full_arguments = [required_arguments, message.first&.source].compact.join(', ')
20+
prefer = "#{assertion_type}_instance_of(#{full_arguments})"
21+
22+
add_offense(node, message: format(MSG, prefer: prefer)) do |corrector|
23+
range = replacement_range(node, assertion_type)
24+
25+
corrector.replace(node.loc.selector, "#{assertion_type}_instance_of")
26+
corrector.replace(range, required_arguments)
27+
end
28+
end
29+
30+
def build_required_arguments(node, method_name, first_capture, second_capture)
31+
if node.method?(method_name)
32+
[second_capture, first_capture]
33+
else
34+
[first_capture, second_capture]
35+
end.map(&:source).join(', ')
36+
end
37+
38+
def replacement_range(node, method_name)
39+
if node.method?(method_name)
40+
node.first_argument
41+
else
42+
first_and_second_arguments_range(node)
43+
end
44+
end
45+
end
46+
end
47+
end
48+
end

test/rubocop/cop/minitest/assert_instance_of_test.rb

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,20 +66,64 @@ def test_do_something
6666
RUBY
6767
end
6868

69-
def test_registers_offense_when_using_assert_with_instance_of_in_redundant_parentheses
69+
def test_registers_offense_when_using_assert_equal_with_class_arguments
7070
assert_offense(<<~RUBY)
7171
class FooTest < Minitest::Test
7272
def test_do_something
73-
assert((object.instance_of?(SomeClass)))
74-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, object)`.
73+
assert_equal(SomeClass, obj.class)
74+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, obj)`.
7575
end
7676
end
7777
RUBY
7878

7979
assert_correction(<<~RUBY)
8080
class FooTest < Minitest::Test
8181
def test_do_something
82-
assert_instance_of((SomeClass, object))
82+
assert_instance_of(SomeClass, obj)
83+
end
84+
end
85+
RUBY
86+
end
87+
88+
def test_registers_offense_when_using_assert_equal_with_class_arguments_and_message
89+
assert_offense(<<~RUBY)
90+
class FooTest < Minitest::Test
91+
def test_do_something
92+
assert_equal(SomeClass, obj.class, 'message')
93+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, obj, 'message')`.
94+
end
95+
end
96+
RUBY
97+
98+
assert_correction(<<~RUBY)
99+
class FooTest < Minitest::Test
100+
def test_do_something
101+
assert_instance_of(SomeClass, obj, 'message')
102+
end
103+
end
104+
RUBY
105+
end
106+
107+
def test_registers_offense_when_using_assert_equal_with_class_arguments_and_heredoc_message
108+
assert_offense(<<~RUBY)
109+
class FooTest < Minitest::Test
110+
def test_do_something
111+
assert_equal(SomeClass, obj.class, <<~MESSAGE
112+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, obj, <<~MESSAGE)`.
113+
message
114+
MESSAGE
115+
)
116+
end
117+
end
118+
RUBY
119+
120+
assert_correction(<<~RUBY)
121+
class FooTest < Minitest::Test
122+
def test_do_something
123+
assert_instance_of(SomeClass, obj, <<~MESSAGE
124+
message
125+
MESSAGE
126+
)
83127
end
84128
end
85129
RUBY

test/rubocop/cop/minitest/refute_instance_of_test.rb

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,26 +66,70 @@ def test_do_something
6666
RUBY
6767
end
6868

69-
def test_registers_offense_when_using_refute_with_instance_of_in_redundant_parentheses
69+
def test_registers_offense_when_using_refute_equal_with_class_arguments
7070
assert_offense(<<~RUBY)
7171
class FooTest < Minitest::Test
7272
def test_do_something
73-
refute((object.instance_of?(SomeClass)))
74-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, object)`.
73+
refute_equal(SomeClass, obj.class)
74+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, obj)`.
7575
end
7676
end
7777
RUBY
7878

7979
assert_correction(<<~RUBY)
8080
class FooTest < Minitest::Test
8181
def test_do_something
82-
refute_instance_of((SomeClass, object))
82+
refute_instance_of(SomeClass, obj)
8383
end
8484
end
8585
RUBY
8686
end
8787

88-
def test_refute_instance_of_method
88+
def test_registers_offense_when_using_refute_equal_with_class_arguments_and_message
89+
assert_offense(<<~RUBY)
90+
class FooTest < Minitest::Test
91+
def test_do_something
92+
refute_equal(SomeClass, obj.class, 'message')
93+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, obj, 'message')`.
94+
end
95+
end
96+
RUBY
97+
98+
assert_correction(<<~RUBY)
99+
class FooTest < Minitest::Test
100+
def test_do_something
101+
refute_instance_of(SomeClass, obj, 'message')
102+
end
103+
end
104+
RUBY
105+
end
106+
107+
def test_registers_offense_when_using_refute_equal_with_class_arguments_and_heredoc_message
108+
assert_offense(<<~RUBY)
109+
class FooTest < Minitest::Test
110+
def test_do_something
111+
refute_equal(SomeClass, obj.class, <<~MESSAGE
112+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, obj, <<~MESSAGE)`.
113+
message
114+
MESSAGE
115+
)
116+
end
117+
end
118+
RUBY
119+
120+
assert_correction(<<~RUBY)
121+
class FooTest < Minitest::Test
122+
def test_do_something
123+
refute_instance_of(SomeClass, obj, <<~MESSAGE
124+
message
125+
MESSAGE
126+
)
127+
end
128+
end
129+
RUBY
130+
end
131+
132+
def test_does_not_register_offense_when_using_refute_instance_of_method
89133
assert_no_offenses(<<~RUBY)
90134
class FooTest < Minitest::Test
91135
def test_do_something

0 commit comments

Comments
 (0)