[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.

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