Merge pull request #1778 from rubocop/support-assert-empty

Add support `assert_empty`, `assert_not_empty` and `refute_empty` for `RSpec/Rails/MinitestAssertions`

Author: ydah

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Master (Unreleased)
 
+- Add support for `assert_empty`, `assert_not_empty` and `refute_empty` to `RSpec/Rails/MinitestAssertions`. ([@ydah])
 - Support correcting `assert_nil` and `refute_nil` in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
 - Support correcting `assert_not_equal` and `assert_not_equal` in `RSpec/Rails/MinitestAssertions`. ([@G-Rath])
 - Fix a false positive for `RSpec/ExpectActual` when used with rspec-rails routing matchers. ([@naveg])

docs/modules/ROOT/pages/cops_rspecrails.adoc

@@ -256,17 +256,15 @@ Check if using Minitest matchers.
 assert_equal(a, b)
 assert_equal a, b, "must be equal"
 refute_equal(a, b)
-
 assert_nil a
-refute_nil a
+refute_empty(b)
 
 # 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)
+expect(a).not_to be_empty
 ----
 
 === References

lib/rubocop/cop/rspec_rails/minitest_assertions.rb

@@ -11,17 +11,15 @@ module Rails
         #   assert_equal(a, b)
         #   assert_equal a, b, "must be equal"
         #   refute_equal(a, b)
-        #
         #   assert_nil a
-        #   refute_nil a
+        #   refute_empty(b)
         #
         #   # 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)
+        #   expect(a).not_to be_empty
         #
         class MinitestAssertions < Base
           extend AutoCorrector
@@ -30,66 +28,111 @@ class MinitestAssertions < Base
           RESTRICT_ON_SEND = %i[
             assert_equal
             assert_not_equal
-            refute_equal
             assert_nil
             assert_not_nil
+            assert_empty
+            assert_not_empty
+            refute_equal
             refute_nil
+            refute_empty
           ].freeze
 
-          # @!method minitest_equal_assertion(node)
-          def_node_matcher :minitest_equal_assertion, <<~PATTERN
+          # @!method minitest_equal(node)
+          def_node_matcher :minitest_equal, <<~PATTERN
             (send nil? {:assert_equal :assert_not_equal :refute_equal} $_ $_ $_?)
           PATTERN
 
-          # @!method minitest_nil_assertion(node)
-          def_node_matcher :minitest_nil_assertion, <<~PATTERN
+          # @!method minitest_nil(node)
+          def_node_matcher :minitest_nil, <<~PATTERN
             (send nil? {:assert_nil :assert_not_nil :refute_nil} $_ $_?)
           PATTERN
 
-          def on_send(node)
-            minitest_equal_assertion(node) do |expected, actual, fail_message|
-              prefer = replace_equal_assertion(node, expected, actual,
-                                               fail_message.first)
-              add_an_offense(node, prefer)
+          # @!method minitest_empty(node)
+          def_node_matcher :minitest_empty, <<~PATTERN
+            (send nil? {:assert_empty :assert_not_empty :refute_empty} $_ $_?)
+          PATTERN
+
+          def on_send(node) # rubocop:disable Metrics/MethodLength
+            minitest_equal(node) do |expected, actual, failure_message|
+              on_assertion(node, EqualAssertion.new(expected, actual,
+                                                    failure_message.first))
             end
 
-            minitest_nil_assertion(node) do |actual, fail_message|
-              prefer = replace_nil_assertion(node, actual,
-                                             fail_message.first)
-              add_an_offense(node, prefer)
+            minitest_nil(node) do |actual, failure_message|
+              on_assertion(node, NilAssertion.new(actual,
+                                                  failure_message.first))
             end
-          end
 
-          private
+            minitest_empty(node) do |actual, failure_message|
+              on_assertion(node, EmptyAssertion.new(actual,
+                                                    failure_message.first))
+            end
+          end
 
-          def add_an_offense(node, prefer)
-            add_offense(node, message: message(prefer)) do |corrector|
-              corrector.replace(node, prefer)
+          def on_assertion(node, assertion)
+            preferred = assertion.replaced(node)
+            add_offense(node, message: message(preferred)) do |corrector|
+              corrector.replace(node, preferred)
             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})"
-            else
-              "expect(#{actual.source}).#{runner}(eq(#{expected.source}), " \
-                "#{failure_message.source})"
+          def message(preferred)
+            format(MSG, prefer: preferred)
+          end
+
+          # :nodoc:
+          class EqualAssertion
+            def initialize(expected, actual, fail_message)
+              @expected = expected
+              @actual = actual
+              @fail_message = fail_message
+            end
+
+            def replaced(node)
+              runner = node.method?(:assert_equal) ? 'to' : 'not_to'
+              if @fail_message.nil?
+                "expect(#{@actual.source}).#{runner} eq(#{@expected.source})"
+              else
+                "expect(#{@actual.source}).#{runner}(eq(#{@expected.source})," \
+                  " #{@fail_message.source})"
+              end
             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})"
+          # :nodoc:
+          class NilAssertion
+            def initialize(actual, fail_message)
+              @actual = actual
+              @fail_message = fail_message
+            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
             end
           end
 
-          def message(prefer)
-            format(MSG, prefer: prefer)
+          # :nodoc:
+          class EmptyAssertion
+            def initialize(actual, fail_message)
+              @actual = actual
+              @fail_message = fail_message
+            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
+            end
           end
         end
       end

spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb

@@ -164,5 +164,85 @@
         expect(a).not_to eq(nil)
       RUBY
     end
+
+    it 'registers an offense when using `assert_empty`' do
+      expect_offense(<<~RUBY)
+        assert_empty(a)
+        ^^^^^^^^^^^^^^^ Use `expect(a).to be_empty`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to be_empty
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_empty` with no parentheses' do
+      expect_offense(<<~RUBY)
+        assert_empty a
+        ^^^^^^^^^^^^^^ Use `expect(a).to be_empty`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to be_empty
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_empty` with failure message' do
+      expect_offense(<<~RUBY)
+        assert_empty a, "must be empty"
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to(be_empty, "must be empty")`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to(be_empty, "must be empty")
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_empty` with ' \
+       'multi-line arguments' do
+      expect_offense(<<~RUBY)
+        assert_empty(a,
+        ^^^^^^^^^^^^^^^ Use `expect(a).to(be_empty, "must be empty")`.
+                      "must be empty")
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).to(be_empty, "must be empty")
+      RUBY
+    end
+
+    it 'registers an offense when using `assert_not_empty`' do
+      expect_offense(<<~RUBY)
+        assert_not_empty a
+        ^^^^^^^^^^^^^^^^^^ Use `expect(a).not_to be_empty`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).not_to be_empty
+      RUBY
+    end
+
+    it 'registers an offense when using `refute_empty`' do
+      expect_offense(<<~RUBY)
+        refute_empty a
+        ^^^^^^^^^^^^^^ Use `expect(a).not_to be_empty`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        expect(a).not_to be_empty
+      RUBY
+    end
+
+    it 'does not register an offense when using `expect(a).to be_empty`' do
+      expect_no_offenses(<<~RUBY)
+        expect(a).to be_empty
+      RUBY
+    end
+
+    it 'does not register an offense when using `expect(a).not_to be_empty`' do
+      expect_no_offenses(<<~RUBY)
+        expect(a).not_to be_empty
+      RUBY
+    end
   end
 end