[Fix #117] Add new Minitest/AssertWithExpectedArgument cop

It is a common mistake to use `assert` instead of `assert_equal`. This
is dangerous because minitest will treat the second argument to `assert`
as the `msg` parameter and, as long as the first argument is truthy, the
test will always pass. This false negative gives the user a sense of
security even though they're not actually testing what they think they
are.

This commit introduces a new cop which will register a violation if
`assert` is called with two arguments and the second argument isn't a
`String`. If the second argument *is* a `String`, then the user is
likely using `assert` as intended. If it's not, most of the time it
means a mistake has been made.

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* [#117](https://github.com/rubocop/rubocop-minitest/issues/117): Add new cop `Minitest/AssertWithExpectedArgument` to check for unintended usages of `assert` instead of `assert_equal`. ([@cstyles][])
+
 ### Bug fixes
 
 * [#118](https://github.com/rubocop/rubocop-minitest/pull/118): **(BREAKING)** Fix `Minitest/AssertEmptyLiteral` by making it check for `assert_equal([], array)` instead of `assert([], array)`. ([@cstyles][])

config/default.yml

@@ -94,6 +94,11 @@ Minitest/AssertTruthy:
   Enabled: true
   VersionAdded: '0.2'
 
+Minitest/AssertWithExpectedArgument:
+  Description: 'This cop tries to detect when a user accidentally used `assert` when they meant to use `assert_equal`.'
+  Enabled: pending
+  VersionAdded: '0.11'
+
 Minitest/GlobalExpectations:
   Description: 'This cop checks for deprecated global expectations.'
   StyleGuide: 'https://minitest.rubystyle.guide#global-expectations'

docs/modules/ROOT/pages/cops.adoc

@@ -27,6 +27,7 @@ based on the https://minitest.rubystyle.guide/[Minitest Style Guide].
 * xref:cops_minitest.adoc#minitestassertrespondto[Minitest/AssertRespondTo]
 * xref:cops_minitest.adoc#minitestassertsilent[Minitest/AssertSilent]
 * xref:cops_minitest.adoc#minitestasserttruthy[Minitest/AssertTruthy]
+* xref:cops_minitest.adoc#minitestassertwithexpectedargument[Minitest/AssertWithExpectedArgument]
 * xref:cops_minitest.adoc#minitestassertioninlifecyclehook[Minitest/AssertionInLifecycleHook]
 * xref:cops_minitest.adoc#minitestglobalexpectations[Minitest/GlobalExpectations]
 * xref:cops_minitest.adoc#minitestliteralasactualargument[Minitest/LiteralAsActualArgument]

docs/modules/ROOT/pages/cops_minitest.adoc

@@ -441,6 +441,35 @@ assert(actual, 'message')
 
 * https://minitest.rubystyle.guide#assert-truthy
 
+== Minitest/AssertWithExpectedArgument
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Pending
+| Yes
+| No
+| 0.11
+| -
+|===
+
+This cop tries to detect when a user accidentally used
+`assert` when they meant to use `assert_equal`.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+assert(3, my_list.length)
+assert(expected, actual)
+
+# good
+assert_equal(3, my_list.length)
+assert_equal(expected, actual)
+assert(foo, 'message')
+----
+
 == Minitest/AssertionInLifecycleHook
 
 |===

lib/rubocop/cop/minitest/assert_with_expected_argument.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Minitest
+      # This cop tries to detect when a user accidentally used
+      # `assert` when they meant to use `assert_equal`.
+      #
+      # @example
+      #   # bad
+      #   assert(3, my_list.length)
+      #   assert(expected, actual)
+      #
+      #   # good
+      #   assert_equal(3, my_list.length)
+      #   assert_equal(expected, actual)
+      #   assert(foo, 'message')
+      #
+      class AssertWithExpectedArgument < Cop
+        MSG = 'Did you mean to use `assert_equal(%<arguments>s)`?'
+        RESTRICT_ON_SEND = %i[assert].freeze
+
+        def_node_matcher :assert_with_two_arguments?, <<~PATTERN
+          (send nil? :assert $_ $_)
+        PATTERN
+
+        def on_send(node)
+          assert_with_two_arguments?(node) do |_expected, message|
+            return if message.str_type? || message.dstr_type?
+
+            arguments = node.arguments.map(&:source).join(', ')
+            add_offense(node, message: format(MSG, arguments: arguments))
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/minitest_cops.rb

@@ -8,6 +8,7 @@
 require_relative 'minitest/assert_empty_literal'
 require_relative 'minitest/assert_equal'
 require_relative 'minitest/assert_in_delta'
+require_relative 'minitest/assert_with_expected_argument'
 require_relative 'minitest/assertion_in_lifecycle_hook'
 require_relative 'minitest/assert_kind_of'
 require_relative 'minitest/assert_nil'

test/rubocop/cop/minitest/assert_with_expected_argument_test.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class AssertWithExpectedArgumentTest < Minitest::Test
+  def test_registers_offense_when_second_argument_is_not_a_string
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          my_list = [1, 2, 3]
+          assert(3, my_list.length)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^ Did you mean to use `assert_equal(3, my_list.length)`?
+        end
+      end
+    RUBY
+  end
+
+  def test_registers_offense_when_second_argument_is_a_variable
+    assert_offense(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          my_list_length = 3
+          assert(3, my_list_length)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^ Did you mean to use `assert_equal(3, my_list_length)`?
+        end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_assert_is_called_with_one_argument
+    assert_no_offenses(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert(true)
+        end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_second_argument_is_a_literal_string
+    assert_no_offenses(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert([], "empty array should be truthy")
+        end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_second_argument_is_an_interpolated_string
+    assert_no_offenses(<<~'RUBY')
+      class FooTest < Minitest::Test
+        def test_do_something
+          additional_message = 'hello world'
+          assert([], "empty array should be truthy #{additional_message}")
+        end
+      end
+    RUBY
+  end
+
+  def test_does_not_register_offense_when_using_assert_equal_with_two_arguments
+    assert_no_offenses(<<~'RUBY')
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert_equal(3, actual)
+        end
+      end
+    RUBY
+  end
+end