From 339a7f5e91071cd30c7624eb0c06d8139ffdb56d Mon Sep 17 00:00:00 2001 From: ydah <13041216+ydah@users.noreply.github.com> Date: Wed, 15 Jun 2022 17:35:39 +0900 Subject: [PATCH] Add new `Rails/FreezeTime` cop Refs: https://github.com/rubocop/rails-style-guide#freeze_time This cop Identifies usages of `travel_to` with argument current time and change them to use `freeze_time` instead. @example ```ruby # bad travel_to(Time.now) travel_to(DateTime.now) travel_to(Time.current) travel_to(Time.zone.now) travel_to(Time.now.in_time_zone) travel_to(Time.current.to_time) # good freeze_time ``` --- .../new_add_new_rails_freeze_time_cop.md | 1 + config/default.yml | 6 ++ lib/rubocop/cop/rails/freeze_time.rb | 65 +++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + spec/rubocop/cop/rails/freeze_time_spec.rb | 73 +++++++++++++++++++ 5 files changed, 146 insertions(+) create mode 100644 changelog/new_add_new_rails_freeze_time_cop.md create mode 100644 lib/rubocop/cop/rails/freeze_time.rb create mode 100644 spec/rubocop/cop/rails/freeze_time_spec.rb diff --git a/changelog/new_add_new_rails_freeze_time_cop.md b/changelog/new_add_new_rails_freeze_time_cop.md new file mode 100644 index 0000000000..03540985db --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index d1180db658..2ea6206f9d 100644 --- a/config/default.yml +++ b/config/default.yml @@ -413,6 +413,12 @@ Rails/FindEach: - select - lock +Rails/FreezeTime: + Description: 'Prefer `freeze_time` over `travel_to` with an argument of the current time.' + StyleGuide: 'https://rails.rubystyle.guide/#freeze-time' + Enabled: pending + VersionAdded: '<>' + Rails/HasAndBelongsToMany: Description: 'Prefer has_many :through to has_and_belongs_to_many.' StyleGuide: 'https://rails.rubystyle.guide#has-many-through' diff --git a/lib/rubocop/cop/rails/freeze_time.rb b/lib/rubocop/cop/rails/freeze_time.rb new file mode 100644 index 0000000000..cb8691d9af --- /dev/null +++ b/lib/rubocop/cop/rails/freeze_time.rb @@ -0,0 +1,65 @@ +# 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. + # + # @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) + # 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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index cf7c6c5769..af165d82c3 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -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' diff --git a/spec/rubocop/cop/rails/freeze_time_spec.rb b/spec/rubocop/cop/rails/freeze_time_spec.rb new file mode 100644 index 0000000000..12c3b91b37 --- /dev/null +++ b/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