[Fix #67] Fix an incorrect auto-correct for Rails/TimeZone

Fixes #67.
Closes #71.

This PR fixes an incorrect auto-correct for `Rails/TimeZone`
when using `DateTime`.

```diff
- DateTime.new
+ DateTime.zone.new
```

```ruby
DateTime.zone.new # NoMethodError: undefined method `zone' for
DateTime:Class
```

This PR changes to ignore `DateTime` with the following as
background.

`DateTime` is not mentioned in The Rails Style Guide and
`Rails/TimeZone` cop's examples.

- https://rails.rubystyle.guide/#time-now
- https://docs.rubocop.org/projects/rails/en/stable/cops_rails/#railstimezone

Recently, #81 and #82 have been feedback.
I think breaking by auto-correction should be fixed with
the highest priority in this case.

Also, `DateTime` and `Time` are not replaced from `DateTime` to `Time`
by auto-correct because they are different objects.

Author: koic

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Bug fixes
+
+* [#67](https://github.com/rubocop-hq/rubocop-rails/issues/67): Fix an incorrect auto-correct for `Rails/TimeZone` when using `DateTime`. ([@koic][])
+
 ## 2.1.0 (2019-06-26)
 
 ### Bug fixes

lib/rubocop/cop/rails/time_zone.rb

@@ -55,8 +55,6 @@ class TimeZone < Cop
         MSG_LOCALTIME = 'Do not use `Time.localtime` without ' \
                         'offset or zone.'
 
-        TIMECLASSES = %i[Time DateTime].freeze
-
         GOOD_METHODS = %i[zone zone_default find_zone find_zone!].freeze
 
         DANGEROUS_METHODS = %i[now local new parse at current].freeze
@@ -67,10 +65,10 @@ class TimeZone < Cop
         def on_const(node)
           mod, klass = *node
           # we should only check core classes
-          # (`DateTime`, `Time`, `::DateTime` or `::Time`)
+          # (`Time` or `::Time`)
           return unless (mod.nil? || mod.cbase_type?) && method_send?(node)
 
-          check_time_node(klass, node.parent) if TIMECLASSES.include?(klass)
+          check_time_node(klass, node.parent) if klass == :Time
         end
 
         def autocorrect(node)
@@ -153,7 +151,7 @@ def method_from_time_class?(node)
           if (receiver.is_a? RuboCop::AST::Node) && !receiver.cbase_type?
             method_from_time_class?(receiver)
           else
-            TIMECLASSES.include?(method_name)
+            method_name == :Time
           end
         end
 

spec/rubocop/cop/rails/time_zone_spec.rb

@@ -6,92 +6,90 @@
   context 'when EnforcedStyle is "strict"' do
     let(:cop_config) { { 'EnforcedStyle' => 'strict' } }
 
-    described_class::TIMECLASSES.each do |klass|
-      it "registers an offense for #{klass}.now" do
-        inspect_source("#{klass}.now")
-        expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.message).to include('`Time.zone.now`')
-      end
+    it 'registers an offense for Time.now' do
+      inspect_source('Time.now')
+      expect(cop.offenses.size).to eq(1)
+      expect(cop.offenses.first.message).to include('`Time.zone.now`')
+    end
 
-      it "registers an offense for #{klass}.current" do
-        inspect_source("#{klass}.current")
-        expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.message).to include('`Time.zone.now`')
-      end
+    it 'registers an offense for Time.current' do
+      inspect_source('Time.current')
+      expect(cop.offenses.size).to eq(1)
+      expect(cop.offenses.first.message).to include('`Time.zone.now`')
+    end
 
-      it "registers an offense for #{klass}.new without argument" do
-        inspect_source("#{klass}.new")
-        expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.message).to include('`Time.zone.now`')
-      end
+    it 'registers an offense for Time.new without argument' do
+      inspect_source('Time.new')
+      expect(cop.offenses.size).to eq(1)
+      expect(cop.offenses.first.message).to include('`Time.zone.now`')
+    end
 
-      it "registers an offense for #{klass}.new with argument" do
-        inspect_source("#{klass}.new(2012, 6, 10, 12, 00)")
-        expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.message).to include('`Time.zone.local`')
-      end
+    it 'registers an offense for Time.new with argument' do
+      inspect_source('Time.new(2012, 6, 10, 12, 00)')
+      expect(cop.offenses.size).to eq(1)
+      expect(cop.offenses.first.message).to include('`Time.zone.local`')
+    end
 
-      it "does not register an offense when a .new method is called
-        independently of the #{klass} class" do
-        expect_no_offenses(<<~RUBY)
-          Range.new(1, #{klass}.class.to_s)
-        RUBY
-      end
+    it 'does not register an offense when a .new method is called
+        independently of the Time class' do
+      expect_no_offenses(<<~RUBY)
+        Range.new(1, Time.class.to_s)
+      RUBY
+    end
 
-      it "does not register an offense for #{klass}.new with zone argument" do
-        expect_no_offenses(<<~RUBY)
-          #{klass}.new(1988, 3, 15, 3, 0, 0, '-05:00')
-        RUBY
-      end
+    it 'does not register an offense for Time.new with zone argument' do
+      expect_no_offenses(<<~RUBY)
+        Time.new(1988, 3, 15, 3, 0, 0, '-05:00')
+      RUBY
+    end
 
-      it "registers an offense for ::#{klass}.now" do
-        inspect_source("::#{klass}.now")
+    it 'registers an offense for ::Time.now' do
+      inspect_source('::Time.now')
+      expect(cop.offenses.size).to eq(1)
+    end
+
+    it 'accepts Some::Time.now' do
+      expect_no_offenses(<<~RUBY)
+        Some::Time.now(0).strftime('%H:%M')
+      RUBY
+    end
+
+    described_class::ACCEPTED_METHODS.each do |a_method|
+      it "registers an offense Time.now.#{a_method}" do
+        inspect_source("Time.now.#{a_method}")
         expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.first.message).to include('`Time.zone.now`')
       end
+    end
 
-      it "accepts Some::#{klass}.now" do
-        expect_no_offenses(<<~RUBY)
-          Some::#{klass}.now(0).strftime('%H:%M')
-        RUBY
+    context 'autocorrect' do
+      let(:cop_config) do
+        { 'AutoCorrect' => 'true', 'EnforcedStyle' => 'strict' }
       end
 
-      described_class::ACCEPTED_METHODS.each do |a_method|
-        it "registers an offense #{klass}.now.#{a_method}" do
-          inspect_source("#{klass}.now.#{a_method}")
-          expect(cop.offenses.size).to eq(1)
-          expect(cop.offenses.first.message).to include('`Time.zone.now`')
-        end
+      it 'autocorrects correctly' do
+        source = 'Time.now.in_time_zone'
+        new_source = autocorrect_source(source)
+        expect(new_source).to eq('Time.zone.now')
       end
 
-      context 'autocorrect' do
-        let(:cop_config) do
-          { 'AutoCorrect' => 'true', 'EnforcedStyle' => 'strict' }
-        end
-
-        it 'autocorrects correctly' do
-          source = "#{klass}.now.in_time_zone"
+      # :current is a special case and is treated separately below
+      (described_class::DANGEROUS_METHODS - [:current]).each do |a_method|
+        it 'corrects the error' do
+          source = <<~RUBY
+            Time.#{a_method}
+          RUBY
           new_source = autocorrect_source(source)
-          expect(new_source).to eq('Time.zone.now')
-        end
-
-        # :current is a special case and is treated separately below
-        (described_class::DANGEROUS_METHODS - [:current]).each do |a_method|
-          it 'corrects the error' do
-            source = <<~RUBY
-              #{klass}.#{a_method}
-            RUBY
-            new_source = autocorrect_source(source)
-            expect(new_source).to eq(<<~RUBY)
-              Time.zone.#{a_method}
-            RUBY
-          end
+          expect(new_source).to eq(<<~RUBY)
+            Time.zone.#{a_method}
+          RUBY
         end
+      end
 
-        describe '.current' do
-          it 'corrects the error' do
-            new_source = autocorrect_source("#{klass}.current")
-            expect(new_source).to eq('Time.zone.now')
-          end
+      describe '.current' do
+        it 'corrects the error' do
+          new_source = autocorrect_source('Time.current')
+          expect(new_source).to eq('Time.zone.now')
         end
       end
     end
@@ -214,81 +212,79 @@
   context 'when EnforcedStyle is "flexible"' do
     let(:cop_config) { { 'EnforcedStyle' => 'flexible' } }
 
-    described_class::TIMECLASSES.each do |klass|
-      it "registers an offense for #{klass}.now" do
-        inspect_source("#{klass}.now")
-        expect(cop.offenses.size).to eq(1)
+    it 'registers an offense for Time.now' do
+      inspect_source('Time.now')
+      expect(cop.offenses.size).to eq(1)
 
-        expect(cop.offenses.first.message).to include('Use one of')
-        expect(cop.offenses.first.message).to include('`Time.zone.now`')
-        expect(cop.offenses.first.message).to include("`#{klass}.current`")
+      expect(cop.offenses.first.message).to include('Use one of')
+      expect(cop.offenses.first.message).to include('`Time.zone.now`')
+      expect(cop.offenses.first.message).to include('`Time.current`')
 
-        described_class::ACCEPTED_METHODS.each do |a_method|
-          expect(cop.offenses.first.message)
-            .to include("#{klass}.now.#{a_method}")
-        end
+      described_class::ACCEPTED_METHODS.each do |a_method|
+        expect(cop.offenses.first.message)
+          .to include("Time.now.#{a_method}")
       end
+    end
+
+    it 'accepts Time.current' do
+      expect_no_offenses(<<~RUBY)
+        Time.current
+      RUBY
+    end
 
-      it "accepts #{klass}.current" do
+    described_class::ACCEPTED_METHODS.each do |a_method|
+      it "accepts Time.now.#{a_method}" do
         expect_no_offenses(<<~RUBY)
-          #{klass}.current
+          Time.now.#{a_method}
         RUBY
       end
+    end
 
-      described_class::ACCEPTED_METHODS.each do |a_method|
-        it "accepts #{klass}.now.#{a_method}" do
-          expect_no_offenses(<<~RUBY)
-            #{klass}.now.#{a_method}
-          RUBY
-        end
-      end
+    it 'accepts Time.zone.now' do
+      expect_no_offenses(<<~RUBY)
+        Time.zone.now
+      RUBY
+    end
 
-      it "accepts #{klass}.zone.now" do
-        expect_no_offenses(<<~RUBY)
-          #{klass}.zone.now
-        RUBY
-      end
+    it 'accepts Time.zone_default.now' do
+      expect_no_offenses(<<~RUBY)
+        Time.zone_default.now
+      RUBY
+    end
 
-      it "accepts #{klass}.zone_default.now" do
-        expect_no_offenses(<<~RUBY)
-          #{klass}.zone_default.now
-        RUBY
-      end
+    it 'accepts Time.find_zone(time_zone).now' do
+      expect_no_offenses(<<~RUBY)
+        Time.find_zone('EST').now
+      RUBY
+    end
 
-      it "accepts #{klass}.find_zone(time_zone).now" do
-        expect_no_offenses(<<~RUBY)
-          #{klass}.find_zone('EST').now
-        RUBY
-      end
+    it 'accepts Time.find_zone!(time_zone).now' do
+      expect_no_offenses(<<~RUBY)
+        Time.find_zone!('EST').now
+      RUBY
+    end
 
-      it "accepts #{klass}.find_zone!(time_zone).now" do
+    described_class::DANGEROUS_METHODS.each do |a_method|
+      it "accepts Time.current.#{a_method}" do
         expect_no_offenses(<<~RUBY)
-          #{klass}.find_zone!('EST').now
+          Time.current.#{a_method}
         RUBY
       end
 
-      described_class::DANGEROUS_METHODS.each do |a_method|
-        it "accepts #{klass}.current.#{a_method}" do
-          expect_no_offenses(<<~RUBY)
-            #{klass}.current.#{a_method}
-          RUBY
+      context 'autocorrect' do
+        let(:cop_config) do
+          { 'AutoCorrect' => 'true', 'EnforcedStyle' => 'flexible' }
         end
 
-        context 'autocorrect' do
-          let(:cop_config) do
-            { 'AutoCorrect' => 'true', 'EnforcedStyle' => 'flexible' }
-          end
-
-          it 'corrects the error' do
-            source = <<~RUBY
-              #{klass}.#{a_method}
+        it 'corrects the error' do
+          source = <<~RUBY
+            Time.#{a_method}
+          RUBY
+          new_source = autocorrect_source(source)
+          unless a_method == :current
+            expect(new_source).to eq(<<~RUBY)
+              Time.zone.#{a_method}
             RUBY
-            new_source = autocorrect_source(source)
-            unless a_method == :current
-              expect(new_source).to eq(<<~RUBY)
-                #{klass}.zone.#{a_method}
-              RUBY
-            end
           end
         end
       end