Merge pull request #1801 from rubocop/1760

Add configuration option `ResponseMethods` to `RSpec/Rails/HaveHttpStatus`

Author: ydah

CHANGELOG.md

@@ -13,6 +13,7 @@
 - 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])
+- Add configuration option `ResponseMethods` to `RSpec/Rails/HaveHttpStatus`. ([@ydah])
 
 ## 2.26.1 (2024-01-05)
 

config/default.yml

@@ -1132,8 +1132,12 @@ RSpec/Rails/AvoidSetupHook:
 RSpec/Rails/HaveHttpStatus:
   Description: Checks that tests use `have_http_status` instead of equality matchers.
   Enabled: pending
+  ResponseMethods:
+    - response
+    - last_response
   SafeAutoCorrect: false
   VersionAdded: '2.12'
+  VersionChanged: "<<next>>"
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/HaveHttpStatus
 
 RSpec/Rails/HttpStatus:

docs/modules/ROOT/pages/cops_rspecrails.adoc

@@ -48,22 +48,50 @@ end
 | Yes
 | Yes (Unsafe)
 | 2.12
-| -
+| <<next>>
 |===
 
 Checks that tests use `have_http_status` instead of equality matchers.
 
 === Examples
 
+==== ResponseMethods: ['response', 'last_response'] (default)
+
 [source,ruby]
 ----
 # bad
 expect(response.status).to be(200)
-expect(response.code).to eq("200")
+expect(last_response.code).to eq("200")
 
 # good
 expect(response).to have_http_status(200)
+expect(last_response).to have_http_status(200)
+----
+
+==== ResponseMethods: ['foo_response']
+
+[source,ruby]
 ----
+# bad
+expect(foo_response.status).to be(200)
+
+# good
+expect(foo_response).to have_http_status(200)
+
+# also good
+expect(response).to have_http_status(200)
+expect(last_response).to have_http_status(200)
+----
+
+=== Configurable attributes
+
+|===
+| Name | Default value | Configurable values
+
+| ResponseMethods
+| `response`, `last_response`
+| Array
+|===
 
 === References
 

lib/rubocop/cop/rspec_rails/have_http_status.rb

@@ -6,20 +6,32 @@ module RSpec
       module Rails
         # Checks that tests use `have_http_status` instead of equality matchers.
         #
-        # @example
+        # @example ResponseMethods: ['response', 'last_response'] (default)
         #   # bad
         #   expect(response.status).to be(200)
-        #   expect(response.code).to eq("200")
+        #   expect(last_response.code).to eq("200")
         #
         #   # good
         #   expect(response).to have_http_status(200)
+        #   expect(last_response).to have_http_status(200)
+        #
+        # @example ResponseMethods: ['foo_response']
+        #   # bad
+        #   expect(foo_response.status).to be(200)
+        #
+        #   # good
+        #   expect(foo_response).to have_http_status(200)
+        #
+        #   # also good
+        #   expect(response).to have_http_status(200)
+        #   expect(last_response).to have_http_status(200)
         #
         class HaveHttpStatus < ::RuboCop::Cop::Base
           extend AutoCorrector
 
           MSG =
-            'Prefer `expect(response).%<to>s have_http_status(%<status>s)` ' \
-            'over `%<bad_code>s`.'
+            'Prefer `expect(%<response>s).%<to>s ' \
+            'have_http_status(%<status>s)` over `%<bad_code>s`.'
 
           RUNNERS = %i[to to_not not_to].to_set
           RESTRICT_ON_SEND = RUNNERS
@@ -28,26 +40,38 @@ class HaveHttpStatus < ::RuboCop::Cop::Base
           def_node_matcher :match_status, <<~PATTERN
             (send
               (send nil? :expect
-                $(send (send nil? :response) {:status :code})
+                $(send $(send nil? #response_methods?) {:status :code})
               )
               $RUNNERS
               $(send nil? {:be :eq :eql :equal} ({int str} $_))
             )
           PATTERN
 
-          def on_send(node)
-            match_status(node) do |response_status, to, match, status|
+          def on_send(node) # rubocop:todo Metrics/MethodLength
+            match_status(node) do
+              |response_status, response_method, to, match, status|
               return unless status.to_s.match?(/\A\d+\z/)
 
-              message = format(MSG, to: to, status: status,
+              message = format(MSG, response: response_method.method_name,
+                                    to: to, status: status,
                                     bad_code: node.source)
               add_offense(node, message: message) do |corrector|
-                corrector.replace(response_status, 'response')
+                corrector.replace(response_status, response_method.method_name)
                 corrector.replace(match.loc.selector, 'have_http_status')
                 corrector.replace(match.first_argument, status.to_s)
               end
             end
           end
+
+          private
+
+          def response_methods?(name)
+            response_methods.include?(name.to_s)
+          end
+
+          def response_methods
+            cop_config.fetch('ResponseMethods', [])
+          end
         end
       end
     end

spec/rubocop/cop/rspec_rails/have_http_status_spec.rb

@@ -34,6 +34,17 @@
     RUBY
   end
 
+  it 'registers an offense for `expect(last_response.status).to eql("200")`' do
+    expect_offense(<<~RUBY)
+      it { expect(last_response.status).to eql("200") }
+           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `expect(last_response).to have_http_status(200)` over `expect(last_response.status).to eql("200")`.
+    RUBY
+
+    expect_correction(<<~RUBY)
+      it { expect(last_response).to have_http_status(200) }
+    RUBY
+  end
+
   it 'does not register an offense for `is_expected.to be(200)`' do
     expect_no_offenses(<<~RUBY)
       it { is_expected.to be(200) }
@@ -51,4 +62,26 @@
       it { expect(response.status).to eq("404 Not Found") }
     RUBY
   end
+
+  context 'when configured with ResponseMethods: [foo_response]' do
+    let(:cop_config) { { 'ResponseMethods' => %w[foo_response] } }
+
+    it 'registers an offense for `expect(foo_response.status).to be(200)`' do
+      expect_offense(<<~RUBY)
+        it { expect(foo_response.status).to be(200) }
+             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `expect(foo_response).to have_http_status(200)` over `expect(foo_response.status).to be(200)`.
+      RUBY
+
+      expect_correction(<<~RUBY)
+        it { expect(foo_response).to have_http_status(200) }
+      RUBY
+    end
+
+    it 'does not register an offense for ' \
+       '`expect(response.status).to be(200)`' do
+      expect_no_offenses(<<~RUBY)
+        it { expect(response.status).to be(200) }
+      RUBY
+    end
+  end
 end