Merge pull request #1773 from G-Rath/assert-nil

feat: support correcting `assert_nil` and `refute_nil` to `RSpec/Rails/MinitestAssertions`

Author: pirj

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Master (Unreleased)
 
+- Support correcting `assert_nil` and `refute_nil` to `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
+
 ## 2.26.1 (2024-01-05)
 
 - Fix an error for `RSpec/SharedExamples` when using examples without argument. ([@ydah])

config/default.yml

@@ -1169,6 +1169,7 @@ RSpec/Rails/MinitestAssertions:
   Description: Check if using Minitest matchers.
   Enabled: pending
   VersionAdded: '2.17'
+  VersionChanged: "<<next>>"
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/MinitestAssertions
 
 RSpec/Rails/NegationBeValid:

docs/modules/ROOT/pages/cops_rspecrails.adoc

@@ -243,7 +243,7 @@ end
 | Yes
 | Yes
 | 2.17
-| -
+| <<next>>
 |===
 
 Check if using Minitest matchers.
@@ -257,10 +257,16 @@ assert_equal(a, b)
 assert_equal a, b, "must be equal"
 refute_equal(a, b)
 
+assert_nil a
+refute_nil a
+
 # good
 expect(b).to eq(a)
 expect(b).to(eq(a), "must be equal")
 expect(b).not_to eq(a)
+
+expect(a).to eq(nil)
+expect(a).not_to eq(nil)
 ----
 
 === References

lib/rubocop/cop/rspec_rails/minitest_assertions.rb

@@ -12,35 +12,61 @@ module Rails
         #   assert_equal a, b, "must be equal"
         #   refute_equal(a, b)
         #
+        #   assert_nil a
+        #   refute_nil a
+        #
         #   # good
         #   expect(b).to eq(a)
         #   expect(b).to(eq(a), "must be equal")
         #   expect(b).not_to eq(a)
         #
+        #   expect(a).to eq(nil)
+        #   expect(a).not_to eq(nil)
+        #
         class MinitestAssertions < Base
           extend AutoCorrector
 
           MSG = 'Use `%<prefer>s`.'
-          RESTRICT_ON_SEND = %i[assert_equal refute_equal].freeze
+          RESTRICT_ON_SEND = %i[
+            assert_equal
+            refute_equal
+            assert_nil
+            refute_nil
+          ].freeze
 
-          # @!method minitest_assertion(node)
-          def_node_matcher :minitest_assertion, <<~PATTERN
+          # @!method minitest_equal_assertion(node)
+          def_node_matcher :minitest_equal_assertion, <<~PATTERN
             (send nil? {:assert_equal :refute_equal} $_ $_ $_?)
           PATTERN
 
+          # @!method minitest_nil_assertion(node)
+          def_node_matcher :minitest_nil_assertion, <<~PATTERN
+            (send nil? {:assert_nil :refute_nil} $_ $_?)
+          PATTERN
+
           def on_send(node)
-            minitest_assertion(node) do |expected, actual, failure_message|
-              prefer = replacement(node, expected, actual,
-                                   failure_message.first)
-              add_offense(node, message: message(prefer)) do |corrector|
-                corrector.replace(node, prefer)
-              end
+            minitest_equal_assertion(node) do |expected, actual, fail_message|
+              prefer = replace_equal_assertion(node, expected, actual,
+                                               fail_message.first)
+              add_an_offense(node, prefer)
+            end
+
+            minitest_nil_assertion(node) do |actual, fail_message|
+              prefer = replace_nil_assertion(node, actual,
+                                             fail_message.first)
+              add_an_offense(node, prefer)
             end
           end
 
           private
 
-          def replacement(node, expected, actual, failure_message)
+          def add_an_offense(node, prefer)
+            add_offense(node, message: message(prefer)) do |corrector|
+              corrector.replace(node, prefer)
+            end
+          end
+
+          def replace_equal_assertion(node, expected, actual, failure_message)
             runner = node.method?(:assert_equal) ? 'to' : 'not_to'
             if failure_message.nil?
               "expect(#{actual.source}).#{runner} eq(#{expected.source})"
@@ -50,6 +76,16 @@ def replacement(node, expected, actual, failure_message)
             end
           end
 
+          def replace_nil_assertion(node, actual, failure_message)
+            runner = node.method?(:assert_nil) ? 'to' : 'not_to'
+            if failure_message.nil?
+              "expect(#{actual.source}).#{runner} eq(nil)"
+            else
+              "expect(#{actual.source}).#{runner}(eq(nil), " \
+                "#{failure_message.source})"
+            end
+          end
+
           def message(prefer)
             format(MSG, prefer: prefer)
           end

spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb

@@ -1,73 +1,146 @@
 # frozen_string_literal: true
 
 RSpec.describe RuboCop::Cop::RSpec::Rails::MinitestAssertions do
-  it 'registers an offense when using `assert_equal`' do
-    expect_offense(<<~RUBY)
-      assert_equal(a, b)
-      ^^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`.
-    RUBY
-
-    expect_correction(<<~RUBY)
-      expect(b).to eq(a)
-    RUBY
-  end
+  context 'with equal assertions' do
+    it 'registers an offense when using `assert_equal`' do
+      expect_offense(<<~RUBY)
+        assert_equal(a, b)
+        ^^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`.
+      RUBY
 
-  it 'registers an offense when using `assert_equal` with no parentheses' do
-    expect_offense(<<~RUBY)
-      assert_equal a, b
-      ^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`.
-    RUBY
+      expect_correction(<<~RUBY)
+        expect(b).to eq(a)
+      RUBY
+    end
 
