Skip to content

Commit d161a5f

Browse files
sambostocksambostock
committed
Update Timecop Cop
- Add documentation & examples - Fix formatting and other offences - Remove erroneous `Timecop.return` with block correction - Add commented out Rails 6 specs for `unfreeze_time` correction
1 parent 9e2486f commit d161a5f

File tree

3 files changed

+227
-32
lines changed

3 files changed

+227
-32
lines changed

‎lib/rubocop/cop/rails/timecop.rb‎

Lines changed: 102 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,105 @@
33
module RuboCop
44
module Cop
55
module Rails
6+
# This cop disallows all usage of `Timecop`, in favour of
7+
# `ActiveSupport::Testing::TimeHelpers`.
8+
#
9+
# ## Migration
10+
# `Timecop.freeze` should be replaced with `freeze_time` when used
11+
# without arguments. Where a `duration` has been passed to `freeze`, it
12+
# should be replaced with `travel`. Likewise, where a `time` has been
13+
# passed to `freeze`, it should be replaced with `travel_to`.
14+
#
15+
# `Timecop.return` should be replaced with `travel_back`, when used
16+
# without a block. `travel_back` does not accept a block, so where
17+
# `return` is used with a block, it should be replaced by explicitly
18+
# calling `freeze_time` with a block, and passing the `time` to
19+
# temporarily return to.
20+
#
21+
# `Timecop.scale` should be replaced by explicitly calling `travel` or
22+
# `travel_to` with the expected `durations` or `times`, respectively,
23+
# rather than relying on allowing time to continue to flow.
24+
#
25+
# `Timecop.travel` should be replaced by `travel` or `travel_to` when
26+
# passed a `duration` or `time`, respectively. As with `Timecop.scale`,
27+
# rather than relying on time continuing to flow, it should be travelled
28+
# to explicitly.
29+
#
30+
# All other usages of `Timecop` are similarly disallowed.
31+
#
32+
# ## Caveats
33+
#
34+
# Note that if using RSpec, `TimeHelpers` are not included by default,
35+
# and must be manually included by updating `spec_helper` (and
36+
# `rails_helper` too, if it exists):
37+
#
38+
# ```ruby
39+
# RSpec.configure do |config|
40+
# config.include ActiveSupport::Testing::TimeHelpers
41+
# end
42+
# ```
43+
#
44+
# @example
45+
# # bad
46+
# Timecop
47+
#
48+
# # bad
49+
# Timecop.freeze
50+
# Timecop.freeze(duration)
51+
# Timecop.freeze(time)
52+
#
53+
# # good
54+
# freeze_time
55+
# travel(duration)
56+
# travel_to(time)
57+
#
58+
# # bad
59+
# Timecop.freeze { assert true }
60+
# Timecop.freeze(duration) { assert true }
61+
# Timecop.freeze(time) { assert true }
62+
#
63+
# # good
64+
# freeze_time { assert true }
65+
# travel(duration) { assert true }
66+
# travel_to(time) { assert true }
67+
#
68+
# # bad
69+
# Timecop.travel(duration)
70+
# Timecop.travel(time)
71+
#
72+
# # good
73+
# travel(duration)
74+
# travel_to(time)
75+
#
76+
# # bad
77+
# Timecop.return
78+
# Timecop.return { assert true }
79+
#
80+
# # good
81+
# travel_back
82+
# travel_to(time) { assert true }
683
class Timecop < Cop
7-
FREEZE_MESSAGE = 'Use `freeze_time` instead of `Timecop.freeze`'
8-
FREEZE_WITH_ARGUMENTS_MESSAGE = 'Use `travel` or `travel_to` instead of `Timecop.freeze`'
9-
RETURN_MESSAGE = 'Use `travel_back` instead of `Timecop.return`'
10-
TRAVEL_MESSAGE = 'Use `travel` or `travel_to` instead of `Timecop.travel`. If you need time to keep flowing, ' \
11-
'simulate it by travelling again.'
12-
MSG = 'Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop`'
13-
14-
FREEZE_TIME = 'freeze_time'
15-
TRAVEL_BACK = 'travel_back'
16-
17-
TIMECOP_PATTERN_STRING = <<~PATTERN
18-
(const {nil? (:cbase)} :Timecop)
19-
PATTERN
84+
FREEZE_MESSAGE =
85+
'Use `freeze_time` instead of `Timecop.freeze`'.freeze
86+
FREEZE_WITH_ARGUMENTS_MESSAGE =
87+
'Use `travel` or `travel_to` instead of `Timecop.freeze`'.freeze
88+
RETURN_MESSAGE =
89+
'Use `travel_back` instead of `Timecop.return`'.freeze
90+
TRAVEL_MESSAGE =
91+
'Use `travel` or `travel_to` instead of `Timecop.travel`. If you ' \
92+
'need time to keep flowing, simulate it by travelling again.'.freeze
93+
MSG =
94+
'Use `ActiveSupport::Testing::TimeHelpers` instead of ' \
95+
'`Timecop`'.freeze
96+
97+
FREEZE_TIME = 'freeze_time'.freeze
98+
TRAVEL_BACK = 'travel_back'.freeze
99+
100+
TIMECOP_PATTERN_STRING = '(const {nil? (:cbase)} :Timecop)'.freeze
20101

