Add support RSpec/Rails/HttpStatus when have_http_status with str…

…ing argument Fix: #1702
rubocop · Sep 4, 2023 · a97c5a1 · a97c5a1
1 parent de0b715
commit a97c5a1
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - Add new `RSpec/Eq` cop. ([@ydah])
 - Fix an incorrect autocorrect for `RSpec/ReceiveMessages` when return values declared between stubs. ([@marocchino])
 - Split `RSpec/FilePath` into `RSpec/SpecFilePathSuffix` and `RSpec/SpecFilePathFormat`. `RSpec/FilePath` cop is enabled by default, the two new cups are pending and need to be enabled explicitly. ([@ydah])
+- Add support `RSpec/Rails/HttpStatus` when `have_http_status` with string argument. ([@ydah])
 
 ## 2.23.2 (2023-08-09)
 

diff --git a/docs/modules/ROOT/pages/cops_rspec_rails.adoc b/docs/modules/ROOT/pages/cops_rspec_rails.adoc
@@ -91,10 +91,12 @@ when setting for `EnforcedStyle: symbolic` or
 # bad
 it { is_expected.to have_http_status 200 }
 it { is_expected.to have_http_status 404 }
+it { is_expected.to have_http_status "403" }
 
 # good
 it { is_expected.to have_http_status :ok }
 it { is_expected.to have_http_status :not_found }
+it { is_expected.to have_http_status :forbidden }
 it { is_expected.to have_http_status :success }
 it { is_expected.to have_http_status :error }
 ----
@@ -106,10 +108,12 @@ it { is_expected.to have_http_status :error }
 # bad
 it { is_expected.to have_http_status :ok }
 it { is_expected.to have_http_status :not_found }
+it { is_expected.to have_http_status "forbidden" }
 
 # good
 it { is_expected.to have_http_status 200 }
 it { is_expected.to have_http_status 404 }
+it { is_expected.to have_http_status 403 }
 it { is_expected.to have_http_status :success }
 it { is_expected.to have_http_status :error }
 ----
@@ -121,8 +125,10 @@ it { is_expected.to have_http_status :error }
 # bad
 it { is_expected.to have_http_status :ok }
 it { is_expected.to have_http_status :not_found }
+it { is_expected.to have_http_status "forbidden" }
 it { is_expected.to have_http_status 200 }
 it { is_expected.to have_http_status 404 }
+it { is_expected.to have_http_status "403" }
 
 # good
 it { is_expected.to be_ok }

diff --git a/lib/rubocop/cop/rspec/rails/http_status.rb b/lib/rubocop/cop/rspec/rails/http_status.rb
@@ -17,30 +17,36 @@ module Rails
         #   # bad
         #   it { is_expected.to have_http_status 200 }
         #   it { is_expected.to have_http_status 404 }
+        #   it { is_expected.to have_http_status "403" }
         #
         #   # good
         #   it { is_expected.to have_http_status :ok }
         #   it { is_expected.to have_http_status :not_found }
+        #   it { is_expected.to have_http_status :forbidden }
         #   it { is_expected.to have_http_status :success }
         #   it { is_expected.to have_http_status :error }
         #
         # @example `EnforcedStyle: numeric`
         #   # bad
         #   it { is_expected.to have_http_status :ok }
         #   it { is_expected.to have_http_status :not_found }
+        #   it { is_expected.to have_http_status "forbidden" }
         #
         #   # good
         #   it { is_expected.to have_http_status 200 }
         #   it { is_expected.to have_http_status 404 }
+        #   it { is_expected.to have_http_status 403 }
         #   it { is_expected.to have_http_status :success }
         #   it { is_expected.to have_http_status :error }
         #
         # @example `EnforcedStyle: be_status`
         #   # bad
         #   it { is_expected.to have_http_status :ok }
         #   it { is_expected.to have_http_status :not_found }
+        #   it { is_expected.to have_http_status "forbidden" }
         #   it { is_expected.to have_http_status 200 }
         #   it { is_expected.to have_http_status 404 }
+        #   it { is_expected.to have_http_status "403" }
         #
         #   # good
         #   it { is_expected.to be_ok }
@@ -55,7 +61,7 @@ class HttpStatus < Base
 
           # @!method http_status(node)
           def_node_matcher :http_status, <<-PATTERN
-            (send nil? :have_http_status ${int sym})
+            (send nil? :have_http_status ${int sym str})
           PATTERN
 
           def on_send(node)
@@ -124,7 +130,7 @@ def prefer
             end
 
             def current
-              number.inspect
+              node.value.inspect
             end
 
             private
@@ -134,7 +140,7 @@ def symbol
             end
 
             def number
-              node.source.to_i
+              node.source.delete('"').to_i
             end
           end
 
@@ -159,7 +165,7 @@ def symbol
             end
 
             def number
-              ::Rack::Utils::SYMBOL_TO_STATUS_CODE[symbol]
+              ::Rack::Utils::SYMBOL_TO_STATUS_CODE[symbol.to_sym]
             end
           end
 
@@ -177,8 +183,10 @@ def offense_range
             def prefer
               if node.sym_type?
                 "be_#{node.value}"
-              else
+              elsif node.int_type?
                 "be_#{symbol}"
+              elsif node.str_type?
+                "be_#{normalize_str}"
               end
             end
 
@@ -195,6 +203,15 @@ def symbol
             def number
               node.source.to_i
             end
+
+            def normalize_str
+              normalized = node.source.delete('"')
+              if normalized.match?(/\A\d+\z/)
+                ::Rack::Utils::SYMBOL_TO_STATUS_CODE.key(normalized.to_i)
+              else
+                normalized
+              end
+            end
           end
         end
       end

diff --git a/spec/rubocop/cop/rspec/rails/http_status_spec.rb b/spec/rubocop/cop/rspec/rails/http_status_spec.rb
@@ -15,6 +15,17 @@
       RUBY
     end
 
+    it 'registers an offense when using string value' do
+      expect_offense(<<-RUBY)
+        it { is_expected.to have_http_status "200" }
+                                             ^^^^^ Prefer `:ok` over `"200"` to describe HTTP status code.
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it { is_expected.to have_http_status :ok }
+      RUBY
+    end
+
     it 'does not register an offense when using symbolic value' do
       expect_no_offenses(<<-RUBY)
         it { is_expected.to have_http_status :ok }
@@ -55,6 +66,17 @@
       RUBY
     end
 
+    it 'registers an offense when using string value' do
+      expect_offense(<<-RUBY)
+        it { is_expected.to have_http_status "ok" }
+                                             ^^^^ Prefer `200` over `"ok"` to describe HTTP status code.
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it { is_expected.to have_http_status 200 }
+      RUBY
+    end
+
     it 'does not register an offense when using numeric value' do
       expect_no_offenses(<<-RUBY)
         it { is_expected.to have_http_status 200 }
@@ -109,6 +131,20 @@
       RUBY
     end
 
+    it 'registers an offense when using string value' do
+      expect_offense(<<-RUBY)
+        it { is_expected.to have_http_status "200" }
+                            ^^^^^^^^^^^^^^^^^^^^^^ Prefer `be_ok` over `have_http_status "200"` to describe HTTP status code.
+        it { is_expected.to have_http_status "ok" }
+                            ^^^^^^^^^^^^^^^^^^^^^ Prefer `be_ok` over `have_http_status "ok"` to describe HTTP status code.
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it { is_expected.to be_ok }
+        it { is_expected.to be_ok }
+      RUBY
+    end
+
     it 'does not register an offense when using numeric value' do
       expect_no_offenses(<<-RUBY)
         it { is_expected.to be_ok }