Skip to content

Commit

Permalink
Add support be_status style for RSpec/Rails/HttpStatus
Browse files Browse the repository at this point in the history
Fix: #1148
  • Loading branch information
ydah committed Apr 18, 2023
1 parent 80c7d96 commit af64ecd
Show file tree
Hide file tree
Showing 5 changed files with 181 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
3 changes: 2 additions & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
26 changes: 24 additions & 2 deletions docs/modules/ROOT/pages/cops_rspec_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,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)
Expand Down Expand Up @@ -108,14 +113,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
Expand Down
122 changes: 89 additions & 33 deletions lib/rubocop/cop/rspec/rails/http_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
65 changes: 65 additions & 0 deletions spec/rubocop/cop/rspec/rails/http_status_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit af64ecd

Please sign in to comment.