Merge pull request #481 from koic/add_allow_to_time_option_to_rails_date

[Fix #288] Add `AllowToTime` option to `Rails/Date`
rubocop · May 12, 2021 · c893de1 · c893de1
2 parents 3683d92 + b06298d
commit c893de1
Show file tree

Hide file tree

Showing 5 changed files with 151 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@
 * [#482](https://github.com/rubocop/rubocop-rails/pull/482): Fix a false positive for `Rails/RelativeDateConstant` when assigning (hashes/arrays/etc)-containing procs to a constant. ([@jdelStrother][])
 * [#419](https://github.com/rubocop/rubocop-rails/issues/419): Fix an error for `Rails/UniqueValidationWithoutIndex` when using a unique index and `check_constraint` that has `nil` first argument. ([@koic][])
 
+### Changes
+
+* [#288](https://github.com/rubocop/rubocop-rails/issues/288): Add `AllowToTime` option (`true` by default) to `Rails/Date`. ([@koic][])
+
 ## 2.10.1 (2021-05-06)
 
 ### Bug fixes

diff --git a/config/default.yml b/config/default.yml
@@ -193,7 +193,7 @@ Rails/Date:
                   such as Date.today, Date.current etc.
   Enabled: true
   VersionAdded: '0.30'
-  VersionChanged: '0.33'
+  VersionChanged: '2.11'
   # The value `strict` disallows usage of `Date.today`, `Date.current`,
   # `Date#to_time` etc.
   # The value `flexible` allows usage of `Date.current`, `Date.yesterday`, etc
@@ -203,6 +203,7 @@ Rails/Date:
   SupportedStyles:
     - strict
     - flexible
+  AllowToTime: true
 
 Rails/DefaultScope:
   Description: 'Avoid use of `default_scope`.'

diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc
@@ -884,7 +884,7 @@ end
 | Yes
 | No
 | 0.30
-| 0.33
+| 2.11
 |===
 
 This cop checks for the correct use of Date methods,
@@ -896,13 +896,15 @@ Rails time zone. You must use `Time.zone.today` instead.
 The cop also reports warnings when you are using `to_time` method,
 because it doesn't know about Rails time zone either.
 
-Two styles are supported for this cop. When EnforcedStyle is 'strict'
+Two styles are supported for this cop. When `EnforcedStyle` is 'strict'
 then the Date methods `today`, `current`, `yesterday`, and `tomorrow`
 are prohibited and the usage of both `to_time`
 and 'to_time_in_current_zone' are reported as warning.
 
-When EnforcedStyle is 'flexible' then only `Date.today` is prohibited
-and only `to_time` is reported as warning.
+When `EnforcedStyle` is `flexible` then only `Date.today` is prohibited.
+
+And you can set a warning for `to_time` with `AllowToTime: false`.
+`AllowToTime` is `true` by default to prevent false positive on `DateTime` object.
 
 === Examples
 
@@ -914,7 +916,6 @@ and only `to_time` is reported as warning.
 Date.current
 Date.yesterday
 Date.today
-date.to_time
 
 # good
 Time.zone.today
@@ -927,7 +928,6 @@ Time.zone.today - 1.day
 ----
 # bad
 Date.today
-date.to_time
 
 # good
 Time.zone.today
@@ -937,6 +937,22 @@ Date.yesterday
 date.in_time_zone
 ----
 
+==== AllowToTime: true (default)
+
+[source,ruby]
+----
+# good
+date.to_time
+----
+
+==== AllowToTime: false
+
+[source,ruby]
+----
+# bad
+date.to_time
+----
+
 === Configurable attributes
 
 |===
@@ -945,6 +961,10 @@ date.in_time_zone
 | EnforcedStyle
 | `flexible`
 | `strict`, `flexible`
+
+| AllowToTime
+| `true`
+| Boolean
 |===
 
 == Rails/DefaultScope

diff --git a/lib/rubocop/cop/rails/date.rb b/lib/rubocop/cop/rails/date.rb
@@ -12,20 +12,21 @@ module Rails
       # The cop also reports warnings when you are using `to_time` method,
       # because it doesn't know about Rails time zone either.
       #
-      # Two styles are supported for this cop. When EnforcedStyle is 'strict'
+      # Two styles are supported for this cop. When `EnforcedStyle` is 'strict'
       # then the Date methods `today`, `current`, `yesterday`, and `tomorrow`
       # are prohibited and the usage of both `to_time`
       # and 'to_time_in_current_zone' are reported as warning.
       #
-      # When EnforcedStyle is 'flexible' then only `Date.today` is prohibited
-      # and only `to_time` is reported as warning.
+      # When `EnforcedStyle` is `flexible` then only `Date.today` is prohibited.
+      #
+      # And you can set a warning for `to_time` with `AllowToTime: false`.
+      # `AllowToTime` is `true` by default to prevent false positive on `DateTime` object.
       #
       # @example EnforcedStyle: strict
       #   # bad
       #   Date.current
       #   Date.yesterday
       #   Date.today
-      #   date.to_time
       #
       #   # good
       #   Time.zone.today
@@ -34,7 +35,6 @@ module Rails
       # @example EnforcedStyle: flexible (default)
       #   # bad
       #   Date.today
-      #   date.to_time
       #
       #   # good
       #   Time.zone.today
@@ -43,6 +43,13 @@ module Rails
       #   Date.yesterday
       #   date.in_time_zone
       #
+      # @example AllowToTime: true (default)
+      #   # good
+      #   date.to_time
+      #
+      # @example AllowToTime: false
+      #   # bad
+      #   date.to_time
       class Date < Base
         include ConfigurableEnforcedStyle
 
@@ -73,7 +80,7 @@ def on_const(node)
 
         def on_send(node)
           return unless node.receiver && bad_methods.include?(node.method_name)
-
+          return if allow_to_time? && node.method?(:to_time)
           return if safe_chain?(node) || safe_to_time?(node)
 
           check_deprecated_methods(node)
@@ -139,6 +146,10 @@ def safe_to_time?(node)
           end
         end
 
+        def allow_to_time?
+          cop_config.fetch('AllowToTime', true)
+        end
+
         def good_days
           style == :strict ? [] : %i[current yesterday tomorrow]
         end

diff --git a/spec/rubocop/cop/rails/date_spec.rb b/spec/rubocop/cop/rails/date_spec.rb
@@ -2,7 +2,8 @@
 
 RSpec.describe RuboCop::Cop::Rails::Date, :config do
   context 'when EnforcedStyle is "strict"' do
-    let(:cop_config) { { 'EnforcedStyle' => 'strict' } }
+    let(:cop_config) { { 'EnforcedStyle' => 'strict', 'AllowToTime' => allow_to_time } }
+    let(:allow_to_time) { true }
 
     shared_examples 'offense' do |method, message|
       it "registers an offense for #{method}" do
@@ -47,25 +48,29 @@
       end
     end
 
-    %w[to_time to_time_in_current_zone].each do |method|
-      it "registers an offense for ##{method}" do
-        offenses = inspect_source("date.#{method}")
-        expect(offenses.size).to eq(1)
+    context 'when using `to_time_in_current_zone`' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          date.to_time_in_current_zone
+               ^^^^^^^^^^^^^^^^^^^^^^^ `to_time_in_current_zone` is deprecated. Use `in_time_zone` instead.
+        RUBY
       end
 
       context 'when using safe navigation operator' do
-        it "registers an offense for ##{method}" do
-          offenses = inspect_source("date&.#{method}")
-          expect(offenses.size).to eq(1)
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            date&.to_time_in_current_zone
+                  ^^^^^^^^^^^^^^^^^^^^^^^ `to_time_in_current_zone` is deprecated. Use `in_time_zone` instead.
+          RUBY
         end
       end
 
-      it "accepts variable named #{method}" do
-        expect_no_offenses("#{method} = 1")
+      it 'accepts variable named `to_time_in_current_zone`' do
+        expect_no_offenses('to_time_in_current_zone = 1')
       end
 
-      it "accepts variable #{method} as range end" do
-        expect_no_offenses("date..#{method}")
+      it 'accepts variable `to_time_in_current_zone` as range end' do
+        expect_no_offenses('date..to_time_in_current_zone')
       end
     end
 
@@ -81,12 +86,91 @@
       end
     end
 
-    context 'when a string literal without timezone' do
-      it 'registers an offense' do
-        expect_offense(<<~RUBY)
-          "2016-07-12 14:36:31".to_time(:utc)
-                                ^^^^^^^ Do not use `to_time` on Date objects, because they know nothing about the time zone in use.
-        RUBY
+    context 'when using `to_time`' do
+      context 'when `AllowToTime: true`' do
+        let(:allow_to_time) { true }
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            date.to_time
+          RUBY
+        end
+
+        context 'when using safe navigation operator' do
+          it 'does not register an offense' do
+            expect_no_offenses(<<~RUBY)
+              date&.to_time
+            RUBY
+          end
+        end
+
+        it 'accepts variable named `to_time`' do
+          expect_no_offenses('to_time = 1')
+        end
+
+        it 'accepts variable `to_time` as range end' do
+          expect_no_offenses('date..to_time')
+        end
+
+        context 'when a string literal without timezone' do
+          it 'does not register an offense' do
+            expect_no_offenses(<<~RUBY)
+              "2016-07-12 14:36:31".to_time(:utc)
+            RUBY
+          end
+        end
+
+        RuboCop::Cop::Rails::TimeZone::ACCEPTED_METHODS.each do |a_method|
+          it "does not register an offense for val.to_time.#{a_method}" do
+            expect_no_offenses(<<~RUBY)
+              val.to_time.#{a_method}
+            RUBY
+          end
+        end
+      end
+
+      context 'when `AllowToTime: false`' do
+        let(:allow_to_time) { false }
+
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            date.to_time
+                 ^^^^^^^ Do not use `to_time` on Date objects, because they know nothing about the time zone in use.
+          RUBY
+        end
+
+        context 'when using safe navigation operator' do
+          it 'registers an offense' do
+            expect_offense(<<~RUBY)
+              date&.to_time
+                    ^^^^^^^ Do not use `to_time` on Date objects, because they know nothing about the time zone in use.
+            RUBY
+          end
+        end
+
+        it 'accepts variable named `to_time`' do
+          expect_no_offenses('to_time = 1')
+        end
+
+        it 'accepts variable `to_time` as range end' do
+          expect_no_offenses('date..to_time')
+        end
+
+        context 'when a string literal without timezone' do
+          it 'registers an offense' do
+            expect_offense(<<~RUBY)
+              "2016-07-12 14:36:31".to_time(:utc)
+                                    ^^^^^^^ Do not use `to_time` on Date objects, because they know nothing about the time zone in use.
+            RUBY
+          end
+        end
+
+        RuboCop::Cop::Rails::TimeZone::ACCEPTED_METHODS.each do |a_method|
+          it "registers an offense for val.to_time.#{a_method}" do
+            offenses = inspect_source("val.to_time.#{a_method}")
+            expect(offenses.size).to eq(1)
+          end
+        end
       end
     end
 
@@ -100,13 +184,6 @@
       expect_no_offenses('A')
     end
 
-    RuboCop::Cop::Rails::TimeZone::ACCEPTED_METHODS.each do |a_method|
-      it "registers an offense for val.to_time.#{a_method}" do
-        offenses = inspect_source("val.to_time.#{a_method}")
-        expect(offenses.size).to eq(1)
-      end
-    end
-
     it 'registers an offense for #to_time_in_current_zone' do
       expect_offense(<<~RUBY)
         "2016-07-12 14:36:31".to_time_in_current_zone