Skip to content

Commit 3613469

Browse files
ydahydah
authored
Merge pull request #1780 from G-Rath/support-assert-instance-of
feat: support `*_instance_of` in `RSpec/Rails/MinitestAssertions`
2 parents d9a818a + a621a0f commit 3613469

File tree

3 files changed

+161
-67
lines changed

3 files changed

+161
-67
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Master (Unreleased)
44

55
- Add support for `assert_empty`, `assert_not_empty` and `refute_empty` to `RSpec/Rails/MinitestAssertions`. ([@ydah])
6+
- Support correcting `*_instance_of` assertions in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
67
- Support correcting `*_includes` assertions in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
78
- Support correcting `assert_not_equal` and `assert_not_nil` in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
89
- Fix a false positive for `RSpec/ExpectActual` when used with rspec-rails routing matchers. ([@naveg])

‎lib/rubocop/cop/rspec_rails/minitest_assertions.rb‎

Lines changed: 59 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,16 @@ class MinitestAssertions < Base
3030
RESTRICT_ON_SEND = %i[
3131
assert_equal
3232
assert_not_equal
33+
assert_instance_of
34+
assert_not_instance_of
3335
assert_includes
3436
assert_not_includes
3537
assert_nil
3638
assert_not_nil
3739
assert_empty
3840
assert_not_empty
3941
refute_equal
42+
refute_instance_of
4043
refute_includes
4144
refute_nil
4245
refute_empty
@@ -47,6 +50,11 @@ class MinitestAssertions < Base
4750
(send nil? {:assert_equal :assert_not_equal :refute_equal} $_ $_ $_?)
4851
PATTERN
4952

53+
# @!method minitest_instance_of(node)
54+
def_node_matcher :minitest_instance_of, <<~PATTERN
55+
(send nil? {:assert_instance_of :assert_not_instance_of :refute_instance_of} $_ $_ $_?)
56+
PATTERN
57+
5058
# @!method minitest_includes(node)
5159
def_node_matcher :minitest_includes, <<~PATTERN
5260
(send nil? {:assert_includes :assert_not_includes :refute_includes} $_ $_ $_?)
@@ -68,18 +76,23 @@ def on_send(node) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
6876
failure_message.first))
6977
end
7078

