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`

changelog/new_make_instance_of_cops_aware_of_equal.md

@@ -0,0 +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

@@ -11,14 +11,30 @@ module Minitest
       #   assert(object.instance_of?(Class))
       #   assert(object.instance_of?(Class), 'message')
       #
+      #   # bad
+      #   assert_equal(Class, object.class)
+      #   assert_equal(Class, object.class, 'message')
+      #
       #   # good
       #   assert_instance_of(Class, object)
       #   assert_instance_of(Class, object, 'message')
       #
       class AssertInstanceOf < Base
-        extend MinitestCopRule
+        include InstanceOfAssertionHandleable
+        extend AutoCorrector
+
+        RESTRICT_ON_SEND = %i[assert assert_equal].freeze
+
+        def_node_matcher :instance_of_assertion?, <<~PATTERN
+          {
+            (send nil? :assert (send $_ :instance_of? $const) $_?)
+            (send nil? :assert_equal $const (send $_ :class) $_?)
+          }
+        PATTERN
 
-        define_rule :assert, target_method: :instance_of?, inverse: true
+        def on_send(node)
+          investigate(node, :assert)
+        end
       end
     end
   end

lib/rubocop/cop/minitest/refute_instance_of.rb

@@ -11,14 +11,30 @@ module Minitest
       #   refute(object.instance_of?(Class))
       #   refute(object.instance_of?(Class), 'message')
       #
+      #   # bad
+      #   refute_equal(Class, object.class)
+      #   refute_equal(Class, object.class, 'message')
+      #
       #   # good
       #   refute_instance_of(Class, object)
       #   refute_instance_of(Class, object, 'message')
       #
       class RefuteInstanceOf < Base
-        extend MinitestCopRule
+        include InstanceOfAssertionHandleable
+        extend AutoCorrector
+
+        RESTRICT_ON_SEND = %i[refute refute_equal].freeze
+
+        def_node_matcher :instance_of_assertion?, <<~PATTERN
+          {
+            (send nil? :refute (send $_ :instance_of? $const) $_?)
+            (send nil? :refute_equal $const (send $_ :class) $_?)
+          }
+        PATTERN
 
-        define_rule :refute, target_method: :instance_of?, inverse: true
+        def on_send(node)
+          investigate(node, :refute)
+        end
       end
     end
   end

lib/rubocop/cop/minitest_cops.rb

@@ -2,6 +2,7 @@
 
 require_relative 'mixin/argument_range_helper'
 require_relative 'mixin/in_delta_mixin'
+require_relative 'mixin/instance_of_assertion_handleable'
 require_relative 'mixin/minitest_cop_rule'
 require_relative 'mixin/minitest_exploration_helpers'
 require_relative 'mixin/nil_assertion_handleable'

lib/rubocop/cop/mixin/instance_of_assertion_handleable.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Minitest
+      # Common functionality for `Minitest/AssertInstanceOf` and `Minitest/RefuteInstanceOf` cops.
+      # @api private
+      module InstanceOfAssertionHandleable
+        include ArgumentRangeHelper
+
+        MSG = 'Prefer using `%<prefer>s`.'
+
+        private
+
+        def investigate(node, assertion_type)
+          return unless (first_capture, second_capture, message = instance_of_assertion?(node))
+
+          required_arguments = build_required_arguments(node, assertion_type, first_capture, second_capture)
+          full_arguments = [required_arguments, message.first&.source].compact.join(', ')
+          prefer = "#{assertion_type}_instance_of(#{full_arguments})"
+
+          add_offense(node, message: format(MSG, prefer: prefer)) do |corrector|
+            range = replacement_range(node, assertion_type)
+
+            corrector.replace(node.loc.selector, "#{assertion_type}_instance_of")
+            corrector.replace(range, required_arguments)
+          end
+        end
+
+        def build_required_arguments(node, method_name, first_capture, second_capture)
+          if node.method?(method_name)
+            [second_capture, first_capture]
+          else
+            [first_capture, second_capture]
+          end.map(&:source).join(', ')
+        end
+
+        def replacement_range(node, method_name)
+          if node.method?(method_name)
+            node.first_argument
+          else
+            first_and_second_arguments_range(node)
+          end
+        end
+      end
+    end
+  end
+end

test/rubocop/cop/minitest/assert_instance_of_test.rb

@@ -66,20 +66,64 @@ def test_do_something
     RUBY
   end
 
-  def test_registers_offense_when_using_assert_with_instance_of_in_redundant_parentheses
+  def test_registers_offense_when_using_assert_equal_with_class_arguments
     assert_offense(<<~RUBY)
       class FooTest < Minitest::Test
         def test_do_something
-          assert((object.instance_of?(SomeClass)))
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, object)`.
+          assert_equal(SomeClass, obj.class)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, obj)`.
         end
       end
     RUBY
 
     assert_correction(<<~RUBY)
       class FooTest < Minitest::Test
         def test_do_something
-          assert_instance_of((SomeClass, object))
+          assert_instance_of(SomeClass, obj)
+        end
+      end
+    RUBY
+  end
+
+  def test_registers_offense_when_using_assert_equal_with_class_arguments_and_message
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_equal(SomeClass, obj.class, 'message')
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, obj, 'message')`.
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_instance_of(SomeClass, obj, 'message')
+        end
+      end
+    RUBY
+  end
+
+  def test_registers_offense_when_using_assert_equal_with_class_arguments_and_heredoc_message
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_equal(SomeClass, obj.class, <<~MESSAGE
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_instance_of(SomeClass, obj, <<~MESSAGE)`.
+            message
+          MESSAGE
+          )
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_instance_of(SomeClass, obj, <<~MESSAGE
+            message
+          MESSAGE
+          )
         end
       end
     RUBY

test/rubocop/cop/minitest/refute_instance_of_test.rb

@@ -66,26 +66,70 @@ def test_do_something
     RUBY
   end
 
-  def test_registers_offense_when_using_refute_with_instance_of_in_redundant_parentheses
+  def test_registers_offense_when_using_refute_equal_with_class_arguments
     assert_offense(<<~RUBY)
       class FooTest < Minitest::Test
         def test_do_something
-          refute((object.instance_of?(SomeClass)))
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, object)`.
+          refute_equal(SomeClass, obj.class)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, obj)`.
         end
       end
     RUBY
 
     assert_correction(<<~RUBY)
       class FooTest < Minitest::Test
         def test_do_something
-          refute_instance_of((SomeClass, object))
+          refute_instance_of(SomeClass, obj)
         end
       end
     RUBY
   end
 
-  def test_refute_instance_of_method
+  def test_registers_offense_when_using_refute_equal_with_class_arguments_and_message
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_equal(SomeClass, obj.class, 'message')
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, obj, 'message')`.
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_instance_of(SomeClass, obj, 'message')
+        end
+      end
+    RUBY
+  end
+
+  def test_registers_offense_when_using_refute_equal_with_class_arguments_and_heredoc_message
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_equal(SomeClass, obj.class, <<~MESSAGE
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_instance_of(SomeClass, obj, <<~MESSAGE)`.
+            message
+          MESSAGE
+          )
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_instance_of(SomeClass, obj, <<~MESSAGE
+            message
+          MESSAGE
+          )
+        end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_using_refute_instance_of_method
     assert_no_offenses(<<~RUBY)
       class FooTest < Minitest::Test
         def test_do_something