Merge pull request #714 from ydah/add_new_rails_freeze_time

Add new `Rails/FreezeTime` cop

Author: koic

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][])

config/default.yml

@@ -417,6 +417,13 @@ 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: '<<next>>'
+  SafeAutoCorrect: false
+
 Rails/HasAndBelongsToMany:
   Description: 'Prefer has_many :through to has_and_belongs_to_many.'
   StyleGuide: 'https://rails.rubystyle.guide#has-many-through'

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)
+      #   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

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'

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