diff --git a/CHANGELOG.md b/CHANGELOG.md index dc7937985..54d5f5644 100644 --- 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 index 642eb9079..edb5f5a0c 100644 --- 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: "<>" 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 index 7703fe1e4..960fa5b12 100644 --- 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 +| <> |=== 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,6 +114,23 @@ 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 |=== @@ -116,7 +138,7 @@ it { is_expected.to have_http_status :error } | 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 index e9ea41f33..7b7a4a1db 100644 --- 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,13 +78,16 @@ def checker_class SymbolicStyleChecker when :numeric NumericStyleChecker + when :be_status + BeStatusStyleChecker end end # :nodoc: - class SymbolicStyleChecker + class StyleCheckerBase MSG = 'Prefer `%s` over `%s` ' \ 'to describe HTTP status code.' + ALLOWED_STATUSES = %i[error success missing redirect].freeze attr_reader :node @@ -73,16 +95,36 @@ 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 `%s` over `%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 index e3523bd4b..8fdbecbba 100644 --- 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