From b06298df91d2bb38f478c1baf38f73fbc2770b6d Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 11 May 2021 00:29:47 +0900 Subject: [PATCH] [Fix #288] Add `AllowToTime` option to `Rails/Date` Fixes #288. This PR adds `AllowToTime` option (`true` by default) to `Rails/Date`. --- CHANGELOG.md | 4 + config/default.yml | 3 +- docs/modules/ROOT/pages/cops_rails.adoc | 32 ++++-- lib/rubocop/cop/rails/date.rb | 23 +++-- spec/rubocop/cop/rails/date_spec.rb | 127 +++++++++++++++++++----- 5 files changed, 151 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ed226d737..13854deefd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ * [#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 index 82881fcdb4..bff32fa070 100644 --- 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 index 066cb6172c..92fb801408 100644 --- 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 index 79b074e78a..decc517d5d 100644 --- 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 index 5dae8af7ae..77fc7fe12c 100644 --- 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