-    expect_correction(<<~RUBY)
-      expect(b).to eq(a)
-    RUBY
-  end
+    it 'registers an offense when using `assert_equal` with no parentheses' do
+      expect_offense(<<~RUBY)
+        assert_equal a, b
+        ^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`.
+      RUBY
 
-  it 'registers an offense when using `assert_equal` with failure message' do
-    expect_offense(<<~RUBY)
-      assert_equal a, b, "must be equal"
-      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`.
-    RUBY
+      expect_correction(<<~RUBY)
+        expect(b).to eq(a)
+      RUBY
+    end
 
-    expect_correction(<<~RUBY)
-      expect(b).to(eq(a), "must be equal")
-    RUBY
-  end
+    it 'registers an offense when using `assert_equal` with failure message' do
+      expect_offense(<<~RUBY)
+        assert_equal a, b, "must be equal"
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`.
+      RUBY
 
-  it 'registers an offense when using `assert_equal` with ' \
-     'multi-line arguments' do
-    expect_offense(<<~RUBY)
-      assert_equal(a,
-      ^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`.
-                    b,
-                    "must be equal")
-    RUBY
-
-    expect_correction(<<~RUBY)
-      expect(b).to(eq(a), "must be equal")
-    RUBY
-  end
+      expect_correction(<<~RUBY)
+        expect(b).to(eq(a), "must be equal")
+      RUBY
+    end
 
-  it 'registers an offense when using `refute_equal`' do
-    expect_offense(<<~RUBY)
-      refute_equal a, b
-      ^^^^^^^^^^^^^^^^^ Use `expect(b).not_to eq(a)`.
-    RUBY
+    it 'registers an offense when using `assert_equal` with ' \
+       'multi-line arguments' do
+      expect_offense(<<~RUBY)
+        assert_equal(a,
+        ^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`.
+                      b,
+                      "must be equal")
+      RUBY
 
-    expect_correction(<<~RUBY)
-      expect(b).not_to eq(a)
-    RUBY
-  end
+      expect_correction(<<~RUBY)
+        expect(b).to(eq(a), "must be equal")
+      RUBY
+    end
 
-  it 'does not register an offense when using `expect(b).to eq(a)`' do
-    expect_no_offenses(<<~RUBY)
-      expect(b).to eq(a)
-    RUBY
+    it 'registers an offense when using `refute_equal`' do
+      expect_offense(<<~RUBY)
+        refute_equal a, b
+        ^^^^^^^^^^^^^^^^^ Use `expect(b).not_to eq(a)`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(b).not_to eq(a)
+      RUBY
+    end
+
+    it 'does not register an offense when using `expect(b).to eq(a)`' do
+      expect_no_offenses(<<~RUBY)
+        expect(b).to eq(a)
+      RUBY
+    end
+
+    it 'does not register an offense when using `expect(b).not_to eq(a)`' do
+      expect_no_offenses(<<~RUBY)
+        expect(b).not_to eq(a)
+      RUBY
+    end
   end
 
-  it 'does not register an offense when using `expect(b).not_to eq(a)`' do
-    expect_no_offenses(<<~RUBY)
-      expect(b).not_to eq(a)
-    RUBY
+  context 'with nil assertions' do
+    it 'registers an offense when using `assert_nil`' do
+      expect_offense(<<~RUBY)
+        assert_nil(a)
+        ^^^^^^^^^^^^^ Use `expect(a).to eq(nil)`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to eq(nil)
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_nil` with no parentheses' do
+      expect_offense(<<~RUBY)
+        assert_nil a
+        ^^^^^^^^^^^^ Use `expect(a).to eq(nil)`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to eq(nil)
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_nil` with failure message' do
+      expect_offense(<<~RUBY)
+        assert_nil a, "must be nil"
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to(eq(nil), "must be nil")`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to(eq(nil), "must be nil")
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_nil` with ' \
+       'multi-line arguments' do
+      expect_offense(<<~RUBY)
+        assert_nil(a,
+        ^^^^^^^^^^^^^ Use `expect(a).to(eq(nil), "must be nil")`.
+                      "must be nil")
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to(eq(nil), "must be nil")
+      RUBY
+    end
+
+    it 'registers an offense when using `refute_nil`' do
+      expect_offense(<<~RUBY)
+        refute_nil a
+        ^^^^^^^^^^^^ Use `expect(a).not_to eq(nil)`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).not_to eq(nil)
+      RUBY
+    end
+
+    it 'does not register an offense when using `expect(a).to eq(nil)`' do
+      expect_no_offenses(<<~RUBY)
+        expect(a).to eq(nil)
+      RUBY
+    end
+
+    it 'does not register an offense when using `expect(a).not_to eq(nil)`' do
+      expect_no_offenses(<<~RUBY)
+        expect(a).not_to eq(nil)
+      RUBY
+    end
   end
 end