diff --git a/CHANGELOG.md b/CHANGELOG.md index 23d633774..44c69da08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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]) diff --git a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb index 897300543..41a802ac9 100644 --- a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb +++ b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb @@ -30,6 +30,8 @@ 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 @@ -37,6 +39,7 @@ class MinitestAssertions < Base assert_empty assert_not_empty refute_equal + refute_instance_of refute_includes refute_nil refute_empty @@ -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} $_ $_ $_?) @@ -68,6 +76,11 @@ 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, failure_message.first)) @@ -114,6 +127,26 @@ def replaced(node) end end + # :nodoc: + class InstanceOfAssertion + def initialize(expected, actual, fail_message) + @expected = expected + @actual = actual + @fail_message = fail_message + end + + def replaced(node) + runner = node.method?(:assert_instance_of) ? 'to' : 'not_to' + if @fail_message.nil? + "expect(#{@actual.source}).#{runner} be_an_instance_of" \ + "(#{@expected.source})" + else + "expect(#{@actual.source}).#{runner}(be_an_instance_of" \ + "(#{@expected.source}), #{@fail_message.source})" + end + end + end + # :nodoc: class IncludesAssertion def initialize(collection, expected, fail_message) diff --git a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb index 7b5df9b1a..72598f107 100644 --- a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb +++ b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb @@ -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) @@ -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