Standardise the return value of `to_time`

This commit standardises the return value of `to_time` to an instance
of `Time` in the local system timezone, matching the Ruby core and
standard library behavior.

The default form for `String#to_time` has been changed from :utc to
:local but research seems to suggest the latter is the more common form.

Also fix an edge condition with `String#to_time` where the string has
a timezone offset in it and the mode is :local. e.g:

  # Before:
  >> "2000-01-01 00:00:00 -0500".to_time(:local)
  => 2000-01-01 05:00:00 -0500

  # After:
  >> "2000-01-01 00:00:00 -0500".to_time(:local)
  => 2000-01-01 00:00:00 -0500

Closes #2453

5 files changed, 63 insertions, 17 deletions

diff --git a/activesupport/test/core_ext/date_ext_test.rb b/activesupport/test/core_ext/date_ext_test.rb
index ec47d0632c..f3fa96ec6f 100644
--- a/activesupport/test/core_ext/date_ext_test.rb
+++ b/activesupport/test/core_ext/date_ext_test.rb
@@ -33,8 +33,12 @@ class DateExtCalculationsTest < ActiveSupport::TestCase
   end
 
   def test_to_time
-    assert_equal Time.local(2005, 2, 21), Date.new(2005, 2, 21).to_time
-    assert_equal Time.local(2039, 2, 21), Date.new(2039, 2, 21).to_time
+    with_env_tz 'US/Eastern' do
+      assert_equal Time, Date.new(2005, 2, 21).to_time.class
+      assert_equal Time.local(2005, 2, 21), Date.new(2005, 2, 21).to_time
+      assert_equal Time.local(2005, 2, 21).utc_offset, Date.new(2005, 2, 21).to_time.utc_offset
+    end
+
     silence_warnings do
       0.upto(138) do |year|
         [:utc, :local].each do |format|
diff --git a/activesupport/test/core_ext/date_time_ext_test.rb b/activesupport/test/core_ext/date_time_ext_test.rb
index 54bbdbb18f..f74b7b9917 100644
--- a/activesupport/test/core_ext/date_time_ext_test.rb
+++ b/activesupport/test/core_ext/date_time_ext_test.rb
@@ -41,11 +41,14 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase
   end
 
   def test_to_time
-    assert_equal Time.utc(2005, 2, 21, 10, 11, 12), DateTime.new(2005, 2, 21, 10, 11, 12, 0).to_time
-    assert_equal Time.utc(2039, 2, 21, 10, 11, 12), DateTime.new(2039, 2, 21, 10, 11, 12, 0).to_time
-    # DateTimes with offsets other than 0 are returned unaltered
-    assert_equal DateTime.new(2005, 2, 21, 10, 11, 12, Rational(-5, 24)), DateTime.new(2005, 2, 21, 10, 11, 12, Rational(-5, 24)).to_time
-    # Fractional seconds are preserved
+    with_env_tz 'US/Eastern' do
+      assert_equal Time, DateTime.new(2005, 2, 21, 10, 11, 12, 0).to_time.class
+      assert_equal Time.local(2005, 2, 21, 5, 11, 12), DateTime.new(2005, 2, 21, 10, 11, 12, 0).to_time
+      assert_equal Time.local(2005, 2, 21, 5, 11, 12).utc_offset, DateTime.new(2005, 2, 21, 10, 11, 12, 0).to_time.utc_offset
+    end
+  end
+
+  def test_to_time_preserves_fractional_seconds
     assert_equal Time.utc(2005, 2, 21, 10, 11, 12, 256), DateTime.new(2005, 2, 21, 10, 11, 12 + Rational(256, 1000000), 0).to_time
   end
 
diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
index e0ddeab548..951e6bac07 100644
--- a/activesupport/test/core_ext/string_ext_test.rb
+++ b/activesupport/test/core_ext/string_ext_test.rb
@@ -286,14 +286,37 @@ end
 
 class StringConversionsTest < ActiveSupport::TestCase
   def test_string_to_time
