diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ae5337ea..e3d72b8f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +- Support correcting `assert_nil` and `refute_nil` to `RSpec/Rails/MinitestAssertions`. ([@G-Rath]) + ## 2.26.1 (2024-01-05) - Fix an error for `RSpec/SharedExamples` when using examples without argument. ([@ydah]) diff --git a/config/default.yml b/config/default.yml index ba24f8b5e..42fdde743 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1169,6 +1169,7 @@ RSpec/Rails/MinitestAssertions: Description: Check if using Minitest matchers. Enabled: pending VersionAdded: '2.17' + VersionChanged: "<>" Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/MinitestAssertions RSpec/Rails/NegationBeValid: diff --git a/docs/modules/ROOT/pages/cops_rspec_rails.adoc b/docs/modules/ROOT/pages/cops_rspec_rails.adoc index 21580e208..82a28fbcc 100644 --- a/docs/modules/ROOT/pages/cops_rspec_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_rails.adoc @@ -243,7 +243,7 @@ end | Yes | Yes | 2.17 -| - +| <> |=== Check if using Minitest matchers. @@ -257,10 +257,16 @@ assert_equal(a, b) assert_equal a, b, "must be equal" refute_equal(a, b) +assert_nil a +refute_nil a + # 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) ---- === References diff --git a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb index 7bc929d79..f12448691 100644 --- a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb +++ b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb @@ -12,35 +12,61 @@ module Rails # assert_equal a, b, "must be equal" # refute_equal(a, b) # + # assert_nil a + # refute_nil a + # # # 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) + # class MinitestAssertions < Base extend AutoCorrector MSG = 'Use `%s`.' - RESTRICT_ON_SEND = %i[assert_equal refute_equal].freeze + RESTRICT_ON_SEND = %i[ + assert_equal + refute_equal + assert_nil + refute_nil + ].freeze - # @!method minitest_assertion(node) - def_node_matcher :minitest_assertion, <<~PATTERN + # @!method minitest_equal_assertion(node) + def_node_matcher :minitest_equal_assertion, <<~PATTERN (send nil? {:assert_equal :refute_equal} $_ $_ $_?) PATTERN + # @!method minitest_nil_assertion(node) + def_node_matcher :minitest_nil_assertion, <<~PATTERN + (send nil? {:assert_nil :refute_nil} $_ $_?) + PATTERN + def on_send(node) - minitest_assertion(node) do |expected, actual, failure_message| - prefer = replacement(node, expected, actual, - failure_message.first) - add_offense(node, message: message(prefer)) do |corrector| - corrector.replace(node, prefer) - end + minitest_equal_assertion(node) do |expected, actual, fail_message| + prefer = replace_equal_assertion(node, expected, actual, + fail_message.first) + add_an_offense(node, prefer) + end + + minitest_nil_assertion(node) do |actual, fail_message| + prefer = replace_nil_assertion(node, actual, + fail_message.first) + add_an_offense(node, prefer) end end private - def replacement(node, expected, actual, failure_message) + def add_an_offense(node, prefer) + add_offense(node, message: message(prefer)) do |corrector| + corrector.replace(node, prefer) + 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})" @@ -50,6 +76,16 @@ def replacement(node, expected, actual, failure_message) 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})" + end + end + def message(prefer) format(MSG, prefer: prefer) end diff --git a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb index bdec29abf..7537060be 100644 --- a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb +++ b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb @@ -1,73 +1,146 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::RSpec::Rails::MinitestAssertions do - it 'registers an offense when using `assert_equal`' do - expect_offense(<<~RUBY) - assert_equal(a, b) - ^^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. - RUBY - - expect_correction(<<~RUBY) - expect(b).to eq(a) - RUBY - end + context 'with equal assertions' do + it 'registers an offense when using `assert_equal`' do + expect_offense(<<~RUBY) + assert_equal(a, b) + ^^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. + RUBY - it 'registers an offense when using `assert_equal` with no parentheses' do - expect_offense(<<~RUBY) - assert_equal a, b - ^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. - RUBY + expect_correction(<<~RUBY) + expect(b).to eq(a) + RUBY + end - expect_correction(<<~RUBY) - expect(b).to eq(a) - RUBY - end + it 'registers an offense when using `assert_equal` with no parentheses' do + expect_offense(<<~RUBY) + assert_equal a, b + ^^^^^^^^^^^^^^^^^ Use `expect(b).to eq(a)`. + RUBY - it 'registers an offense when using `assert_equal` with failure message' do - expect_offense(<<~RUBY) - assert_equal a, b, "must be equal" - ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. - RUBY + expect_correction(<<~RUBY) + expect(b).to eq(a) + RUBY + end - expect_correction(<<~RUBY) - expect(b).to(eq(a), "must be equal") - RUBY - end + it 'registers an offense when using `assert_equal` with failure message' do + expect_offense(<<~RUBY) + assert_equal a, b, "must be equal" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. + RUBY - it 'registers an offense when using `assert_equal` with ' \ - 'multi-line arguments' do - expect_offense(<<~RUBY) - assert_equal(a, - ^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. - b, - "must be equal") - RUBY - - expect_correction(<<~RUBY) - expect(b).to(eq(a), "must be equal") - RUBY - end + expect_correction(<<~RUBY) + expect(b).to(eq(a), "must be equal") + RUBY + end - it 'registers an offense when using `refute_equal`' do - expect_offense(<<~RUBY) - refute_equal a, b - ^^^^^^^^^^^^^^^^^ Use `expect(b).not_to eq(a)`. - RUBY + it 'registers an offense when using `assert_equal` with ' \ + 'multi-line arguments' do + expect_offense(<<~RUBY) + assert_equal(a, + ^^^^^^^^^^^^^^^ Use `expect(b).to(eq(a), "must be equal")`. + b, + "must be equal") + RUBY - expect_correction(<<~RUBY) - expect(b).not_to eq(a) - RUBY - end + expect_correction(<<~RUBY) + expect(b).to(eq(a), "must be equal") + RUBY + end - it 'does not register an offense when using `expect(b).to eq(a)`' do - expect_no_offenses(<<~RUBY) - expect(b).to eq(a) - RUBY + it 'registers an offense when using `refute_equal`' do + expect_offense(<<~RUBY) + refute_equal a, b + ^^^^^^^^^^^^^^^^^ Use `expect(b).not_to eq(a)`. + RUBY + + expect_correction(<<~RUBY) + expect(b).not_to eq(a) + RUBY + end + + it 'does not register an offense when using `expect(b).to eq(a)`' do + expect_no_offenses(<<~RUBY) + expect(b).to eq(a) + RUBY + end + + it 'does not register an offense when using `expect(b).not_to eq(a)`' do + expect_no_offenses(<<~RUBY) + expect(b).not_to eq(a) + RUBY + end end - it 'does not register an offense when using `expect(b).not_to eq(a)`' do - expect_no_offenses(<<~RUBY) - expect(b).not_to eq(a) - RUBY + context 'with nil assertions' do + it 'registers an offense when using `assert_nil`' do + expect_offense(<<~RUBY) + assert_nil(a) + ^^^^^^^^^^^^^ Use `expect(a).to eq(nil)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to eq(nil) + RUBY + end + + it 'registers an offense when using `assert_nil` with no parentheses' do + expect_offense(<<~RUBY) + assert_nil a + ^^^^^^^^^^^^ Use `expect(a).to eq(nil)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to eq(nil) + RUBY + end + + it 'registers an offense when using `assert_nil` with failure message' do + expect_offense(<<~RUBY) + assert_nil a, "must be nil" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to(eq(nil), "must be nil")`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to(eq(nil), "must be nil") + RUBY + end + + it 'registers an offense when using `assert_nil` with ' \ + 'multi-line arguments' do + expect_offense(<<~RUBY) + assert_nil(a, + ^^^^^^^^^^^^^ Use `expect(a).to(eq(nil), "must be nil")`. + "must be nil") + RUBY + + expect_correction(<<~RUBY) + expect(a).to(eq(nil), "must be nil") + RUBY + end + + it 'registers an offense when using `refute_nil`' do + expect_offense(<<~RUBY) + refute_nil a + ^^^^^^^^^^^^ Use `expect(a).not_to eq(nil)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).not_to eq(nil) + RUBY + end + + it 'does not register an offense when using `expect(a).to eq(nil)`' do + expect_no_offenses(<<~RUBY) + expect(a).to eq(nil) + RUBY + end + + it 'does not register an offense when using `expect(a).not_to eq(nil)`' do + expect_no_offenses(<<~RUBY) + expect(a).not_to eq(nil) + RUBY + end end end