Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support *_instance_of in RSpec/Rails/MinitestAssertions #1780

Merged
merged 2 commits into from
Jan 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Master (Unreleased)

- Add support for `assert_empty`, `assert_not_empty` and `refute_empty` to `RSpec/Rails/MinitestAssertions`. ([@ydah])
- Support correcting `*_instance_of` assertions in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
- Support correcting `*_includes` assertions in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
- Support correcting `assert_not_equal` and `assert_not_nil` in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
- Fix a false positive for `RSpec/ExpectActual` when used with rspec-rails routing matchers. ([@naveg])
Expand Down
109 changes: 59 additions & 50 deletions lib/rubocop/cop/rspec/rails/minitest_assertions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ class MinitestAssertions < Base
RESTRICT_ON_SEND = %i[
assert_equal
assert_not_equal
assert_instance_of
assert_not_instance_of
assert_includes
assert_not_includes
assert_nil
assert_not_nil
assert_empty
assert_not_empty
refute_equal
refute_instance_of
refute_includes
refute_nil
refute_empty
Expand All @@ -47,6 +50,11 @@ class MinitestAssertions < Base
(send nil? {:assert_equal :assert_not_equal :refute_equal} $_ $_ $_?)
PATTERN

# @!method minitest_instance_of(node)
def_node_matcher :minitest_instance_of, <<~PATTERN
(send nil? {:assert_instance_of :assert_not_instance_of :refute_instance_of} $_ $_ $_?)
PATTERN

# @!method minitest_includes(node)
def_node_matcher :minitest_includes, <<~PATTERN
(send nil? {:assert_includes :assert_not_includes :refute_includes} $_ $_ $_?)
Expand All @@ -68,18 +76,23 @@ def on_send(node) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
failure_message.first))
end

minitest_instance_of(node) do |expected, actual, failure_message|
on_assertion(node, InstanceOfAssertion.new(expected, actual,
failure_message.first))
end

