Skip to content

Commit

Permalink
Change AssertEmptyLiteral cop to check for assert_equal
Browse files Browse the repository at this point in the history
While this cop is correct that we should be using `assert_empty(list)`
over `assert([], list)`, it's right for the wrong reasons. That example
is bad because it's using `assert` and not `assert_equal`. An empty
list/hash is truthy so the assertion will always succeed. That is,
`assert([], [1, 2, 3])` will succeed even though at a glance it looks
like it shouldn't. I believe that what this cop is intending to check
for is `assert_equal([], list)` instead so I've updated the references
to `assert` to `assert_equal`.

Issue rubocop#117 proposes that we try to detect when `assert` is used where
the user likely wanted `assert_equal` so while this cop will no longer
register a violation for `assert([], list)`, if that proposal is
accepted then a future cop *will* register a violation.
  • Loading branch information
cstyles committed Feb 21, 2021
1 parent 09c018d commit c7c05de
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
16 changes: 8 additions & 8 deletions lib/rubocop/cop/minitest/assert_empty_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ module RuboCop
module Cop
module Minitest
# This cop enforces the test to use `assert_empty`
# instead of using `assert([], object)`.
# instead of using `assert_equal([], object)`.
#
# @example
# # bad
# assert([], object)
# assert({}, object)
# assert_equal([], object)
# assert_equal({}, object)
#
# # good
# assert_empty(object)
Expand All @@ -18,14 +18,14 @@ class AssertEmptyLiteral < Cop
include ArgumentRangeHelper

MSG = 'Prefer using `assert_empty(%<arguments>s)` over ' \
'`assert(%<literal>s, %<arguments>s)`.'
'`assert_equal(%<literal>s, %<arguments>s)`.'

def_node_matcher :assert_with_empty_literal, <<~PATTERN
(send nil? :assert ${hash array} $...)
def_node_matcher :assert_equal_with_empty_literal, <<~PATTERN
(send nil? :assert_equal ${hash array} $...)
PATTERN

def on_send(node)
assert_with_empty_literal(node) do |literal, matchers|
assert_equal_with_empty_literal(node) do |literal, matchers|
return unless literal.values.empty?

args = matchers.map(&:source).join(', ')
Expand All @@ -36,7 +36,7 @@ def on_send(node)
end

def autocorrect(node)
assert_with_empty_literal(node) do |_literal, matchers|
assert_equal_with_empty_literal(node) do |_literal, matchers|
object = matchers.first

lambda do |corrector|
Expand Down
14 changes: 7 additions & 7 deletions test/rubocop/cop/minitest/assert_empty_literal_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ def test_registers_offense_when_asserting_empty_array
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert([], somestuff)
^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_empty(somestuff)` over `assert([], somestuff)`.
assert_equal([], somestuff)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_empty(somestuff)` over `assert_equal([], somestuff)`.
end
end
RUBY
Expand All @@ -26,8 +26,8 @@ def test_registers_offense_when_asserting_empty_hash
assert_offense(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert({}, somestuff)
^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_empty(somestuff)` over `assert({}, somestuff)`.
assert_equal({}, somestuff)
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using `assert_empty(somestuff)` over `assert_equal({}, somestuff)`.
end
end
RUBY
Expand All @@ -41,7 +41,7 @@ def test_do_something
RUBY
end

def test_does_not_register_offense_when_using_assert_equal
def test_does_not_register_offense_when_asserting_non_empty_array
assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
Expand All @@ -51,11 +51,11 @@ def test_do_something
RUBY
end

def test_does_not_register_offense_when_using_assert_with_single_parameter
def test_does_not_register_offense_when_asserting_two_parameters
assert_no_offenses(<<~RUBY)
class FooTest < Minitest::Test
def test_do_something
assert(somestuff)
assert_equal(somestuff, someotherstuff)
end
end
RUBY
Expand Down

0 comments on commit c7c05de

Please sign in to comment.