diff --git a/CHANGELOG.md b/CHANGELOG.md index f1d0d69..e5f5a0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Master (Unreleased) - Add `RSpecRails/HttpStatusNameConsistency` cop. ([@taketo1113]) +- Support correcting `assert_response` assertion in in `RSpec/Rails/MinitestAssertions`. ([@nzlaura]) ## 2.31.0 (2025-03-10) @@ -84,6 +85,7 @@ [@jojos003]: https://github.com/jojos003 [@mothonmars]: https://github.com/MothOnMars [@mvz]: https://github.com/mvz +[@nzlaura]: https://github.com/nzlaura [@paydaylight]: https://github.com/paydaylight [@pirj]: https://github.com/pirj [@r7kamura]: https://github.com/r7kamura diff --git a/docs/modules/ROOT/pages/cops_rspecrails.adoc b/docs/modules/ROOT/pages/cops_rspecrails.adoc index 6ec8b30..7f41bd5 100644 --- a/docs/modules/ROOT/pages/cops_rspecrails.adoc +++ b/docs/modules/ROOT/pages/cops_rspecrails.adoc @@ -364,6 +364,7 @@ assert_nil a refute_empty(b) assert_true(a) assert_false(a) +assert_response :ok # good expect(b).to eq(a) @@ -374,6 +375,7 @@ expect(a).to eq(nil) expect(a).not_to be_empty expect(a).to be(true) expect(a).to be(false) +expect(response).to have_http_status(:ok) ---- [#references-rspecrailsminitestassertions] diff --git a/lib/rubocop/cop/rspec_rails/minitest_assertions.rb b/lib/rubocop/cop/rspec_rails/minitest_assertions.rb index 05a5522..88975dd 100644 --- a/lib/rubocop/cop/rspec_rails/minitest_assertions.rb +++ b/lib/rubocop/cop/rspec_rails/minitest_assertions.rb @@ -18,6 +18,7 @@ module RSpecRails # refute_empty(b) # assert_true(a) # assert_false(a) + # assert_response :ok # # # good # expect(b).to eq(a) @@ -28,6 +29,7 @@ module RSpecRails # expect(a).not_to be_empty # expect(a).to be(true) # expect(a).to be(false) + # expect(response).to have_http_status(:ok) # class MinitestAssertions < ::RuboCop::Cop::Base extend AutoCorrector @@ -315,6 +317,28 @@ def assertion end end + # :nodoc: + class ResponseAssertion < BasicAssertion + MATCHERS = %i[ + assert_response + ].freeze + + ASSERT_ACTUAL_NODE = Struct.new(:source).new('response') + + # @!method self.minitest_assertion(node) + def_node_matcher 'self.minitest_assertion', <<~PATTERN # rubocop:disable InternalAffairs/NodeMatcherDirective + (send nil? {:assert_response} $_ $_?) + PATTERN + + def self.match(expected, failure_message) + new(expected, ASSERT_ACTUAL_NODE, failure_message.first) + end + + def assertion + "have_http_status(#{expected})" + end + end + MSG = 'Use `%s`.' # TODO: replace with `BasicAssertion.subclasses` in Ruby 3.1+ diff --git a/spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb b/spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb index 179330c..6c6cebf 100644 --- a/spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb +++ b/spec/rubocop/cop/rspec_rails/minitest_assertions_spec.rb @@ -947,4 +947,90 @@ RUBY end end + + context 'with response assertions' do + it 'registers an offense when using `assert_response`' do + expect_offense(<<~RUBY) + assert_response :redirect + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(response).to have_http_status(:redirect)`. + RUBY + + expect_correction(<<~RUBY) + expect(response).to have_http_status(:redirect) + RUBY + end + + it 'registers an offense when using `assert_response` with a number' do + expect_offense(<<~RUBY) + assert_response 302 + ^^^^^^^^^^^^^^^^^^^ Use `expect(response).to have_http_status(302)`. + RUBY + + expect_correction(<<~RUBY) + expect(response).to have_http_status(302) + RUBY + end + + it 'registers an offense when using `assert_response` with parentheses' do + expect_offense(<<~RUBY) + assert_response(:success) + ^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(response).to have_http_status(:success)`. + RUBY + + expect_correction(<<~RUBY) + expect(response).to have_http_status(:success) + RUBY + end + + it 'registers an offense when using `assert_response` with ' \ + 'failure message' do + expect_offense(<<~RUBY) + assert_response :success, "expected success status" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(response).to(have_http_status(:success), "expected success status")`. + RUBY + + expect_correction(<<~RUBY) + expect(response).to(have_http_status(:success), "expected success status") + RUBY + end + + it 'registers an offense when using `assert_response` with ' \ + 'multi-line arguments' do + expect_offense(<<~RUBY) + assert_response(:redirect, + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `expect(response).to(have_http_status(:redirect), "expected redirect status")`. + "expected redirect status") + RUBY + + expect_correction(<<~RUBY) + expect(response).to(have_http_status(:redirect), "expected redirect status") + RUBY + end + + it 'registers an offense when using `assert_response` with ' \ + 'numeric status' do + expect_offense(<<~RUBY) + assert_response 200 + ^^^^^^^^^^^^^^^^^^^ Use `expect(response).to have_http_status(200)`. + RUBY + + expect_correction(<<~RUBY) + expect(response).to have_http_status(200) + RUBY + end + + it 'does not register an offense when using ' \ + '`expect(response).to have_http_status`' do + expect_no_offenses(<<~RUBY) + expect(response).to have_http_status(:success) + RUBY + end + + it 'does not register an offense when using ' \ + '`expect(response).to have_http_status` with numeric status' do + expect_no_offenses(<<~RUBY) + expect(response).to have_http_status(200) + RUBY + end + end end