Merge pull request #162 from koic/make_assert_nil_aware_of_assert_pre…

…dicate

Make `Minitest/AssertNil` aware of `assert_predicate`

changelog/change_make_assert_nil_aware_of_assert_predicate.md

@@ -0,0 +1 @@
+* [#162](https://github.com/rubocop/rubocop-minitest/pull/162): Make `Minitest/AssertNil` (`Minitest/RefuteNil`) aware of `assert_predicate(obj, :nil?)` (`refute_predicate(obj, :nil?)`). ([@koic][])

lib/rubocop/cop/minitest/assert_nil.rb

@@ -4,14 +4,16 @@ module RuboCop
   module Cop
     module Minitest
       # This cop enforces the test to use `assert_nil` instead of using
-      # `assert_equal(nil, something)` or `assert(something.nil?)`.
+      # `assert_equal(nil, something)`, `assert(something.nil?)`, or `assert_predicate(something, :nil?)`.
       #
       # @example
       #   # bad
       #   assert_equal(nil, actual)
       #   assert_equal(nil, actual, 'message')
       #   assert(object.nil?)
       #   assert(object.nil?, 'message')
+      #   assert_predicate(object, :nil?)
+      #   assert_predicate(object, :nil?, 'message')
       #
       #   # good
       #   assert_nil(actual)
@@ -23,12 +25,13 @@ class AssertNil < Base
         extend AutoCorrector
 
         ASSERTION_TYPE = 'assert'
-        RESTRICT_ON_SEND = %i[assert_equal assert].freeze
+        RESTRICT_ON_SEND = %i[assert assert_equal assert_predicate].freeze
 
         def_node_matcher :nil_assertion, <<~PATTERN
           {
             (send nil? :assert_equal nil $_ $...)
             (send nil? :assert (send $_ :nil?) $...)
+            (send nil? :assert_predicate $_ (sym :nil?) $...)
           }
         PATTERN
 

lib/rubocop/cop/minitest/refute_nil.rb

@@ -4,14 +4,16 @@ module RuboCop
   module Cop
     module Minitest
       # This cop enforces the test to use `refute_nil` instead of using
-      # `refute_equal(nil, something)` or `refute(something.nil?)`.
+      # `refute_equal(nil, something)`, `refute(something.nil?)`, or `refute_predicate(something, :nil?)`.
       #
       # @example
       #   # bad
       #   refute_equal(nil, actual)
       #   refute_equal(nil, actual, 'message')
       #   refute(actual.nil?)
       #   refute(actual.nil?, 'message')
+      #   refute_predicate(object, :nil?)
+      #   refute_predicate(object, :nil?, 'message')
       #
       #   # good
       #   refute_nil(actual)
@@ -23,12 +25,13 @@ class RefuteNil < Base
         extend AutoCorrector
 
         ASSERTION_TYPE = 'refute'
-        RESTRICT_ON_SEND = %i[refute_equal refute].freeze
+        RESTRICT_ON_SEND = %i[refute refute_equal refute_predicate].freeze
 
         def_node_matcher :nil_refutation, <<~PATTERN
           {
             (send nil? :refute_equal nil $_ $...)
             (send nil? :refute (send $_ :nil?) $...)
+            (send nil? :refute_predicate $_ (sym :nil?) $...)
           }
         PATTERN
 

lib/rubocop/cop/mixin/nil_assertion_handleable.rb

@@ -5,7 +5,7 @@ module Cop
     module Minitest
       # Common functionality for `AssertNil` and `RefuteNil` cops.
       module NilAssertionHandleable
-        MSG = 'Prefer using `%<assertion_type>s_nil(%<preferred_args>s)` over `%<method>s(%<current_args>s)`.'
+        MSG = 'Prefer using `%<assertion_type>s_nil(%<preferred_args>s)`.'
 
         private
 
@@ -22,32 +22,27 @@ def build_message(node, actual, message)
           message_source = message&.source
 
           preferred_args = [actual.source, message_source].compact
-          current_args = if comparison_assertion_method?(node)
-                           ['nil', preferred_args].join(', ')
-                         else
-                           ["#{actual.source}.nil?", message_source].compact.join(', ')
-                         end
 
           format(
             MSG,
             assertion_type: assertion_type,
             preferred_args: preferred_args.join(', '),
-            method: node.method_name, current_args: current_args
+            method: node.method_name
           )
         end
 
         def autocorrect(corrector, node, actual)
           corrector.replace(node.loc.selector, :"#{assertion_type}_nil")
-          if comparison_assertion_method?(node)
+          if comparison_or_predicate_assertion_method?(node)
             corrector.replace(first_and_second_arguments_range(node), actual.source)
           else
             corrector.remove(node.first_argument.loc.dot)
             corrector.remove(node.first_argument.loc.selector)
           end
         end
 
-        def comparison_assertion_method?(node)
-          node.method?(:"#{assertion_type}_equal")
+        def comparison_or_predicate_assertion_method?(node)
+          node.method?(:"#{assertion_type}_equal") || node.method?(:"#{assertion_type}_predicate")
         end
       end
     end

test/rubocop/cop/minitest/assert_nil_test.rb

@@ -8,7 +8,7 @@ def test_registers_offense_when_using_assert_equal_with_nil
       class FooTest < Minitest::Test
         def test_do_something
           assert_equal(nil, somestuff)
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff)` over `assert_equal(nil, somestuff)`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff)`.
         end
       end
     RUBY
