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 new Rails/FreezeTime cop #714

Merged
merged 1 commit into from Jul 18, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/new_add_new_rails_freeze_time_cop.md
@@ -0,0 +1 @@
* [#714](https://github.com/rubocop/rubocop-rails/pull/714): Add new `Rails/FreezeTime` cop. ([@ydah][])
7 changes: 7 additions & 0 deletions config/default.yml
Expand Up @@ -417,6 +417,13 @@ Rails/FindEach:
- select
- lock

Rails/FreezeTime:
Description: 'Prefer `freeze_time` over `travel_to` with an argument of the current time.'
ydah marked this conversation as resolved.
Show resolved Hide resolved
StyleGuide: 'https://rails.rubystyle.guide/#freeze-time'
Enabled: pending
VersionAdded: '<<next>>'
SafeAutoCorrect: false

Rails/HasAndBelongsToMany:
Description: 'Prefer has_many :through to has_and_belongs_to_many.'
StyleGuide: 'https://rails.rubystyle.guide#has-many-through'
Expand Down
70 changes: 70 additions & 0 deletions lib/rubocop/cop/rails/freeze_time.rb
@@ -0,0 +1,70 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Identifies usages of `travel_to` with an argument of the current time and
# change them to use `freeze_time` instead.
#
# @safety
# This cop’s autocorrection is unsafe because `freeze_time` just delegates to
# `travel_to` with a default `Time.now`, it is not strictly equivalent to `Time.now`
# if the argument of `travel_to` is the current time considering time zone.
#
# @example
# # bad
# travel_to(Time.now)
# travel_to(Time.new)
# travel_to(DateTime.now)
# travel_to(Time.current)
# travel_to(Time.zone.now)
# travel_to(Time.now.in_time_zone)
ydah marked this conversation as resolved.
Show resolved Hide resolved
# travel_to(Time.current.to_time)
#
# # good
# freeze_time
#
class FreezeTime < Base
extend AutoCorrector

MSG = 'Use `freeze_time` instead of `travel_to`.'
NOW_METHODS = %i[now new current].freeze
CONV_METHODS = %i[to_time in_time_zone].freeze
RESTRICT_ON_SEND = %i[travel_to].freeze

# @!method time_now?(node)
def_node_matcher :time_now?, <<~PATTERN
(const nil? {:Time :DateTime})
PATTERN

# @!method zoned_time_now?(node)
def_node_matcher :zoned_time_now?, <<~PATTERN
(send (const nil? :Time) :zone)
PATTERN

def on_send(node)
child_node, method_name = *node.first_argument.children
if current_time?(child_node, method_name) ||
current_time_with_convert?(child_node, method_name)
add_offense(node) { |corrector| corrector.replace(node, 'freeze_time') }
end
end

private

def current_time?(node, method_name)
return false unless NOW_METHODS.include?(method_name)

node.send_type? ? zoned_time_now?(node) : time_now?(node)
end

def current_time_with_convert?(node, method_name)
return false unless CONV_METHODS.include?(method_name)

child_node, child_method_name = *node.children
current_time?(child_node, child_method_name)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Expand Up @@ -50,6 +50,7 @@
require_relative 'rails/find_by'
require_relative 'rails/find_by_id'
require_relative 'rails/find_each'
require_relative 'rails/freeze_time'
require_relative 'rails/has_and_belongs_to_many'
require_relative 'rails/has_many_or_has_one_dependent'
require_relative 'rails/helper_instance_variable'
Expand Down
73 changes: 73 additions & 0 deletions spec/rubocop/cop/rails/freeze_time_spec.rb
@@ -0,0 +1,73 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::FreezeTime, :config do
it 'registers an offense when using `travel_to` with an argument of the current time' do
expect_offense(<<~RUBY)
travel_to(Time.now)
^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
travel_to(Time.new)
^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
travel_to(DateTime.now)
^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
travel_to(Time.current)
^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
travel_to(Time.zone.now)
^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
travel_to(Time.now.in_time_zone)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
travel_to(Time.current.to_time)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
RUBY

expect_correction(<<~RUBY)
freeze_time
freeze_time
freeze_time
freeze_time
freeze_time
freeze_time
freeze_time
RUBY
end

it 'registers an offense when using `travel_to` with an argument of the current time and `do-end` block' do
expect_offense(<<~RUBY)
travel_to(Time.now) do
^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
do_something
end
RUBY

expect_correction(<<~RUBY)
freeze_time do
do_something
end
RUBY
end

it 'registers an offense when using `travel_to` with an argument of the current time and `{}` block' do
expect_offense(<<~RUBY)
travel_to(Time.now) { do_something }
^^^^^^^^^^^^^^^^^^^ Use `freeze_time` instead of `travel_to`.
RUBY

expect_correction(<<~RUBY)
freeze_time { do_something }
RUBY
end

it 'does not register an offense when using `freeze_time`' do
expect_no_offenses(<<~RUBY)
freeze_time
RUBY
end

it 'does not register an offense when using `travel_to` with an argument of the not current time' do
expect_no_offenses(<<~RUBY)
travel_to(Time.current.yesterday)
travel_to(Time.zone.tomorrow)
travel_to(DateTime.next_day)
travel_to(Time.zone.yesterday.in_time_zone)
RUBY
end
end