Merge pull request #1780 from G-Rath/support-assert-instance-of

feat: support `*_instance_of` in `RSpec/Rails/MinitestAssertions`

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])

lib/rubocop/cop/rspec/rails/minitest_assertions.rb

@@ -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
@@ -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,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
@@ -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})"
             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

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