Skip to content

Commit

Permalink
Add Timecop cop
Browse files Browse the repository at this point in the history
This cop makes `Timecop` illegal, in favour of
`ActiveSupport::Testing::TimeHelpers`.

Specifically,

- `Timecop.freeze` should be replaced with `freeze_time` (autocorrected)
- `Timecop.freeze(...)` should be replaced with `travel` or `travel_to`
- `Timecop.return` should be replaced with `travel_back` or `unfreeze_time` (autocorrected)
- `Timecop.scale` & `Timecop.travel` should be replaced with `travel` or `travel_to`.
  - Explicitly travelling again should be used instead of relying on time continuing to flow
- `Timecop` should not appear anywhere
  • Loading branch information
sambostock committed Feb 14, 2023
1 parent 38a2ad0 commit c14c393
Show file tree
Hide file tree
Showing 7 changed files with 473 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -784,3 +784,4 @@
[@bensheldon]: https://github.com/bensheldon
[@gurix]: https://github.com/gurix
[@etiennebarrie]: https://github.com/etiennebarrie
[@sambostock]: https://github.com/sambostock
1 change: 1 addition & 0 deletions changelog/new_add_timecop_cop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#38](https://github.com/rubocop/rubocop-rails/pull/38): Add `Timecop` cop. ([@sambostock][])
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,11 @@ Rails/TimeZoneAssignment:
- spec/**/*.rb
- test/**/*.rb

Rails/Timecop:
Description: 'Prefer `ActiveSupport::Testing::TimeHelpers` over `Timecop`.'
Enabled: pending
VersionAdded: <<next>>

