diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c4336d17..50386ca3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Master (Unreleased) +- Add support `assert_empty`, `assert_not_empty` and `refute_empty` for `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]) diff --git a/docs/modules/ROOT/pages/cops_rspec_rails.adoc b/docs/modules/ROOT/pages/cops_rspec_rails.adoc index d26d4687c..0dcedfbb2 100644 --- a/docs/modules/ROOT/pages/cops_rspec_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_rails.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 eq([]) ---- === References diff --git a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb index 15e53c3d3..f0b175807 100644 --- a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb +++ b/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 eq([]) # class MinitestAssertions < Base extend AutoCorrector @@ -30,66 +28,117 @@ 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(node, 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(node, actual, + failure_message.first)) end - end - private + minitest_empty(node) do |actual, failure_message| + on_assertion(node, EmptyAssertion.new(node, 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 + 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(node, expected, actual, fail_message) + @node = node + @expected = expected + @actual = actual + @fail_message = fail_message + super() + end + + def replaced + 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(node, actual, fail_message) + @node = node + @actual = actual + @fail_message = fail_message + super() + end + + def replaced + 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(node, actual, fail_message) + @node = node + @actual = actual + @fail_message = fail_message + super() + end + + def replaced + runner = @node.method?(:assert_empty) ? 'to' : 'not_to' + if @fail_message.nil? + "expect(#{@actual.source}).#{runner} eq([])" + else + "expect(#{@actual.source}).#{runner}(eq([]), " \ + "#{@fail_message.source})" + end + end end end end diff --git a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb index d1e817a61..73173f9b1 100644 --- a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb +++ b/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 eq([])`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to eq([]) + RUBY + end + + it 'registers an offense when using `assert_empty` with no parentheses' do + expect_offense(<<~RUBY) + assert_empty a + ^^^^^^^^^^^^^^ Use `expect(a).to eq([])`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to eq([]) + 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(eq([]), "must be empty")`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to(eq([]), "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(eq([]), "must be empty")`. + "must be empty") + RUBY + + expect_correction(<<~RUBY) + expect(a).to(eq([]), "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 eq([])`. + RUBY + + expect_correction(<<~RUBY) + expect(a).not_to eq([]) + RUBY + end + + it 'registers an offense when using `refute_empty`' do + expect_offense(<<~RUBY) + refute_empty a + ^^^^^^^^^^^^^^ Use `expect(a).not_to eq([])`. + RUBY + + expect_correction(<<~RUBY) + expect(a).not_to eq([]) + RUBY + end + + it 'does not register an offense when using `expect(a).to eq([])`' do + expect_no_offenses(<<~RUBY) + expect(a).to eq([]) + RUBY + end + + it 'does not register an offense when using `expect(a).not_to eq([])`' do + expect_no_offenses(<<~RUBY) + expect(a).not_to eq([]) + RUBY + end end end