Merge pull request #1527 from ydah/feature/1148

bquorning · web-flow · commit 0610b7346c86 · 2023-04-18T08:22:21.000+02:00
Add support `be_status` style for `RSpec/Rails/HttpStatus`
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Master (Unreleased)
 
+- Add support `be_status` style for `RSpec/Rails/HttpStatus`. ([@ydah])
 - Add new `RSpec/IndexedLet` cop. ([@dmitrytsepelev])
 - Fix order of expected and actual in correction for `RSpec/Rails/MinitestAssertions` ([@mvz])
 - Fix a false positive for `RSpec/FactoryBot/ConsistentParenthesesStyle` inside `&&`, `||` and `:?` when `omit_parentheses` is on ([@dmitrytsepelev])
diff --git a/config/default.yml b/config/default.yml
@@ -1061,8 +1061,9 @@ RSpec/Rails/HttpStatus:
   SupportedStyles:
     - numeric
     - symbolic
+    - be_status
   VersionAdded: '1.23'
-  VersionChanged: '2.0'
+  VersionChanged: "<<next>>"
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/HttpStatus
 
 RSpec/Rails/InferredSpecType:
diff --git a/docs/modules/ROOT/pages/cops_rspec_rails.adoc b/docs/modules/ROOT/pages/cops_rspec_rails.adoc
@@ -72,11 +72,16 @@ expect(response).to have_http_status(200)
 | Yes
 | Yes
 | 1.23
-| 2.0
+| <<next>>
 |===
 
 Enforces use of symbolic or numeric value to describe HTTP status.
 
+This cop inspects only `have_http_status` calls.
+So, this cop does not check if a method starting with `be_*` is used
+when setting for `EnforcedStyle: symbolic` or
+`EnforcedStyle: numeric`.
+
 === Examples
 
 ==== `EnforcedStyle: symbolic` (default)
@@ -109,14 +114,31 @@ it { is_expected.to have_http_status :success }
 it { is_expected.to have_http_status :error }
 ----
 
+==== `EnforcedStyle: be_status`
+
+[source,ruby]
+----
+# 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 200 }
+it { is_expected.to have_http_status 404 }
+
+# good
+it { is_expected.to be_ok }
+it { is_expected.to be_not_found }
+it { is_expected.to have_http_status :success }
+it { is_expected.to have_http_status :error }
+----
+
 === Configurable attributes
 
 |===
 | Name | Default value | Configurable values
 
 | EnforcedStyle
 | `symbolic`
-| `numeric`, `symbolic`
+| `numeric`, `symbolic`, `be_status`
 |===
 
 === References
diff --git a/lib/rubocop/cop/rspec/rails/http_status.rb b/lib/rubocop/cop/rspec/rails/http_status.rb
@@ -8,6 +8,11 @@ module RSpec
       module Rails
         # Enforces use of symbolic or numeric value to describe HTTP status.
         #
+        # This cop inspects only `have_http_status` calls.
+        # So, this cop does not check if a method starting with `be_*` is used
+        # when setting for `EnforcedStyle: symbolic` or
+        # `EnforcedStyle: numeric`.
+        #
         # @example `EnforcedStyle: symbolic` (default)
         #   # bad
         #   it { is_expected.to have_http_status 200 }
@@ -30,6 +35,19 @@ module Rails
         #   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 200 }
+        #   it { is_expected.to have_http_status 404 }
+        #
+        #   # good
+        #   it { is_expected.to be_ok }
+        #   it { is_expected.to be_not_found }
+        #   it { is_expected.to have_http_status :success }
+        #   it { is_expected.to have_http_status :error }
+        #
         class HttpStatus < Base
           extend AutoCorrector
           include ConfigurableEnforcedStyle
@@ -41,12 +59,13 @@ class HttpStatus < Base
           PATTERN
 
           def on_send(node)
-            http_status(node) do |ast_node|
-              checker = checker_class.new(ast_node)
+            http_status(node) do |arg|
+              checker = checker_class.new(arg)
               return unless checker.offensive?
 
-              add_offense(checker.node, message: checker.message) do |corrector|
-                corrector.replace(checker.node, checker.preferred_style)
+              add_offense(checker.offense_range,
+                          message: checker.message) do |corrector|
+                corrector.replace(checker.offense_range, checker.prefer)
               end
             end
           end
@@ -59,30 +78,53 @@ def checker_class
               SymbolicStyleChecker
             when :numeric
               NumericStyleChecker
+            when :be_status
+              BeStatusStyleChecker
             end
           end
 
           # :nodoc:
-          class SymbolicStyleChecker
+          class StyleCheckerBase
             MSG = 'Prefer `%<prefer>s` over `%<current>s` ' \
                   'to describe HTTP status code.'
+            ALLOWED_STATUSES = %i[error success missing redirect].freeze
 
             attr_reader :node
 
             def initialize(node)
               @node = node
             end
 
