Skip to content

Commit

Permalink
Add RSpec/Rails/TravelAround cop
Browse files Browse the repository at this point in the history
  • Loading branch information
r7kamura authored and pirj committed Jan 23, 2023
1 parent 1d652a5 commit c8505ac
Show file tree
Hide file tree
Showing 7 changed files with 301 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Fix a false positive for `RSpec/VariableDefinition` when inside non-spec code. ([@ydah])
- Add new `RSpec/PendingBlockInsideExample` cop. ([@ydah])
- Add `RSpec/RedundantAround` cop. ([@r7kamura])
- Add `RSpec/Rails/TravelAround` cop. ([@r7kamura])

## 2.18.1 (2023-01-19)

Expand Down
7 changes: 7 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1068,3 +1068,10 @@ RSpec/Rails/MinitestAssertions:
Enabled: pending
VersionAdded: '2.17'
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/MinitestAssertions

RSpec/Rails/TravelAround:
Description: Prefer to travel in `before` rather than `around`.
Enabled: pending
Safe: false
VersionAdded: "<<next>>"
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/TravelAround
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -123,5 +123,6 @@
* xref:cops_rspec_rails.adoc#rspecrails/httpstatus[RSpec/Rails/HttpStatus]
* xref:cops_rspec_rails.adoc#rspecrails/inferredspectype[RSpec/Rails/InferredSpecType]
* xref:cops_rspec_rails.adoc#rspecrails/minitestassertions[RSpec/Rails/MinitestAssertions]
* xref:cops_rspec_rails.adoc#rspecrails/travelaround[RSpec/Rails/TravelAround]

// END_COP_LIST
42 changes: 42 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,45 @@ expect(a).not_to eq(b)
=== References

* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/MinitestAssertions

== RSpec/Rails/TravelAround

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Pending
| No
| Yes (Unsafe)
| <<next>>
| -
|===

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.

And also, this cop's autocorrection is unsafe because the order of
execution will change if other steps exist before traveling in
`around`.

=== Examples

[source,ruby]
----
# bad
around do |example|
freeze_time do
example.run
end
end
# good
before { freeze_time }
----

=== References

* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Rails/TravelAround
92 changes: 92 additions & 0 deletions lib/rubocop/cop/rspec/rails/travel_around.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
module Rails
# 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.
#
# And also, this cop's autocorrection is unsafe because the order of
# execution will change if other steps exist before traveling in
# `around`.
#
# @example
# # bad
# around do |example|
# freeze_time do
# example.run
# end
# end
#
# # good
# before { freeze_time }
class TravelAround < Base
extend AutoCorrector

MSG = 'Prefer to travel in `before` rather than `around`.'

TRAVEL_METHOD_NAMES = %i[
freeze_time
travel
travel_to
].to_set.freeze

# @!method extract_run_in_travel(node)
def_node_matcher :extract_run_in_travel, <<~PATTERN
(block
$(send nil? TRAVEL_METHOD_NAMES ...)
(args ...)
(send _ :run)
)
PATTERN

# @!method match_around_each?(node)
def_node_matcher :match_around_each?, <<~PATTERN
(block
(send _ :around (sym :each)?)
...
)
PATTERN

def on_block(node)
run_node = extract_run_in_travel(node)
return unless run_node

around_node = extract_surrounding_around_block(run_node)
return unless around_node

add_offense(node) do |corrector|
autocorrect(corrector, node, run_node, around_node)
end
end
alias on_numblock on_block

private

def autocorrect(corrector, node, run_node, around_node)
corrector.replace(
node,
node.body.source
)
corrector.insert_before(
around_node,
"before { #{run_node.source} }\n\n"
)
end

# @param node [RuboCop::AST::BlockNode]
# @return [RuboCop::AST::BlockNode, nil]
def extract_surrounding_around_block(node)
node.each_ancestor(:block).find do |ancestor|
match_around_each?(ancestor)
end
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rspec_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
end
require_relative 'rspec/rails/inferred_spec_type'
require_relative 'rspec/rails/minitest_assertions'
require_relative 'rspec/rails/travel_around'

require_relative 'rspec/align_left_let_brace'
require_relative 'rspec/align_right_let_brace'
Expand Down
157 changes: 157 additions & 0 deletions spec/rubocop/cop/rspec/rails/travel_around_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::RSpec::Rails::TravelAround do
context 'with `freeze_time` in `before`' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
before { freeze_time }
RUBY
end
end

context 'with `freeze_time` in `around(:all)`' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
around(:all) do |example|
freeze_time do
example.run
end
end
RUBY
end
end

context 'with `freeze_time` in `around(:suite)`' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
around(:suite) do |example|
freeze_time do
example.run
end
end
RUBY
end
end

context 'with another step in `freeze_time`' do
it 'registers no offense' do
expect_no_offenses(<<~RUBY)
around do |example|
freeze_time do
do_some_preparation
example.run
end
end
RUBY
end
end

context 'with `freeze_time` in `around`' do
it 'registers offense' do
expect_offense(<<~RUBY)
around do |example|
freeze_time do
^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
example.run
end
end
RUBY

expect_correction(<<~RUBY)
before { freeze_time }
around do |example|
example.run
end
RUBY
end
end

context 'with `freeze_time` in `around(:each)`' do
it 'registers offense' do
expect_offense(<<~RUBY)
around(:each) do |example|
freeze_time do
^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
example.run
end
end
RUBY

expect_correction(<<~RUBY)
before { freeze_time }
around(:each) do |example|
example.run
end
RUBY
end
end

context 'with `freeze_time` and another node in `around`' do
it 'registers offense' do
expect_offense(<<~RUBY)
around do |example|
foo
freeze_time do
^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
example.run
end
end
RUBY

expect_correction(<<~RUBY)
before { freeze_time }
around do |example|
foo
example.run
end
RUBY
end
end

context 'with `travel` in `around`' do
it 'registers offense' do
expect_offense(<<~RUBY)
around do |example|
travel(duration) do
^^^^^^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
example.run
end
end
RUBY

expect_correction(<<~RUBY)
before { travel(duration) }
around do |example|
example.run
end
RUBY
end
end

context 'with `travel_to` in `around`' do
it 'registers offense' do
expect_offense(<<~RUBY)
around do |example|
travel_to(time) do
^^^^^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
example.run
end
end
RUBY

expect_correction(<<~RUBY)
before { travel_to(time) }
around do |example|
example.run
end
RUBY
end
end
end

0 comments on commit c8505ac

Please sign in to comment.