Skip to content

Commit

Permalink
Extract RSpec Rails cops
Browse files Browse the repository at this point in the history
  • Loading branch information
ydah committed Mar 28, 2024
1 parent 281a65b commit ae19ff1
Show file tree
Hide file tree
Showing 24 changed files with 289 additions and 2,694 deletions.
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Layout/LineLength:
Max: 80 # default: 120
AllowedPatterns:
- '^\s*# .*https?:\/\/.+\[.+\]\.?$' # Allow long asciidoc links
Exclude:
- lib/rubocop/cop/rspec/rails/have_http_status.rb

Layout/MultilineMethodCallIndentation:
EnforcedStyle: indented
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Master (Unreleased)

- Extract RSpec Rails cops to a separate repository, [`rubocop-rspec_rails`](https://github.com/rubocop/rubocop-rspec_rails). The `rubocop-rspec_rails` repository is a dependency of `rubocop-rspec` and the cops related to rspec-rails are aliased (`RSpec/Rails/Foo` == `RSpecRails/Foo`) until v3.0 is released, so the change will be invisible to users until then. ([@ydah])

## 2.27.1 (2024-03-03)

- Fix a false positive for `RSpec/RepeatedSubjectCall` when `subject.method_call`. ([@ydah])
Expand Down
7 changes: 7 additions & 0 deletions config/obsoletion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,10 @@ renamed:
RSpec/FactoryBot/FactoryClassName: FactoryBot/FactoryClassName
RSpec/FactoryBot/FactoryNameStyle: FactoryBot/FactoryNameStyle
RSpec/FactoryBot/SyntaxMethods: FactoryBot/SyntaxMethods
RSpec/Rails/AvoidSetupHook: RSpecRails/AvoidSetupHook
RSpec/Rails/HaveHttpStatus: RSpecRails/HaveHttpStatus
RSpec/Rails/HttpStatus: RSpecRails/HttpStatus
RSpec/Rails/InferredSpecType: RSpecRails/InferredSpecType
RSpec/Rails/MinitestAssertions: RSpecRails/MinitestAssertions
RSpec/Rails/NegationBeValid: RSpecRails/NegationBeValid
RSpec/Rails/TravelAround: RSpecRails/TravelAround
20 changes: 11 additions & 9 deletions docs/modules/ROOT/pages/cops_rspec_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
| -
|===
Checks that tests use RSpec `before` hook over Rails `setup` method.
Checks that tests use RSpec `before` hook over Rails `setup`
method.
=== Examples
Expand Down Expand Up @@ -112,8 +113,8 @@ expect(last_response).to have_http_status(200)
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
So, this cop does not check if a method starting with `be_*` is
used when setting for `EnforcedStyle: symbolic` or
`EnforcedStyle: numeric`.
=== Examples
Expand Down Expand Up @@ -201,7 +202,8 @@ Identifies redundant spec type.
After setting up rspec-rails, you will have enabled
`config.infer_spec_type_from_file_location!` by default in
spec/rails_helper.rb. This cop works in conjunction with this config.
spec/rails_helper.rb. This cop works in conjunction with
this config.
If you disable this config, disable this cop as well.
=== Safety
Expand Down Expand Up @@ -393,12 +395,12 @@ Prefer to travel in `before` rather than `around`.
=== Safety
This cop is unsafe because the automatic `travel_back` is only run
on test cases that are considered as Rails related.
This cop is unsafe because the automatic `travel_back` is only
run on test cases that are considered as Rails related.
And also, this cop's autocorrection is unsafe because the order of
execution will change if other steps exist before traveling in
`around`.
And also, this cop's autocorrection is unsafe because the order
of execution will change if other steps exist before traveling
in `around`.
=== Examples
Expand Down
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/departments.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ There are plans for extracting the following three departments before RuboCop RS
|Extracted in https://github.com/rubocop/rubocop-rspec/releases/tag/v2.22.0[v2.22.0]

|RSpec/Rails
|rubocop-rspec-rails
|Not yet extracted
|rubocop-rspec_rails
|Extracted in https://github.com/rubocop/rubocop-rspec/releases/tag/v2.28.0[v2.28.0]
|===

=== Migration manual
Expand Down
3 changes: 3 additions & 0 deletions lib/rubocop-rspec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
require_relative 'rubocop/rspec/example_group'
require_relative 'rubocop/rspec/hook'

# need after `require 'rubocop/cop/rspec/base'``
require 'rubocop-rspec_rails'

RuboCop::RSpec::Inject.defaults!

require_relative 'rubocop/cop/rspec_cops'
Expand Down
50 changes: 17 additions & 33 deletions lib/rubocop/cop/rspec/rails/avoid_setup_hook.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,23 @@ module RuboCop
module Cop
module RSpec
module Rails
# Checks that tests use RSpec `before` hook over Rails `setup` method.
#
# @example
# # bad
# setup do
# allow(foo).to receive(:bar)
# end
#
# # good
# before do
# allow(foo).to receive(:bar)
# end
#
class AvoidSetupHook < Base
extend AutoCorrector

MSG = 'Use `before` instead of `setup`.'

# @!method setup_call(node)
def_node_matcher :setup_call, <<~PATTERN
(block
$(send nil? :setup)
(args) _)
PATTERN

def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
setup_call(node) do |setup|
add_offense(node) do |corrector|
corrector.replace setup, 'before'
end
end
end
end
# @!parse
# # Checks that tests use RSpec `before` hook over Rails `setup`
# # method.
# #
# # @example
# # # bad
# # setup do
# # allow(foo).to receive(:bar)
# # end
# #
# # # good
# # before do
# # allow(foo).to receive(:bar)
# # end
# #
# class AvoidSetupHook < RuboCop::Cop::RSpecRails::Base; end
AvoidSetupHook = ::RuboCop::Cop::RSpecRails::AvoidSetupHook
end
end
end
Expand Down
94 changes: 25 additions & 69 deletions lib/rubocop/cop/rspec/rails/have_http_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,75 +4,31 @@ module RuboCop
module Cop
module RSpec
module Rails
# Checks that tests use `have_http_status` instead of equality matchers.
#
# @example ResponseMethods: ['response', 'last_response'] (default)
# # bad
# expect(response.status).to be(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>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

# @!method match_status(node)
def_node_matcher :match_status, <<~PATTERN
(send
(send nil? :expect
$(send $(send nil? #response_methods?) {:status :code})
)
$RUNNERS
$(send nil? {:be :eq :eql :equal} ({int str} $_))
)
PATTERN

def on_send(node) # rubocop:disable 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, 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_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
# @!parse
# # Checks that tests use `have_http_status` instead of equality matchers.
# #
# # @example ResponseMethods: ['response', 'last_response'] (default)
# # # bad
# # expect(response.status).to be(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; end
HaveHttpStatus = ::RuboCop::Cop::RSpecRails::HaveHttpStatus
end
end
end
Expand Down

0 comments on commit ae19ff1

Please sign in to comment.