diff --git a/CHANGELOG.md b/CHANGELOG.md index 6075b58b0..23d633774 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,8 +3,8 @@ ## Master (Unreleased) - Add support for `assert_empty`, `assert_not_empty` and `refute_empty` to `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]) +- Support correcting `*_includes` assertions in `RSpec/Rails/MinitestAssertions`. ([@G-Rath]) +- Support correcting `assert_not_equal` and `assert_not_nil` in `RSpec/Rails/MinitestAssertions`. ([@G-Rath]) - Fix a false positive for `RSpec/ExpectActual` when used with rspec-rails routing matchers. ([@naveg]) - Add new `RSpec/RepeatedSubjectCall` cop. ([@drcapulet]) diff --git a/docs/modules/ROOT/pages/cops_rspec_rails.adoc b/docs/modules/ROOT/pages/cops_rspec_rails.adoc index 477cd1fe8..cd6fd7819 100644 --- a/docs/modules/ROOT/pages/cops_rspec_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_rails.adoc @@ -255,6 +255,7 @@ Check if using Minitest matchers. # bad assert_equal(a, b) assert_equal a, b, "must be equal" +assert_not_includes a, b refute_equal(a, b) assert_nil a refute_empty(b) @@ -262,6 +263,7 @@ refute_empty(b) # good expect(b).to eq(a) expect(b).to(eq(a), "must be equal") +expect(a).not_to include(b) expect(b).not_to eq(a) expect(a).to eq(nil) expect(a).not_to be_empty diff --git a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb index dab19a926..897300543 100644 --- a/lib/rubocop/cop/rspec/rails/minitest_assertions.rb +++ b/lib/rubocop/cop/rspec/rails/minitest_assertions.rb @@ -10,6 +10,7 @@ module Rails # # bad # assert_equal(a, b) # assert_equal a, b, "must be equal" + # assert_not_includes a, b # refute_equal(a, b) # assert_nil a # refute_empty(b) @@ -17,6 +18,7 @@ module Rails # # good # expect(b).to eq(a) # expect(b).to(eq(a), "must be equal") + # expect(a).not_to include(b) # expect(b).not_to eq(a) # expect(a).to eq(nil) # expect(a).not_to be_empty @@ -28,11 +30,14 @@ class MinitestAssertions < Base RESTRICT_ON_SEND = %i[ assert_equal assert_not_equal + assert_includes + assert_not_includes assert_nil assert_not_nil assert_empty assert_not_empty refute_equal + refute_includes refute_nil refute_empty ].freeze @@ -42,6 +47,11 @@ class MinitestAssertions < Base (send nil? {:assert_equal :assert_not_equal :refute_equal} $_ $_ $_?) PATTERN + # @!method minitest_includes(node) + def_node_matcher :minitest_includes, <<~PATTERN + (send nil? {:assert_includes :assert_not_includes :refute_includes} $_ $_ $_?) + PATTERN + # @!method minitest_nil(node) def_node_matcher :minitest_nil, <<~PATTERN (send nil? {:assert_nil :assert_not_nil :refute_nil} $_ $_?) @@ -52,12 +62,17 @@ class MinitestAssertions < Base (send nil? {:assert_empty :assert_not_empty :refute_empty} $_ $_?) PATTERN - def on_send(node) # rubocop:disable Metrics/MethodLength + def on_send(node) # rubocop:disable Metrics/MethodLength, Metrics/AbcSize minitest_equal(node) do |expected, actual, failure_message| on_assertion(node, EqualAssertion.new(expected, actual, failure_message.first)) end + minitest_includes(node) do |collection, expected, failure_message| + on_assertion(node, IncludesAssertion.new(collection, expected, + failure_message.first)) + end + minitest_nil(node) do |actual, failure_message| on_assertion(node, NilAssertion.new(actual, failure_message.first)) @@ -99,6 +114,28 @@ def replaced(node) end end + # :nodoc: + class IncludesAssertion + def initialize(collection, expected, fail_message) + @collection = collection + @expected = expected + @fail_message = fail_message + end + + def replaced(node) + a_source = @collection.source + b_source = @expected.source + + runner = node.method?(:assert_includes) ? 'to' : 'not_to' + if @fail_message.nil? + "expect(#{a_source}).#{runner} include(#{b_source})" + else + "expect(#{a_source}).#{runner}(include(#{b_source})," \ + " #{@fail_message.source})" + end + end + end + # :nodoc: class NilAssertion def initialize(actual, fail_message) diff --git a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb index a4217fd24..7b5df9b1a 100644 --- a/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb +++ b/spec/rubocop/cop/rspec/rails/minitest_assertions_spec.rb @@ -84,6 +84,92 @@ end end + context 'with includes assertions' do + it 'registers an offense when using `assert_includes`' do + expect_offense(<<~RUBY) + assert_includes(a, b) + ^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to include(b)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to include(b) + RUBY + end + + it 'registers an offense when using `assert_includes` with ' \ + 'no parentheses' do + expect_offense(<<~RUBY) + assert_includes a, b + ^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to include(b)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to include(b) + RUBY + end + + it 'registers an offense when using `assert_includes` with ' \ + 'failure message' do + expect_offense(<<~RUBY) + assert_includes a, b, "must be include" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).to(include(b), "must be include")`. + RUBY + + expect_correction(<<~RUBY) + expect(a).to(include(b), "must be include") + RUBY + end + + it 'registers an offense when using `assert_includes` with ' \ + 'multi-line arguments' do + expect_offense(<<~RUBY) + assert_includes(a, + ^^^^^^^^^^^^^^^^^^ Use `expect(a).to(include(b), "must be include")`. + b, + "must be include") + RUBY + + expect_correction(<<~RUBY) + expect(a).to(include(b), "must be include") + RUBY + end + + it 'registers an offense when using `assert_not_includes`' do + expect_offense(<<~RUBY) + assert_not_includes a, b + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(a).not_to include(b)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).not_to include(b) + RUBY + end + + it 'registers an offense when using `refute_includes`' do + expect_offense(<<~RUBY) + refute_includes a, b + ^^^^^^^^^^^^^^^^^^^^ Use `expect(a).not_to include(b)`. + RUBY + + expect_correction(<<~RUBY) + expect(a).not_to include(b) + RUBY + end + + it 'does not register an offense when using `expect(a).to include(b)`' do + expect_no_offenses(<<~RUBY) + expect(a).to include(b) + RUBY + end + + it 'does not register an offense when ' \ + 'using `expect(a).not_to include(b)`' do + expect_no_offenses(<<~RUBY) + expect(a).not_to include(b) + RUBY + end + end + context 'with nil assertions' do it 'registers an offense when using `assert_nil`' do expect_offense(<<~RUBY)