minitest_includes(node) do |collection, expected, failure_message|
on_assertion(node, IncludesAssertion.new(collection, expected,
on_assertion(node, IncludesAssertion.new(expected, collection,
failure_message.first))
end

minitest_nil(node) do |actual, failure_message|
on_assertion(node, NilAssertion.new(actual,
on_assertion(node, NilAssertion.new(nil, actual,
failure_message.first))
end

minitest_empty(node) do |actual, failure_message|
on_assertion(node, EmptyAssertion.new(actual,
on_assertion(node, EmptyAssertion.new(nil, actual,
failure_message.first))
end
end
Expand All @@ -96,79 +109,75 @@ def message(preferred)
end

# :nodoc:
class EqualAssertion
class BasicAssertion
def initialize(expected, actual, fail_message)
@expected = expected
@actual = actual
@fail_message = fail_message
@expected = expected&.source
@actual = actual.source
@fail_message = fail_message&.source
end

def replaced(node)
runner = node.method?(:assert_equal) ? 'to' : 'not_to'
runner = negated?(node) ? 'not_to' : 'to'
if @fail_message.nil?
"expect(#{@actual.source}).#{runner} eq(#{@expected.source})"
"expect(#{@actual}).#{runner} #{assertion}"
else
"expect(#{@actual.source}).#{runner}(eq(#{@expected.source})," \
" #{@fail_message.source})"
"expect(#{@actual}).#{runner}(#{assertion}, #{@fail_message})"
end
end
end

# :nodoc:
class IncludesAssertion
def initialize(collection, expected, fail_message)
@collection = collection
@expected = expected
@fail_message = fail_message
class EqualAssertion < BasicAssertion
def negated?(node)
!node.method?(:assert_equal)
end

def replaced(node)
a_source = @collection.source
b_source = @expected.source
def assertion
"eq(#{@expected})"
end
end

runner = node.method?(:assert_includes) ? 'to' : 'not_to'
if @fail_message.nil?
"expect(#{a_source}).#{runner} include(#{b_source})"
else
"expect(#{a_source}).#{runner}(include(#{b_source})," \
" #{@fail_message.source})"
end
# :nodoc:
class InstanceOfAssertion < BasicAssertion
def negated?(node)
!node.method?(:assert_instance_of)
end

def assertion
"be_an_instance_of(#{@expected})"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with this form since it's the grammatically correct version and the "original" (be_instance_of is an alias)

end
end

# :nodoc:
class NilAssertion
def initialize(actual, fail_message)
@actual = actual
@fail_message = fail_message
class IncludesAssertion < BasicAssertion
def negated?(node)
!node.method?(:assert_includes)
end

def replaced(node)
runner = node.method?(:assert_nil) ? 'to' : 'not_to'
if @fail_message.nil?
"expect(#{@actual.source}).#{runner} eq(nil)"
else
"expect(#{@actual.source}).#{runner}(eq(nil), " \
"#{@fail_message.source})"
end
def assertion
"include(#{@expected})"
end
end

# :nodoc:
class EmptyAssertion
def initialize(actual, fail_message)
@actual = actual
@fail_message = fail_message
class NilAssertion < BasicAssertion
def negated?(node)
!node.method?(:assert_nil)
end

def replaced(node)
runner = node.method?(:assert_empty) ? 'to' : 'not_to'
if @fail_message.nil?
"expect(#{@actual.source}).#{runner} be_empty"
else
"expect(#{@actual.source}).#{runner}(be_empty, " \
"#{@fail_message.source})"
end
def assertion
'eq(nil)'
end
end

# :nodoc:
class EmptyAssertion < BasicAssertion
def negated?(node)
!node.method?(:assert_empty)
end

def assertion
'be_empty'
end
end
end
Expand Down
118 changes: 101 additions & 17 deletions spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,107 @@
end
end

context 'with instance_of assertions' do
it 'registers an offense when using `assert_instance_of`' do
expect_offense(<<~RUBY)
assert_instance_of(a, b)
^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to be_an_instance_of(a)`.
RUBY

expect_correction(<<~RUBY)
expect(b).to be_an_instance_of(a)
RUBY
end

it 'registers an offense when using `assert_instance_of` with ' \
'no parentheses' do
expect_offense(<<~RUBY)
assert_instance_of a, b
^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to be_an_instance_of(a)`.
RUBY

expect_correction(<<~RUBY)
expect(b).to be_an_instance_of(a)
RUBY
end

it 'registers an offense when using `assert_instance_of` with' \
'failure message' do
expect_offense(<<~RUBY)
assert_instance_of a, b, "must be instance of"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(be_an_instance_of(a), "must be instance of")`.
RUBY

expect_correction(<<~RUBY)
expect(b).to(be_an_instance_of(a), "must be instance of")
RUBY
end

it 'registers an offense when using `assert_instance_of` with ' \
'multi-line arguments' do
expect_offense(<<~RUBY)
assert_instance_of(a,
^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(be_an_instance_of(a), "must be instance of")`.
b,
"must be instance of")
RUBY

expect_correction(<<~RUBY)
expect(b).to(be_an_instance_of(a), "must be instance of")
RUBY
end

it 'registers an offense when using `assert_not_instance_of`' do
expect_offense(<<~RUBY)
assert_not_instance_of a, b
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).not_to be_an_instance_of(a)`.
RUBY

expect_correction(<<~RUBY)
expect(b).not_to be_an_instance_of(a)
RUBY
end

it 'registers an offense when using `refute_instance_of`' do
expect_offense(<<~RUBY)
refute_instance_of a, b
^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).not_to be_an_instance_of(a)`.
RUBY

expect_correction(<<~RUBY)
expect(b).not_to be_an_instance_of(a)
RUBY
end

it 'does not register an offense when ' \
'using `expect(b).to be_an_instance_of(a)`' do
expect_no_offenses(<<~RUBY)
expect(b).to be_an_instance_of(a)
RUBY
end

it 'does not register an offense when ' \
'using `expect(b).not_to be_an_instance_of(a)`' do
expect_no_offenses(<<~RUBY)
expect(b).not_to be_an_instance_of(a)
RUBY
end

it 'does not register an offense when ' \
'using `expect(b).to be_instance_of(a)`' do
expect_no_offenses(<<~RUBY)
expect(b).to be_instance_of(a)
RUBY
end

it 'does not register an offense when ' \
'using `expect(b).not_to be_instance_of(a)`' do
expect_no_offenses(<<~RUBY)
expect(b).not_to be_instance_of(a)
RUBY
end
end

context 'with includes assertions' do
it 'registers an offense when using `assert_includes`' do
expect_offense(<<~RUBY)
Expand Down Expand Up @@ -150,23 +251,6 @@
refute_includes a, b
^^^^^^^^^^^^^^^^^^^^ Use `expect(a).not_to include(b)`.
RUBY

expect_correction(<<~RUBY)
expect(a).not_to include(b)
RUBY
end

it 'does not register an offense when using `expect(a).to include(b)`' do
expect_no_offenses(<<~RUBY)
expect(a).to include(b)
RUBY
end

it 'does not register an offense when ' \
'using `expect(a).not_to include(b)`' do
expect_no_offenses(<<~RUBY)
expect(a).not_to include(b)
RUBY
end
end

Expand Down