Merge pull request #52370 from jhawthorn/time_addition_deprecation

Combined time addition deprecation

activesupport/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Deprecate addition and since between two `Time` and `ActiveSupport::TimeWithZone`.
+
+    Previously adding time instances together such as `10.days.ago + 10.days.ago` or `10.days.ago.since(10.days.ago)` produced a nonsensical future date. This behavior is deprecated and will be removed in Rails 8.1.
+
+    *Nick Schwaderer*
+
 *   Support rfc2822 format for Time#to_fs & Date#to_fs.
 
     *Akshay Birajdar*

activesupport/lib/active_support/core_ext/time/calculations.rb

@@ -224,8 +224,13 @@ def ago(seconds)
   # Returns a new Time representing the time a number of seconds since the instance time
   def since(seconds)
     self + seconds
-  rescue
-    to_datetime.since(seconds)
+  rescue TypeError
+    result = to_datetime.since(seconds)
+    ActiveSupport.deprecator.warn(
+      "Passing an instance of #{seconds.class} to #{self.class}#since is deprecated. This behavior will raise " \
+      "a `TypeError` in Rails 8.1."
+    )
+    result
   end
   alias :in :since
 

activesupport/lib/active_support/time_with_zone.rb

@@ -300,7 +300,16 @@ def +(other)
       if duration_of_variable_length?(other)
         method_missing(:+, other)
       else
-        result = utc + other
+        begin
+          result = utc + other
+        rescue TypeError
+          result = utc.to_datetime.since(other)
+          ActiveSupport.deprecator.warn(
+            "Adding an instance of #{other.class} to an instance of #{self.class} is deprecated. This behavior will raise " \
+            "a `TypeError` in Rails 8.1."
+          )
+          result.in_time_zone(time_zone)
+        end
         result.in_time_zone(time_zone)
       end
     end

activesupport/test/core_ext/time_ext_test.rb

@@ -282,6 +282,12 @@ def test_daylight_savings_time_crossings_backward_end_1day
     end
   end
 
+  def test_since_with_instance_of_time_deprecated
+    assert_deprecated(ActiveSupport.deprecator) do
+      Time.now.since(Time.now)
+    end
+  end
+
   def test_since
     assert_equal Time.local(2005, 2, 22, 10, 10, 11), Time.local(2005, 2, 22, 10, 10, 10).since(1)
     assert_equal Time.local(2005, 2, 22, 11, 10, 10), Time.local(2005, 2, 22, 10, 10, 10).since(3600)

activesupport/test/core_ext/time_with_zone_test.rb

@@ -400,6 +400,22 @@ def test_no_limit_on_times
     assert_equal [0, 0, 19, 31, 12, -8001], (twz - 10_000.years).to_a[0, 6]
   end
 
+  def test_plus_two_time_instances_raises_deprecation_warning
+    twz = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1), @time_zone)
+    assert_deprecated(ActiveSupport.deprecator) do
+      twz + 10.days.ago
+    end
+  end
+
+  def test_plus_with_invalid_argument
+    twz = ActiveSupport::TimeWithZone.new(Time.utc(2000, 1, 1), @time_zone)
+    assert_not_deprecated(ActiveSupport.deprecator) do
+      assert_raises TypeError do
+        twz + Object.new
+      end
+    end
+  end
+
   def test_plus_with_duration
     assert_equal Time.utc(2000, 1, 5, 19, 0, 0), (@twz + 5.days).time
   end