79+
minitest_instance_of(node) do |expected, actual, failure_message|
80+
on_assertion(node, InstanceOfAssertion.new(expected, actual,
81+
failure_message.first))
82+
end
83+
7184
minitest_includes(node) do |collection, expected, failure_message|
72-
on_assertion(node, IncludesAssertion.new(collection, expected,
85+
on_assertion(node, IncludesAssertion.new(expected, collection,
7386
failure_message.first))
7487
end
7588

7689
minitest_nil(node) do |actual, failure_message|
77-
on_assertion(node, NilAssertion.new(actual,
90+
on_assertion(node, NilAssertion.new(nil, actual,
7891
failure_message.first))
7992
end
8093

8194
minitest_empty(node) do |actual, failure_message|
82-
on_assertion(node, EmptyAssertion.new(actual,
95+
on_assertion(node, EmptyAssertion.new(nil, actual,
8396
failure_message.first))
8497
end
8598
end
@@ -96,79 +109,75 @@ def message(preferred)
96109
end
97110

98111
# :nodoc:
99-
class EqualAssertion
112+
class BasicAssertion
100113
def initialize(expected, actual, fail_message)
101-
@expected = expected
102-
@actual = actual
103-
@fail_message = fail_message
114+
@expected = expected&.source
115+
@actual = actual.source
116+
@fail_message = fail_message&.source
104117
end
105118

106119
def replaced(node)
107-
runner = node.method?(:assert_equal) ? 'to' : 'not_to'
120+
runner = negated?(node) ? 'not_to' : 'to'
108121
if @fail_message.nil?
109-
"expect(#{@actual.source}).#{runner} eq(#{@expected.source})"
122+
"expect(#{@actual}).#{runner} #{assertion}"
110123
else
111-
"expect(#{@actual.source}).#{runner}(eq(#{@expected.source})," \
112-
" #{@fail_message.source})"
124+
"expect(#{@actual}).#{runner}(#{assertion}, #{@fail_message})"
113125
end
114126
end
115127
end
116128

117129
# :nodoc:
118-
class IncludesAssertion
119-
def initialize(collection, expected, fail_message)
120-
@collection = collection
121-
@expected = expected
122-
@fail_message = fail_message
130+
class EqualAssertion < BasicAssertion
131+
def negated?(node)
132+
!node.method?(:assert_equal)
123133
end
124134

125-
def replaced(node)
126-
a_source = @collection.source
127-
b_source = @expected.source
135+
def assertion
136+
"eq(#{@expected})"
137+
end
138+
end
128139

129-
runner = node.method?(:assert_includes) ? 'to' : 'not_to'
130-
if @fail_message.nil?
131-
"expect(#{a_source}).#{runner} include(#{b_source})"
132-
else
133-
"expect(#{a_source}).#{runner}(include(#{b_source})," \
134-
" #{@fail_message.source})"
135-
end
140+
# :nodoc:
141+
class InstanceOfAssertion < BasicAssertion
142+
def negated?(node)
143+
!node.method?(:assert_instance_of)
144+
end
145+
146+
def assertion
147+
"be_an_instance_of(#{@expected})"
136148
end
137149
end
138150

139151
# :nodoc:
140-
class NilAssertion
141-
def initialize(actual, fail_message)
142-
@actual = actual
143-
@fail_message = fail_message
152+
class IncludesAssertion < BasicAssertion
153+
def negated?(node)
154+
!node.method?(:assert_includes)
144155
end
145156

146-
def replaced(node)
147-
runner = node.method?(:assert_nil) ? 'to' : 'not_to'
148-
if @fail_message.nil?
149-
"expect(#{@actual.source}).#{runner} eq(nil)"
150-
else
151-
"expect(#{@actual.source}).#{runner}(eq(nil), " \
152-
"#{@fail_message.source})"
153-
end
157+
def assertion
158+
"include(#{@expected})"
154159
end
155160
end
156161

157162
# :nodoc:
158-
class EmptyAssertion
159-
def initialize(actual, fail_message)
160-
@actual = actual
161-
@fail_message = fail_message
163+
class NilAssertion < BasicAssertion
164+
def negated?(node)
165+
!node.method?(:assert_nil)
162166
end
163167

164-
def replaced(node)
165-
runner = node.method?(:assert_empty) ? 'to' : 'not_to'
166-
if @fail_message.nil?
167-
"expect(#{@actual.source}).#{runner} be_empty"
168-
else
169-
"expect(#{@actual.source}).#{runner}(be_empty, " \
170-
"#{@fail_message.source})"
171-
end
168+
def assertion
169+
'eq(nil)'
170+
end
171+
end
172+
173+
# :nodoc:
174+
class EmptyAssertion < BasicAssertion
175+
def negated?(node)
176+
!node.method?(:assert_empty)
177+
end
178+
179+
def assertion
180+
'be_empty'
172181
end
173182
end
174183
end

‎spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb‎

Lines changed: 101 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,107 @@
8484
end
8585
end
8686

87+
context 'with instance_of assertions' do
88+
it 'registers an offense when using `assert_instance_of`' do
89+
expect_offense(<<~RUBY)
90+
assert_instance_of(a, b)
91+
^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to be_an_instance_of(a)`.
92+
RUBY
93+
94+
expect_correction(<<~RUBY)
95+
expect(b).to be_an_instance_of(a)
96+
RUBY
97+
end
98+
99+
it 'registers an offense when using `assert_instance_of` with ' \
100+
'no parentheses' do
101+
expect_offense(<<~RUBY)
102+
assert_instance_of a, b
103+
^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to be_an_instance_of(a)`.
104+
RUBY
105+
106+
expect_correction(<<~RUBY)
107+
expect(b).to be_an_instance_of(a)
108+
RUBY
109+
end
110+
111+
it 'registers an offense when using `assert_instance_of` with' \
112+
'failure message' do
113+
expect_offense(<<~RUBY)
114+
assert_instance_of a, b, "must be instance of"
115+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(be_an_instance_of(a), "must be instance of")`.
116+
RUBY
117+
118+
expect_correction(<<~RUBY)
119+
expect(b).to(be_an_instance_of(a), "must be instance of")
120+
RUBY
121+
end
122+
123+
it 'registers an offense when using `assert_instance_of` with ' \
124+
'multi-line arguments' do
125+
expect_offense(<<~RUBY)
126+
assert_instance_of(a,
127+
^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(be_an_instance_of(a), "must be instance of")`.
128+
b,
129+
"must be instance of")
130+
RUBY
131+
132+
expect_correction(<<~RUBY)
133+
expect(b).to(be_an_instance_of(a), "must be instance of")
134+
RUBY
135+
end
136+
137+
it 'registers an offense when using `assert_not_instance_of`' do
138+
expect_offense(<<~RUBY)
139+
assert_not_instance_of a, b
140+
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).not_to be_an_instance_of(a)`.
141+
RUBY
142+
143+
expect_correction(<<~RUBY)
144+
expect(b).not_to be_an_instance_of(a)
145+
RUBY
146+
end
147+
148+
it 'registers an offense when using `refute_instance_of`' do
149+
expect_offense(<<~RUBY)
150+
refute_instance_of a, b
151+
^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).not_to be_an_instance_of(a)`.
152+
RUBY
153+
154+
expect_correction(<<~RUBY)
155+
expect(b).not_to be_an_instance_of(a)
156+
RUBY
157+
end
158+
159+
it 'does not register an offense when ' \
160+
'using `expect(b).to be_an_instance_of(a)`' do
161+
expect_no_offenses(<<~RUBY)
162+
expect(b).to be_an_instance_of(a)
163+
RUBY
164+
end
165+
166+
it 'does not register an offense when ' \
167+
'using `expect(b).not_to be_an_instance_of(a)`' do
168+
expect_no_offenses(<<~RUBY)
169+
expect(b).not_to be_an_instance_of(a)
170+
RUBY
171+
end
172+
173+
it 'does not register an offense when ' \
174+
'using `expect(b).to be_instance_of(a)`' do
175+
expect_no_offenses(<<~RUBY)
176+
expect(b).to be_instance_of(a)
177+
RUBY
178+
end
179+
180+
it 'does not register an offense when ' \
181+
'using `expect(b).not_to be_instance_of(a)`' do
182+
expect_no_offenses(<<~RUBY)
183+
expect(b).not_to be_instance_of(a)
184+
RUBY
185+
end
186+
end
187+
87188
context 'with includes assertions' do
88189
it 'registers an offense when using `assert_includes`' do
89190
expect_offense(<<~RUBY)
@@ -150,23 +251,6 @@
150251
refute_includes a, b
151252
^^^^^^^^^^^^^^^^^^^^ Use `expect(a).not_to include(b)`.
152253
RUBY
153-
154-
expect_correction(<<~RUBY)
155-
expect(a).not_to include(b)
156-
RUBY
157-
end
158-
159-
it 'does not register an offense when using `expect(a).to include(b)`' do
160-
expect_no_offenses(<<~RUBY)
161-
expect(a).to include(b)
162-
RUBY
163-
end
164-
165-
it 'does not register an offense when ' \
166-
'using `expect(a).not_to include(b)`' do
167-
expect_no_offenses(<<~RUBY)
168-
expect(a).not_to include(b)
169-
RUBY
170254
end
171255
end
172256

0 commit comments

Comments
 (0)