[Fix rubocop#117] Add new Minitest/AssertTwoArgs 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.

config/default.yml

@@ -94,6 +94,11 @@ Minitest/AssertTruthy:
   Enabled: true
   VersionAdded: '0.2'
 
+Minitest/AssertWithTwoArguments:
+  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#minitestassertwithtwoarguments[Minitest/AssertWithTwoArguments]
 * 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/AssertWithTwoArguments
+
+|===
+| 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_two_arguments.rb

@@ -0,0 +1,37 @@
+# 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 AssertWithTwoArguments < Cop
+        MSG = 'Did you mean to use `assert_equal(%<arguments>s)`?'
+
+        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?
+
+            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_two_arguments'
 require_relative 'minitest/assertion_in_lifecycle_hook'
 require_relative 'minitest/assert_kind_of'
 require_relative 'minitest/assert_nil'

test/rubocop/cop/minitest/assert_with_two_arguments_test.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class AssertWithTwoArgumentsTest < 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_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_string
+    assert_no_offenses(<<~RUBY)
+      class FooTest < Minitest::Test
+        def test_do_something
+          assert([], "empty array should be truthy")
+        end
+      end
+    RUBY
+  end
+end