-
-
Notifications
You must be signed in to change notification settings - Fork 270
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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` (autocorrected) - `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
1 parent
dea8939
commit 9e2486f
Showing
3 changed files
with
235 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
# frozen_string_literal: true | ||
|
||
module RuboCop | ||
module Cop | ||
module Rails | ||
class Timecop < Cop | ||
FREEZE_MESSAGE = 'Use `freeze_time` instead of `Timecop.freeze`' | ||
FREEZE_WITH_ARGUMENTS_MESSAGE = 'Use `travel` or `travel_to` instead of `Timecop.freeze`' | ||
RETURN_MESSAGE = 'Use `travel_back` instead of `Timecop.return`' | ||
TRAVEL_MESSAGE = 'Use `travel` or `travel_to` instead of `Timecop.travel`. If you need time to keep flowing, ' \ | ||
'simulate it by travelling again.' | ||
MSG = 'Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop`' | ||
|
||
FREEZE_TIME = 'freeze_time' | ||
TRAVEL_BACK = 'travel_back' | ||
|
||
TIMECOP_PATTERN_STRING = <<~PATTERN | ||
(const {nil? (:cbase)} :Timecop) | ||
PATTERN | ||
|
||
def_node_matcher :timecop, TIMECOP_PATTERN_STRING | ||
|
||
def_node_matcher :timecop_send, <<~PATTERN | ||
(send | ||
#{TIMECOP_PATTERN_STRING} ${:freeze :return :travel} | ||
$... | ||
) | ||
PATTERN | ||
|
||
def on_const(node) | ||
return unless timecop(node) | ||
|
||
timecop_send(node.parent) do |message, arguments| | ||
return on_timecop_send(node.parent, message, arguments) | ||
end | ||
|
||
add_offense(node) | ||
end | ||
|
||
def autocorrect(node) | ||
lambda do |corrector| | ||
timecop_send(node) do |message, arguments| | ||
case message | ||
when :freeze | ||
autocorrect_freeze(corrector, node, arguments) | ||
when :return | ||
autocorrect_return(corrector, node, arguments) | ||
end | ||
end | ||
end | ||
end | ||
|
||
private | ||
|
||
def on_timecop_send(node, message, arguments) | ||
case message | ||
when :freeze | ||
on_timecop_freeze(node, arguments) | ||
when :return | ||
on_timecop_return(node, arguments) | ||
when :travel | ||
on_timecop_travel(node, arguments) | ||
else | ||
add_offense(node) | ||
end | ||
end | ||
|
||
def on_timecop_freeze(node, arguments) | ||
if arguments.empty? | ||
add_offense(node, message: FREEZE_MESSAGE) | ||
else | ||
add_offense(node, message: FREEZE_WITH_ARGUMENTS_MESSAGE) | ||
end | ||
end | ||
|
||
def on_timecop_return(node, _arguments) | ||
add_offense(node, message: RETURN_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), FREEZE_TIME) | ||
end | ||
|
||
def autocorrect_return(corrector, node, _arguments) | ||
corrector.replace(receiver_and_message_range(node), TRAVEL_BACK) | ||
end | ||
|
||
def receiver_and_message_range(node) | ||
# FIXME: There is probably a better way to do this | ||
# Just trying to get the range of `Timecop.method_name`, without args, or block, or anything | ||
node.location.expression.with(end_pos: node.location.selector.end_pos) | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
# frozen_string_literal: true | ||
|
||
RSpec.describe(RuboCop::Cop::Rails::Timecop, :config) do | ||
subject(:cop) { described_class.new(config) } | ||
|
||
describe 'Timecop.freeze' do | ||
context 'without a block' do | ||
context 'without arguments' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.freeze | ||
^^^^^^^^^^^^^^ Use `freeze_time` instead of `Timecop.freeze` | ||
RUBY | ||
end | ||
|
||
it 'autocorrects to `freeze_time`' do | ||
expect(autocorrect_source('Timecop.freeze')).to(eq('freeze_time')) | ||
end | ||
end | ||
|
||
context 'with arguments' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.freeze(123) | ||
^^^^^^^^^^^^^^^^^^^ Use `travel` or `travel_to` instead of `Timecop.freeze` | ||
RUBY | ||
end | ||
|
||
it 'does not autocorrect' do | ||
source = 'Timecop.freeze(123)' | ||
|
||
expect(autocorrect_source(source)).to(eq(source)) | ||
end | ||
end | ||
end | ||
|
||
context 'with a block' do | ||
context 'without arguments' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.freeze { } | ||
^^^^^^^^^^^^^^ Use `freeze_time` instead of `Timecop.freeze` | ||
RUBY | ||
end | ||
|
||
it 'autocorrects to `freeze_time`' do | ||
expect(autocorrect_source('Timecop.freeze { }')).to(eq('freeze_time { }')) | ||
end | ||
end | ||
|
||
context 'with arguments' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.freeze(123) { } | ||
^^^^^^^^^^^^^^^^^^^ Use `travel` or `travel_to` instead of `Timecop.freeze` | ||
RUBY | ||
end | ||
|
||
# FIXME: Is this how NOT autocorrecting something should be tested? | ||
it 'does not autocorrect' do | ||
source = 'Timecop.freeze(123) { }' | ||
|
||
expect(autocorrect_source(source)).to(eq(source)) | ||
end | ||
end | ||
end | ||
end | ||
|
||
describe 'Timecop.return' do | ||
context 'without a block' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.return | ||
^^^^^^^^^^^^^^ Use `travel_back` instead of `Timecop.return` | ||
RUBY | ||
end | ||
|
||
it 'autocorrects to `travel_back`' do | ||
expect(autocorrect_source('Timecop.return')).to(eq('travel_back')) | ||
end | ||
end | ||
|
||
context 'with a block' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.return { } | ||
^^^^^^^^^^^^^^ Use `travel_back` instead of `Timecop.return` | ||
RUBY | ||
end | ||
|
||
it 'autocorrects to `travel_back`' do | ||
expect(autocorrect_source('Timecop.return { }')).to(eq('travel_back { }')) | ||
end | ||
end | ||
end | ||
|
||
describe 'Timecop.travel' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.travel(123) { } | ||
^^^^^^^^^^^^^^^^^^^ Use `travel` or `travel_to` instead of `Timecop.travel`. If you need time to keep flowing, simulate it by travelling again. | ||
RUBY | ||
end | ||
end | ||
|
||
describe 'Timecop.*' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.foo | ||
^^^^^^^ Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop` | ||
RUBY | ||
end | ||
end | ||
|
||
describe 'Timecop' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
Timecop.foo | ||
^^^^^^^ Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop` | ||
RUBY | ||
end | ||
end | ||
|
||
describe '::Timecop' do | ||
it 'adds an offense' do | ||
expect_offense(<<~RUBY) | ||
::Timecop.foo | ||
^^^^^^^^^ Use `ActiveSupport::Testing::TimeHelpers` instead of `Timecop` | ||
RUBY | ||
end | ||
end | ||
end |