21102
def_node_matcher :timecop, TIMECOP_PATTERN_STRING
22103

23-
def_node_matcher :timecop_send, <<~PATTERN
104+
def_node_matcher :timecop_send, <<-PATTERN.strip_indent
24105
(send
25106
#{TIMECOP_PATTERN_STRING} ${:freeze :return :travel}
26107
$...
@@ -88,12 +169,17 @@ def autocorrect_freeze(corrector, node, arguments)
88169
end
89170

90171
def autocorrect_return(corrector, node, _arguments)
172+
return if was_passed_block?(node)
173+
91174
corrector.replace(receiver_and_message_range(node), TRAVEL_BACK)
92175
end
93176

177+
def was_passed_block?(node)
178+
node.send_type? && node.parent &&
179+
node.parent.block_type? && node.parent.send_node == node
180+
end
181+
94182
def receiver_and_message_range(node)
95-
# FIXME: There is probably a better way to do this
96-
# Just trying to get the range of `Timecop.method_name`, without args, or block, or anything
97183
node.location.expression.with(end_pos: node.location.selector.end_pos)
98184
end
99185
end

‎manual/cops_rails.md‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2160,6 +2160,70 @@ EnforcedStyle | `flexible` | `strict`, `flexible`
21602160
* [https://github.com/rubocop-hq/rails-style-guide#time](https://github.com/rubocop-hq/rails-style-guide#time)
21612161
* [http://danilenko.org/2012/7/6/rails_timezones](http://danilenko.org/2012/7/6/rails_timezones)
21622162

2163+
## Rails/Timecop
2164+
2165+
Enabled by default | Supports autocorrection
2166+
--- | ---
2167+
Enabled | Yes
2168+
2169+
This cop disallows all usage of `Timecop`, in favour of
2170+
`ActiveSupport::Testing::TimeHelpers`.
2171+
2172+
FIXME: Add moar
2173+
2174+
Note that if using RSpec, `TimeHelpers` are not included by default,
2175+
and must be manually included by updating `spec_helper` (and
2176+
`rails_helper` too, if it exists):
2177+
2178+
```ruby
2179+
RSpec.configure do |config|
2180+
config.include ActiveSupport::Testing::TimeHelpers
2181+
end
2182+
```
2183+
2184+
### Examples
2185+
2186+
```ruby
2187+
# bad
2188+
Timecop
2189+
2190+
# bad
2191+
Timecop.freeze
2192+
Timecop.freeze(duration)
2193+
Timecop.freeze(time)
2194+
2195+
# good
2196+
freeze_time
2197+
travel(duration)
2198+
travel_to(time)
2199+
2200+
# bad
2201+
Timecop.freeze { assert true }
2202+
Timecop.freeze(duration) { assert true }
2203+
Timecop.freeze(time) { assert true }
2204+
2205+
# good
2206+
freeze_time { assert true }
2207+
travel(duration) { assert true }
2208+
travel_to(time) { assert true }
2209+
2210+
# bad
2211+
Timecop.travel(duration)
2212+
Timecop.travel(time)
2213+
2214+
# good
2215+
travel(duration)
2216+
travel_to(time)
2217+
2218+
# bad
2219+
Timecop.return
2220+
Timecop.return { assert true }
2221+
2222+
# good
2223+
travel_back
2224+
travel_to(time) { assert true }
2225+
```
2226+
21632227
## Rails/UniqBeforePluck
21642228

21652229
Enabled by default | Supports autocorrection

‎spec/rubocop/cop/rails/timecop_spec.rb‎

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
context 'without a block' do
88
context 'without arguments' do
99
it 'adds an offense' do
10-
expect_offense(<<~RUBY)
10+
expect_offense(<<-RUBY.strip_indent)
1111
Timecop.freeze
1212
^^^^^^^^^^^^^^ Use `freeze_time` instead of `Timecop.freeze`
1313
RUBY
@@ -16,11 +16,26 @@
1616
it 'autocorrects to `freeze_time`' do
1717
expect(autocorrect_source('Timecop.freeze')).to(eq('freeze_time'))
1818
end
19+
20+
context 'spread over multiple lines' do
21+
it 'adds an offense' do
22+
expect_offense(<<-RUBY.strip_indent)
23+
Timecop
24+
^^^^^^^ Use `freeze_time` instead of `Timecop.freeze`
25+
.freeze
26+
RUBY
27+
end
28+
29+
it 'autocorrects to `freeze_time`' do
30+
expect(autocorrect_source("Timecop\n .freeze"))
31+
.to(eq('freeze_time'))
32+
end
33+
end
1934
end
2035

2136
context 'with arguments' do
2237
it 'adds an offense' do
23-
expect_offense(<<~RUBY)
38+
expect_offense(<<-RUBY.strip_indent)
2439
Timecop.freeze(123)
2540
^^^^^^^^^^^^^^^^^^^ Use `travel` or `travel_to` instead of `Timecop.freeze`
2641
RUBY
@@ -37,26 +52,26 @@
3752
context 'with a block' do
3853
context 'without arguments' do
3954
it 'adds an offense' do
40-
expect_offense(<<~RUBY)
55+
expect_offense(<<-RUBY.strip_indent)
4156
Timecop.freeze { }
4257
^^^^^^^^^^^^^^ Use `freeze_time` instead of `Timecop.freeze`
4358
RUBY
4459
end
4560

4661
it 'autocorrects to `freeze_time`' do
47-
expect(autocorrect_source('Timecop.freeze { }')).to(eq('freeze_time { }'))
62+
expect(autocorrect_source('Timecop.freeze { }'))
63+
.to(eq('freeze_time { }'))
4864
end
4965
end
5066

5167
context 'with arguments' do
5268
it 'adds an offense' do
53-
expect_offense(<<~RUBY)
69+
expect_offense(<<-RUBY.strip_indent)
5470
Timecop.freeze(123) { }
5571
^^^^^^^^^^^^^^^^^^^ Use `travel` or `travel_to` instead of `Timecop.freeze`
5672
RUBY
5773
end
5874

59-
# FIXME: Is this how NOT autocorrecting something should be tested?
6075
it 'does not autocorrect' do
6176
source = 'Timecop.freeze(123) { }'
6277

@@ -69,34 +84,64 @@
6984
describe 'Timecop.return' do
7085
context 'without a block' do
7186
it 'adds an offense' do
72-
expect_offense(<<~RUBY)
87+
expect_offense(<<-RUBY.strip_indent)
7388
Timecop.return
7489
^^^^^^^^^^^^^^ Use `travel_back` instead of `Timecop.return`
7590
RUBY
7691
end
7792

78-
it 'autocorrects to `travel_back`' do
79-
expect(autocorrect_source('Timecop.return')).to(eq('travel_back'))
93+
context 'in Rails < 6.0', :rails5 do
94+
it 'autocorrects to `travel_back`' do
95+
expect(autocorrect_source('Timecop.return')).to(eq('travel_back'))
96+
end
97+
98+
context 'inside a block' do
99+
it 'autocorrects to `travel_back`' do
100+
expect(autocorrect_source('foo { Timecop.return }'))
101+
.to(eq('foo { travel_back }'))
102+
end
103+
end
80104
end
105+
106+
# context 'in Rails > 6.0', :rails6 do
107+
# it 'autocorrects to `unfreeze`' do
108+
# expect(autocorrect_source('Timecop.return')).to(eq('unfreeze'))
109+
# end
110+
111+
# context 'inside a block' do
112+
# it 'autocorrects to `unfreeze`' do
113+
# expect(autocorrect_source('foo { Timecop.return }'))
114+
# .to(eq('foo { unfreeze }'))
115+
# end
116+
# end
117+
# end
81118
end
82119

83120
context 'with a block' do
84121
it 'adds an offense' do
85-
expect_offense(<<~RUBY)
122+
expect_offense(<<-RUBY.strip_indent)
86123
Timecop.return { }
87124
^^^^^^^^^^^^^^ Use `travel_back` instead of `Timecop.return`
88125
RUBY
89126
end
90127

91-
it 'autocorrects to `travel_back`' do
92-
expect(autocorrect_source('Timecop.return { }')).to(eq('travel_back { }'))
128+
it 'does not autocorrect' do
129+
expect(autocorrect_source('Timecop.return { }'))
130+
.to(eq('Timecop.return { }'))
131+
end
132+
133+
context 'inside a block' do
134+
it 'does not autocorrect' do
135+
expect(autocorrect_source('foo { Timecop.return { } }'))
136+
.to(eq('foo { Timecop.return { } }'))
137+
end
93138
end
94139
end
95140
end
96141

97142
describe 'Timecop.travel' do
98143
it 'adds an offense' do
99-
expect_offense(<<~RUBY)
144+
expect_offense(<<-RUBY.strip_indent)
100145
Timecop.travel(123) { }
101146
^^^^^^^^^^^^^^^^^^^ Use `travel` or `travel_to` instead of `Timecop.travel`. If you need time to keep flowing, simulate it by travelling again.
102147
RUBY
@@ -105,7 +150,7 @@
105150

106151
describe 'Timecop.*' do
107152
it 'adds an offense' do
108-
expect_offense(<<~RUBY)
153+
expect_offense(<<-RUBY.strip_indent)
109154
Timecop.foo
110155
^^^^^^^ Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop`
111156
RUBY
@@ -114,7 +159,7 @@
114159

115160
describe 'Timecop' do
116161
it 'adds an offense' do
117-
expect_offense(<<~RUBY)
162+
expect_offense(<<-RUBY.strip_indent)
118163
Timecop.foo
119164
^^^^^^^ Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop`
120165
RUBY
@@ -123,7 +168,7 @@
123168

124169
describe '::Timecop' do
125170
it 'adds an offense' do
126-
expect_offense(<<~RUBY)
171+
expect_offense(<<-RUBY.strip_indent)
127172
::Timecop.foo
128173
^^^^^^^^^ Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop`
129174
RUBY

0 commit comments

Comments
 (0)