@@ -27,7 +27,7 @@ def test_registers_offense_when_using_assert_equal_with_nil_and_message
       class FooTest < Minitest::Test
         def test_do_something
           assert_equal(nil, somestuff, 'message')
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff, 'message')` over `assert_equal(nil, somestuff, 'message')`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff, 'message')`.
         end
       end
     RUBY
@@ -46,7 +46,7 @@ def test_registers_offense_when_using_assert_equal_with_a_method_call
       class FooTest < Minitest::Test
         def test_do_something
           assert_equal(nil, obj.do_something, 'message')
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(obj.do_something, 'message')` over `assert_equal(nil, obj.do_something, 'message')`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(obj.do_something, 'message')`.
         end
       end
     RUBY
@@ -65,7 +65,7 @@ def test_registers_offense_when_using_assert_equal_with_nil_and_heredoc_message
       class FooTest < Minitest::Test
         def test_do_something
           assert_equal(nil, obj.do_something, <<~MESSAGE
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(obj.do_something, <<~MESSAGE)` over `assert_equal(nil, obj.do_something, <<~MESSAGE)`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(obj.do_something, <<~MESSAGE)`.
             message
           MESSAGE
           )
@@ -90,7 +90,7 @@ def test_registers_offense_when_using_assert_with_nil_predicate_method_call
       class FooTest < Minitest::Test
         def test_do_something
           assert(somestuff.nil?)
-          ^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff)` over `assert(somestuff.nil?)`.
+          ^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff)`.
         end
       end
     RUBY
@@ -109,7 +109,7 @@ def test_registers_offense_when_using_assert_with_nil_predicate_method_call_and_
       class FooTest < Minitest::Test
         def test_do_something
           assert(somestuff.nil?, 'message')
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff, 'message')` over `assert(somestuff.nil?, 'message')`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(somestuff, 'message')`.
         end
       end
     RUBY
@@ -123,6 +123,44 @@ def test_do_something
     RUBY
   end
 
+  def test_registers_offense_when_using_assert_predicate
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_predicate(object, :nil?)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(object)`.
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_nil(object)
+        end
+      end
+    RUBY
+  end
+
+  def test_registers_offense_when_using_assert_predicate_with_message
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_predicate(object, :nil?, 'message')
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_nil(object, 'message')`.
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_nil(object, 'message')
+        end
+      end
+    RUBY
+  end
+
   def test_does_not_register_offense_when_using_assert_nil_method
     assert_no_offenses(<<~RUBY)
       class FooTest < Minitest::Test

test/rubocop/cop/minitest/refute_nil_test.rb

@@ -8,7 +8,7 @@ def test_registers_offense_when_using_refute_equal_with_nil
       class FooTest < Minitest::Test
         def test_do_something
           refute_equal(nil, somestuff)
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff)` over `refute_equal(nil, somestuff)`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff)`.
         end
       end
     RUBY
@@ -27,7 +27,7 @@ def test_registers_offense_when_using_refute_equal_with_nil_and_message
       class FooTest < Minitest::Test
         def test_do_something
           refute_equal(nil, somestuff, 'message')
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff, 'message')` over `refute_equal(nil, somestuff, 'message')`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff, 'message')`.
         end
       end
     RUBY
@@ -46,7 +46,7 @@ def test_registers_offense_when_using_refute_equal_with_a_method_call
       class FooTest < Minitest::Test
         def test_do_something
           refute_equal(nil, obj.do_something, 'message')
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(obj.do_something, 'message')` over `refute_equal(nil, obj.do_something, 'message')`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(obj.do_something, 'message')`.
         end
       end
     RUBY
@@ -65,7 +65,7 @@ def test_registers_offense_when_using_refute_equal_with_nil_and_heredoc_message
       class FooTest < Minitest::Test
         def test_do_something
           refute_equal(nil, obj.do_something, <<~MESSAGE
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(obj.do_something, <<~MESSAGE)` over `refute_equal(nil, obj.do_something, <<~MESSAGE)`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(obj.do_something, <<~MESSAGE)`.
             message
           MESSAGE
           )
@@ -90,7 +90,7 @@ def test_registers_offense_when_using_refute_with_nil_predicate_method_call
       class FooTest < Minitest::Test
         def test_do_something
           refute(somestuff.nil?)
-          ^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff)` over `refute(somestuff.nil?)`.
+          ^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff)`.
         end
       end
     RUBY
@@ -109,7 +109,7 @@ def test_registers_offense_when_using_refute_with_nil_predicate_method_call_and_
       class FooTest < Minitest::Test
         def test_do_something
           refute(somestuff.nil?, 'message')
-          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff, 'message')` over `refute(somestuff.nil?, 'message')`.
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(somestuff, 'message')`.
         end
       end
     RUBY
@@ -123,6 +123,44 @@ def test_do_something
     RUBY
   end
 
+  def test_registers_offense_when_using_refute_predicate
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_predicate(object, :nil?)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(object)`.
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_nil(object)
+        end
+      end
+    RUBY
+  end
+
+  def test_registers_offense_when_using_refute_predicate_with_message
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_predicate(object, :nil?, 'message')
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `refute_nil(object, 'message')`.
+        end
+      end
+    RUBY
+
+    assert_correction(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          refute_nil(object, 'message')
+        end
+      end
+    RUBY
+  end
+
   def test_does_not_register_offense_when_using_refute_nil_method
     assert_no_offenses(<<~RUBY)
       class FooTest < Minitest::Test