Rails/ToFormattedS:
Description: 'Checks for consistent uses of `to_fs` or `to_formatted_s`.'
StyleGuide: 'https://rails.rubystyle.guide/#prefer-to-fs'
Expand Down
87 changes: 87 additions & 0 deletions legacy-docs/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -2571,6 +2571,93 @@ EnforcedStyle | `flexible` | `strict`, `flexible`
* [https://rails.rubystyle.guide#time](https://rails.rubystyle.guide#time)
* [http://danilenko.org/2012/7/6/rails_timezones](http://danilenko.org/2012/7/6/rails_timezones)

## Rails/Timecop

Enabled by default | Supports autocorrection
--- | ---
Enabled | Yes

This cop disallows all usage of `Timecop`, in favour of
`ActiveSupport::Testing::TimeHelpers`.

## Migration
`Timecop.freeze` should be replaced with `freeze_time` when used
without arguments. Where a `duration` has been passed to `freeze`, it
should be replaced with `travel`. Likewise, where a `time` has been
passed to `freeze`, it should be replaced with `travel_to`.

`Timecop.return` should be replaced with `travel_back`, when used
without a block. `travel_back` does not accept a block, so where
`return` is used with a block, it should be replaced by explicitly
calling `freeze_time` with a block, and passing the `time` to
temporarily return to.

`Timecop.scale` should be replaced by explicitly calling `travel` or
`travel_to` with the expected `durations` or `times`, respectively,
rather than relying on allowing time to continue to flow.

`Timecop.travel` should be replaced by `travel` or `travel_to` when
passed a `duration` or `time`, respectively. As with `Timecop.scale`,
rather than relying on time continuing to flow, it should be travelled
to explicitly.

All other usages of `Timecop` are similarly disallowed.

## Caveats

Note that if using RSpec, `TimeHelpers` are not included by default,
and must be manually included by updating `spec_helper` (or
`rails_helper`):

```ruby
RSpec.configure do |config|
config.include ActiveSupport::Testing::TimeHelpers
end
```

### Examples

```ruby
# bad
Timecop

# bad
Timecop.freeze
Timecop.freeze(duration)
Timecop.freeze(time)

# good
freeze_time
travel(duration)
travel_to(time)

# bad
Timecop.freeze { assert true }
Timecop.freeze(duration) { assert true }
Timecop.freeze(time) { assert true }

# good
freeze_time { assert true }
travel(duration) { assert true }
travel_to(time) { assert true }

# bad
Timecop.travel(duration)
Timecop.travel(time)

# good
travel(duration)
travel_to(time)

# bad
Timecop.return
Timecop.return { assert true }

# good
travel_back
travel_to(time) { assert true }
```

## Rails/UniqBeforePluck

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
198 changes: 198 additions & 0 deletions lib/rubocop/cop/rails/timecop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Disallows all usage of `Timecop`, in favour of
# `ActiveSupport::Testing::TimeHelpers`.
#
# ## Migration
# `Timecop.freeze` should be replaced with `freeze_time` when used
# without arguments. Where a `duration` has been passed to `freeze`, it
# should be replaced with `travel`. Likewise, where a `time` has been
# passed to `freeze`, it should be replaced with `travel_to`.
#
# `Timecop.scale` should be replaced by explicitly calling `travel` or
# `travel_to` with the expected `durations` or `times`, respectively,
# rather than relying on allowing time to continue to flow.
#
# `Timecop.return` should be replaced with `travel_back`, when used
# without a block. `travel_back` does not accept a block, so where
# `return` is used with a block, it should be replaced by explicitly
# calling `freeze_time` with a block, and passing the `time` to
# temporarily return to.
#
# `Timecop.travel` should be replaced by `travel` or `travel_to` when
# passed a `duration` or `time`, respectively. As with `Timecop.scale`,
# rather than relying on time continuing to flow, it should be travelled
# to explicitly.
#
# All other usages of `Timecop` are similarly disallowed.
#
# ## RSpec Caveats
#
# Note that if using RSpec, `TimeHelpers` are not included by default,
# and must be manually included by updating `rails_helper` accordingly:
#
# ```ruby
# RSpec.configure do |config|
# config.include ActiveSupport::Testing::TimeHelpers
# end
# ```
#
# Moreover, because `TimeHelpers` relies on Minitest teardown hooks,
# `rails_helper` must be required (instead of `spec_helper`), or a
# similar adapter layer must be in effect.
#
# @example
# # bad
# Timecop
#
# # bad
# Timecop.freeze
# Timecop.freeze(duration)
# Timecop.freeze(time)
#
# # good
# freeze_time
# travel(duration)
# travel_to(time)
#
# # bad
# Timecop.freeze { assert true }
# Timecop.freeze(duration) { assert true }
# Timecop.freeze(time) { assert true }
#
# # good
# freeze_time { assert true }
# travel(duration) { assert true }
# travel_to(time) { assert true }
#
# # bad
# Timecop.travel(duration)
# Timecop.travel(time)
#
# # good
# travel(duration)
# travel_to(time)
#
# # bad
# Timecop.return
# Timecop.return { assert true }
#
# # good
# travel_back
# travel_to(time) { assert true }
#
# # bad
# Timecop.scale(factor)
# Timecop.scale(factor) { assert true }
#
# # good
# travel(duration)
# travel_to(time)
# travel(duration) { assert true }
# travel_to(time) { assert true }
class Timecop < Base
extend AutoCorrector

FREEZE_MESSAGE = 'Use `%<replacement>s` instead of `Timecop.freeze`'
FREEZE_WITH_ARGUMENTS_MESSAGE = 'Use `travel` or `travel_to` instead of `Timecop.freeze`'
RETURN_MESSAGE = 'Use `%<replacement>s` instead of `Timecop.return`'
FLOW_ADDENDUM = 'If you need time to keep flowing, simulate it by travelling again.'
TRAVEL_MESSAGE = "Use `travel` or `travel_to` instead of `Timecop.travel`. #{FLOW_ADDENDUM}"
SCALE_MESSAGE = "Use `travel` or `travel_to` instead of `Timecop.scale`. #{FLOW_ADDENDUM}"
MSG = 'Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop`'

def_node_matcher :timecop_const?, <<~PATTERN
(const {nil? cbase} :Timecop)
PATTERN

def_node_matcher :timecop_send, <<~PATTERN
(send
#timecop_const? ${:freeze :return :scale :travel}
$...
)
PATTERN

def on_const(node)
return unless timecop_const?(node)

timecop_send(node.parent) do |message, arguments|
return on_timecop_send(node.parent, message, arguments)
end

add_offense(node)
end

private

def on_timecop_send(node, message, arguments)
case message
when :freeze then on_timecop_freeze(node, arguments)
when :return then on_timecop_return(node, arguments)
when :scale then on_timecop_scale(node, arguments)
when :travel then on_timecop_travel(node, arguments)
else add_offense(node)
end
end

def on_timecop_freeze(node, arguments)
if arguments.empty?
add_offense(node, message: format(FREEZE_MESSAGE, replacement: preferred_freeze_replacement)) do |corrector|
autocorrect_freeze(corrector, node, arguments)
end
else
add_offense(node, message: FREEZE_WITH_ARGUMENTS_MESSAGE)
end
end

def on_timecop_return(node, arguments)
add_offense(node, message: format(RETURN_MESSAGE, replacement: preferred_return_replacement)) do |corrector|
autocorrect_return(corrector, node, arguments)
end
end

def on_timecop_scale(node, _arguments)
add_offense(node, message: SCALE_MESSAGE)
end

def on_timecop_travel(node, _arguments)
add_offense(node, message: TRAVEL_MESSAGE)
end

def autocorrect_freeze(corrector, node, arguments)
return unless arguments.empty?

corrector.replace(receiver_and_message_range(node), preferred_freeze_replacement)
end

def autocorrect_return(corrector, node, _arguments)
return if given_block?(node)

corrector.replace(receiver_and_message_range(node), preferred_return_replacement)
end

def given_block?(node)
node.send_type? && node.parent && node.parent.block_type? && node.parent.send_node == node
end

def receiver_and_message_range(node)
node.location.expression.with(end_pos: node.location.selector.end_pos)
end

def preferred_freeze_replacement
return 'travel_to(Time.now)' if target_rails_version < 5.2

'freeze_time'
end

def preferred_return_replacement
return 'travel_back' if target_rails_version < 6.0

'unfreeze_time'
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
require_relative 'rails/to_s_with_argument'
require_relative 'rails/top_level_hash_with_indifferent_access'
require_relative 'rails/transaction_exit_statement'
require_relative 'rails/timecop'
require_relative 'rails/uniq_before_pluck'
require_relative 'rails/unique_validation_without_index'
require_relative 'rails/unknown_env'
Expand Down

0 comments on commit c14c393

Please sign in to comment.