-    assert_equal Time.utc(2005, 2, 27, 23, 50), "2005-02-27 23:50".to_time
-    assert_equal Time.local(2005, 2, 27, 23, 50), "2005-02-27 23:50".to_time(:local)
-    assert_equal Time.utc(2005, 2, 27, 23, 50, 19, 275038), "2005-02-27T23:50:19.275038".to_time
-    assert_equal Time.local(2005, 2, 27, 23, 50, 19, 275038), "2005-02-27T23:50:19.275038".to_time(:local)
-    assert_equal DateTime.civil(2039, 2, 27, 23, 50), "2039-02-27 23:50".to_time
-    assert_equal Time.local(2039, 2, 27, 23, 50), "2039-02-27 23:50".to_time(:local)
-    assert_equal Time.utc(2011, 2, 27, 23, 50), "2011-02-27 22:50 -0100".to_time
-    assert_nil "".to_time
+    with_env_tz "US/Eastern" do
+      assert_equal Time.utc(2005, 2, 27, 23, 50), "2005-02-27 23:50".to_time(:utc)
+      assert_equal Time.local(2005, 2, 27, 23, 50), "2005-02-27 23:50".to_time
+      assert_equal Time.utc(2005, 2, 27, 23, 50, 19, 275038), "2005-02-27T23:50:19.275038".to_time(:utc)
+      assert_equal Time.local(2005, 2, 27, 23, 50, 19, 275038), "2005-02-27T23:50:19.275038".to_time
+      assert_equal Time.utc(2039, 2, 27, 23, 50), "2039-02-27 23:50".to_time(:utc)
+      assert_equal Time.local(2039, 2, 27, 23, 50), "2039-02-27 23:50".to_time
+      assert_equal Time.local(2011, 2, 27, 18, 50), "2011-02-27 22:50 -0100".to_time
+      assert_equal Time.utc(2011, 2, 27, 23, 50), "2011-02-27 22:50 -0100".to_time(:utc)
+      assert_equal Time.local(2005, 2, 27, 23, 50), "2005-02-27 23:50 -0500".to_time
+      assert_nil "".to_time
+    end
+  end
+
+  def test_string_to_time_utc_offset
+    with_env_tz "US/Eastern" do
+      assert_equal 0, "2005-02-27 23:50".to_time(:utc).utc_offset
+      assert_equal(-18000, "2005-02-27 23:50".to_time.utc_offset)
+      assert_equal 0, "2005-02-27 22:50 -0100".to_time(:utc).utc_offset
+      assert_equal(-18000, "2005-02-27 22:50 -0100".to_time.utc_offset)
+    end
+  end
+
+  def test_partial_string_to_time
+    with_env_tz "US/Eastern" do
+      now = Time.now
+      assert_equal Time.local(now.year, now.month, now.day, 23, 50), "23:50".to_time
+      assert_equal Time.utc(now.year, now.month, now.day, 23, 50), "23:50".to_time(:utc)
+      assert_equal Time.local(now.year, now.month, now.day, 18, 50), "22:50 -0100".to_time
+      assert_equal Time.utc(now.year, now.month, now.day, 23, 50), "22:50 -0100".to_time(:utc)
+    end
   end
 
   def test_string_to_datetime
@@ -309,6 +332,14 @@ class StringConversionsTest < ActiveSupport::TestCase
     assert_nil "".to_date
     assert_equal Date.new(Date.today.year, 2, 3), "Feb 3rd".to_date
   end
+
+  protected
+    def with_env_tz(new_tz = 'US/Eastern')
+      old_tz, ENV['TZ'] = ENV['TZ'], new_tz
+      yield
+    ensure
+      old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ')
+    end
 end
 
 class StringBehaviourTest < ActiveSupport::TestCase
diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb
index a2fefee3b8..43c92003dc 100644
--- a/activesupport/test/core_ext/time_ext_test.rb
+++ b/activesupport/test/core_ext/time_ext_test.rb
@@ -526,7 +526,11 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase
   end
 
   def test_to_time
-    assert_equal Time.local(2005, 2, 21, 17, 44, 30), Time.local(2005, 2, 21, 17, 44, 30).to_time
+    with_env_tz 'US/Eastern' do
+      assert_equal Time, Time.local(2005, 2, 21, 17, 44, 30).to_time.class
+      assert_equal Time.local(2005, 2, 21, 17, 44, 30), Time.local(2005, 2, 21, 17, 44, 30).to_time
+      assert_equal Time.local(2005, 2, 21, 17, 44, 30).utc_offset, Time.local(2005, 2, 21, 17, 44, 30).to_time.utc_offset
+    end
   end
 
   # NOTE: this test seems to fail (changeset 1958) only on certain platforms,
diff --git a/activesupport/test/core_ext/time_with_zone_test.rb b/activesupport/test/core_ext/time_with_zone_test.rb
index 6c773770f0..18eca4cd8b 100644
--- a/activesupport/test/core_ext/time_with_zone_test.rb
+++ b/activesupport/test/core_ext/time_with_zone_test.rb
@@ -327,7 +327,11 @@ class TimeWithZoneTest < ActiveSupport::TestCase
   end
 
   def test_to_time
-    assert_equal @twz, @twz.to_time
+    with_env_tz 'US/Eastern' do
+      assert_equal Time, @twz.to_time.class
+      assert_equal Time.local(1999, 12, 31, 19), @twz.to_time
+      assert_equal Time.local(1999, 12, 31, 19).utc_offset, @twz.to_time.utc_offset
+    end
   end
 
   def test_to_date