Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RSpec/Rails/TravelAround and RSpec/RedundantAround cops #1503

Merged
merged 2 commits into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ RSpec/NoExpectationExample:
Enabled: true
RSpec/PendingWithoutReason:
Enabled: true
RSpec/RedundantAround:
Enabled: true
RSpec/SortMetadata:
Enabled: true
RSpec/SubjectDeclaration:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
- Fix a false positive for `RSpec/VariableName` when inside non-spec code. ([@ydah])
- 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
13 changes: 13 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,12 @@ RSpec/ReceiveNever:
VersionAdded: '1.28'
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ReceiveNever

RSpec/RedundantAround:
Description: Remove redundant `around` hook.
Enabled: pending
VersionAdded: "<<next>>"
Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RedundantAround

RSpec/RepeatedDescription:
Description: Check for repeated description strings in example groups.
Enabled: true
Expand Down Expand Up @@ -1062,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
2 changes: 2 additions & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
* xref:cops_rspec.adoc#rspecpredicatematcher[RSpec/PredicateMatcher]
* xref:cops_rspec.adoc#rspecreceivecounts[RSpec/ReceiveCounts]
* xref:cops_rspec.adoc#rspecreceivenever[RSpec/ReceiveNever]
* xref:cops_rspec.adoc#rspecredundantaround[RSpec/RedundantAround]
* xref:cops_rspec.adoc#rspecrepeateddescription[RSpec/RepeatedDescription]
* xref:cops_rspec.adoc#rspecrepeatedexample[RSpec/RepeatedExample]
* xref:cops_rspec.adoc#rspecrepeatedexamplegroupbody[RSpec/RepeatedExampleGroupBody]
Expand Down Expand Up @@ -122,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
30 changes: 30 additions & 0 deletions docs/modules/ROOT/pages/cops_rspec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -4136,6 +4136,36 @@ expect(foo).not_to receive(:bar)

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

== RSpec/RedundantAround

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

| Pending
| Yes
| Yes
| <<next>>
| -
|===

Remove redundant `around` hook.

=== Examples

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

=== References

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

== RSpec/RepeatedDescription

|===
Expand Down
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
69 changes: 69 additions & 0 deletions lib/rubocop/cop/rspec/redundant_around.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# frozen_string_literal: true

module RuboCop
module Cop
module RSpec
# Remove redundant `around` hook.
#
# @example
# # bad
# around do |example|
# example.run
# end
#
# # good
#
class RedundantAround < Base
extend AutoCorrector

MSG = 'Remove redundant `around` hook.'

RESTRICT_ON_SEND = %i[around].freeze

def on_block(node)
return unless match_redundant_around_hook_block?(node)

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

def on_send(node)
return unless match_redundant_around_hook_send?(node)

add_offense(node) do |corrector|
autocorrect(corrector, node)
end
end

private

# @!method match_redundant_around_hook_block?(node)
def_node_matcher :match_redundant_around_hook_block?, <<~PATTERN
(block
(send _ :around ...)
(args _?)
(send _ :run)
)
PATTERN

# @!method match_redundant_around_hook_send?(node)
def_node_matcher :match_redundant_around_hook_send?, <<~PATTERN
(send
_
:around
...
(block-pass
(sym :run)
)
)
PATTERN

def autocorrect(corrector, node)
corrector.remove(node)
end
end
end
end
end
2 changes: 2 additions & 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 Expand Up @@ -96,6 +97,7 @@
require_relative 'rspec/predicate_matcher'
require_relative 'rspec/receive_counts'
require_relative 'rspec/receive_never'
require_relative 'rspec/redundant_around'
require_relative 'rspec/repeated_description'
require_relative 'rspec/repeated_example'
require_relative 'rspec/repeated_example_group_body'
Expand Down
Loading