+            def message
+              format(MSG, prefer: prefer, current: current)
+            end
+
+            def offense_range
+              node
+            end
+
+            def allowed_symbol?
+              node.sym_type? && ALLOWED_STATUSES.include?(node.value)
+            end
+
+            def custom_http_status_code?
+              node.int_type? &&
+                !::Rack::Utils::SYMBOL_TO_STATUS_CODE.value?(node.source.to_i)
+            end
+          end
+
+          # :nodoc:
+          class SymbolicStyleChecker < StyleCheckerBase
             def offensive?
               !node.sym_type? && !custom_http_status_code?
             end
 
-            def message
-              format(MSG, prefer: preferred_style, current: number.to_s)
+            def prefer
+              symbol.inspect
             end
 
-            def preferred_style
-              symbol.inspect
+            def current
+              number.inspect
             end
 
             private
@@ -94,50 +136,64 @@ def symbol
             def number
               node.source.to_i
             end
-
-            def custom_http_status_code?
-              node.int_type? &&
-                !::Rack::Utils::SYMBOL_TO_STATUS_CODE.value?(node.source.to_i)
-            end
           end
 
           # :nodoc:
-          class NumericStyleChecker
-            MSG = 'Prefer `%<prefer>s` over `%<current>s` ' \
-                  'to describe HTTP status code.'
-
-            ALLOWED_STATUSES = %i[error success missing redirect].freeze
-
-            attr_reader :node
-
-            def initialize(node)
-              @node = node
-            end
-
+          class NumericStyleChecker < StyleCheckerBase
             def offensive?
               !node.int_type? && !allowed_symbol?
             end
 
-            def message
-              format(MSG, prefer: preferred_style, current: symbol.inspect)
+            def prefer
+              number.to_s
             end
 
-            def preferred_style
-              number.to_s
+            def current
+              symbol.inspect
             end
 
             private
 
+            def symbol
+              node.value
+            end
+
             def number
               ::Rack::Utils::SYMBOL_TO_STATUS_CODE[symbol]
             end
+          end
+
+          # :nodoc:
+          class BeStatusStyleChecker < StyleCheckerBase
+            def offensive?
+              (!node.sym_type? && !custom_http_status_code?) ||
+                (!node.int_type? && !allowed_symbol?)
+            end
+
+            def offense_range
+              node.parent
+            end
+
+            def prefer
+              if node.sym_type?
+                "be_#{node.value}"
+              else
+                "be_#{symbol}"
+              end
+            end
+
+            def current
+              offense_range.source
+            end
+
+            private
 
             def symbol
-              node.value
+              ::Rack::Utils::SYMBOL_TO_STATUS_CODE.key(number)
             end
 
-            def allowed_symbol?
-              node.sym_type? && ALLOWED_STATUSES.include?(node.value)
+            def number
+              node.source.to_i
             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
@@ -83,4 +83,69 @@
       end
     end
   end
+
+  context 'when EnforcedStyle is `be_status`' do
+    let(:cop_config) { { 'EnforcedStyle' => 'be_status' } }
+
+    it 'registers an offense when using numeric 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.
+      RUBY
+
+      expect_correction(<<-RUBY)
+        it { is_expected.to be_ok }
+      RUBY
+    end
+
+    it 'registers an offense when using symbolic value' do
+      expect_offense(<<-RUBY)
+        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 }
+      RUBY
+    end
+
+    it 'does not register an offense when using numeric value' do
+      expect_no_offenses(<<-RUBY)
+        it { is_expected.to be_ok }
+      RUBY
+    end
+
+    it 'does not register an offense when using allowed symbols' do
+      expect_no_offenses(<<-RUBY)
+        it { is_expected.to have_http_status :error }
+        it { is_expected.to have_http_status :success }
+        it { is_expected.to have_http_status :missing }
+        it { is_expected.to have_http_status :redirect }
+      RUBY
+    end
+
+    context 'with parenthesis' do
+      it 'registers an offense when using numeric value' do
+        expect_offense(<<-RUBY)
+          it { is_expected.to have_http_status(404) }
+                              ^^^^^^^^^^^^^^^^^^^^^ Prefer `be_not_found` over `have_http_status(404)` to describe HTTP status code.
+        RUBY
+
+        expect_correction(<<-RUBY)
+          it { is_expected.to be_not_found }
+        RUBY
+      end
+
+      it 'registers an offense when using symbolic value' do
+        expect_offense(<<-RUBY)
+          it { is_expected.to have_http_status(:not_found) }
+                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `be_not_found` over `have_http_status(:not_found)` to describe HTTP status code.
+        RUBY
+
+        expect_correction(<<-RUBY)
+          it { is_expected.to be_not_found }
+        RUBY
+      end
+    end
+  end
 end
