Skip to content

Commit 2d257f2

Browse files
authored
Merge pull request #59 from rubocop/issue49
Fix a false negative for `RSpecRails/TravelAround` cop when passed as a proc to a travel method
2 parents 801dfe4 + 3d76777 commit 2d257f2

File tree

4 files changed

+95
-14
lines changed

4 files changed

+95
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Master (Unreleased)
44

55
- Handle unknown HTTP status codes for `RSpecRails/HttpStatus` cop. ([@viralpraxis])
6+
- Fix a false negative for `RSpecRails/TravelAround` cop when passed as a proc to a travel method. ([@ydah])
67

78
## 2.30.0 (2024-06-12)
89

docs/modules/ROOT/pages/cops_rspecrails.adoc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -456,6 +456,11 @@ around do |example|
456456
end
457457
end
458458
459+
# bad
460+
around do |example|
461+
freeze_time(&example)
462+
end
463+
459464
# good
460465
before { freeze_time }
461466
----

lib/rubocop/cop/rspec_rails/travel_around.rb

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@ module RSpecRails
2121
# end
2222
# end
2323
#
24+
# # bad
25+
# around do |example|
26+
# freeze_time(&example)
27+
# end
28+
#
2429
# # good
2530
# before { freeze_time }
31+
#
2632
class TravelAround < ::RuboCop::Cop::Base
2733
extend AutoCorrector
2834

@@ -43,6 +49,13 @@ class TravelAround < ::RuboCop::Cop::Base
4349
)
4450
PATTERN
4551

52+
# @!method extract_travel_with_block_pass(node)
53+
def_node_search :extract_travel_with_block_pass, <<~PATTERN
54+
$(send _ TRAVEL_METHOD_NAMES
55+
(block_pass $lvar)
56+
)
57+
PATTERN
58+
4659
# @!method match_around_each?(node)
4760
def_node_matcher :match_around_each?, <<~PATTERN
4861
(block
@@ -52,29 +65,39 @@ class TravelAround < ::RuboCop::Cop::Base
5265
PATTERN
5366

5467
def on_block(node)
55-
run_node = extract_run_in_travel(node)
56-
return unless run_node
68+
extract_run_in_travel(node) do |run_node|
69+
run_in_travel(node, run_node)
70+
end
71+
extract_travel_with_block_pass(node) do |travel_node, lvar|
72+
travel_with_block_pass(travel_node, lvar)
73+
end
74+
end
75+
alias on_numblock on_block
76+
77+
private
5778

79+
def run_in_travel(node, run_node)
5880
around_node = extract_surrounding_around_block(run_node)
5981
return unless around_node
6082

6183
add_offense(node) do |corrector|
62-
autocorrect(corrector, node, run_node, around_node)
84+
corrector.replace(node, node.body.source)
85+
corrector.insert_before(around_node,
86+
"before { #{run_node.source} }\n\n")
6387
end
6488
end
65-
alias on_numblock on_block
6689

67-
private
90+
def travel_with_block_pass(node, lvar)
91+
around_node = extract_surrounding_around_block(node)
92+
return unless around_node
6893

69-
def autocorrect(corrector, node, run_node, around_node)
70-
corrector.replace(
71-
node,
72-
node.body.source
73-
)
74-
corrector.insert_before(
75-
around_node,
76-
"before { #{run_node.source} }\n\n"
77-
)
94+
add_offense(node) do |corrector|
95+
corrector.replace(node, "#{lvar.name}.run")
96+
corrector.insert_before(
97+
around_node,
98+
"before { #{node.method_name} }\n\n"
99+
)
100+
end
78101
end
79102

80103
# @param node [RuboCop::AST::BlockNode]

spec/rubocop/cop/rspec_rails/travel_around_spec.rb

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,35 @@
6767
end
6868
end
6969

70+
context 'with `freeze_time` with `&example` in `around`' do
71+
it 'registers offense' do
72+
expect_offense(<<~RUBY)
73+
around do |example|
74+
freeze_time(&example)
75+
^^^^^^^^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
76+
end
77+
RUBY
78+
79+
expect_correction(<<~RUBY)
80+
before { freeze_time }
81+
82+
around do |example|
83+
example.run
84+
end
85+
RUBY
86+
end
87+
end
88+
89+
context 'with `freeze_time` with `&example` not in `around`' do
90+
it 'registers no offense' do
91+
expect_no_offenses(<<~RUBY)
92+
examples.each do |example|
93+
freeze_time(&example)
94+
end
95+
RUBY
96+
end
97+
end
98+
7099
context 'with `freeze_time` in `around(:each)`' do
71100
it 'registers offense' do
72101
expect_offense(<<~RUBY)
@@ -113,6 +142,29 @@
113142
end
114143
end
115144

145+
context 'with `freeze_time` with `&example` and another node in `around`' do
146+
it 'registers offense' do
147+
expect_offense(<<~RUBY)
148+
around do |example|
149+
foo
150+
151+
freeze_time(&example)
152+
^^^^^^^^^^^^^^^^^^^^^ Prefer to travel in `before` rather than `around`.
153+
end
154+
RUBY
155+
156+
expect_correction(<<~RUBY)
157+
before { freeze_time }
158+
159+
around do |example|
160+
foo
161+
162+
example.run
163+
end
164+
RUBY
165+
end
166+
end
167+
116168
context 'with `travel` in `around`' do
117169
it 'registers offense' do
118170
expect_offense(<<~RUBY)

0 commit comments

Comments
 (0)