Deprecate implicit coercion of `ActiveSupport::Duration`

Currently `ActiveSupport::Duration` implicitly converts to a seconds
value when used in a calculation except for the explicit examples of
addition and subtraction where the duration is the receiver, e.g:

    >> 2 * 1.day
    => 172800

This results in lots of confusion especially when using durations
with dates because adding/subtracting a value from a date treats
integers as a day and not a second, e.g:

    >> Date.today
    => Wed, 01 Mar 2017
    >> Date.today + 2 * 1.day
    => Mon, 10 Apr 2490

To fix this we're implementing `coerce` so that we can provide a
deprecation warning with the intent of removing the implicit coercion
in Rails 5.2, e.g:

    >> 2 * 1.day
    DEPRECATION WARNING: Implicit coercion of ActiveSupport::Duration
    to a Numeric is deprecated and will raise a TypeError in Rails 5.2.
    => 172800

In Rails 5.2 it will raise `TypeError`, e.g:

    >> 2 * 1.day
    TypeError: ActiveSupport::Duration can't be coerced into Integer

This is the same behavior as with other types in Ruby, e.g:

    >> 2 * "foo"
    TypeError: String can't be coerced into Integer
    >> "foo" * 2
    => "foofoo"

As part of this deprecation add `*` and `/` methods to `AS::Duration`
so that calculations that keep the duration as the receiver work
correctly whether the final receiver is a `Date` or `Time`, e.g:

    >> Date.today
    => Wed, 01 Mar 2017
    >> Date.today + 1.day * 2
    => Fri, 03 Mar 2017

Fixes #27457.
This commit is contained in:
Andrew White 2017-02-27 18:31:35 +00:00
parent a815c7c721
commit 75924c4517
6 changed files with 149 additions and 8 deletions

View File

@ -76,7 +76,7 @@ class MessageDeliveryTest < ActiveSupport::TestCase
test "should enqueue a delivery with a delay" do
travel_to Time.new(2004, 11, 24, 01, 04, 44) do
assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current.to_f + 600.seconds, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do
assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current.to_f + 600, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do
@mail.deliver_later wait: 600.seconds
end
end

View File

@ -832,7 +832,7 @@ class DateHelperTest < ActionView::TestCase
def test_select_date_with_too_big_range_between_start_year_and_end_year
assert_raise(ArgumentError) { select_date(Time.mktime(2003, 8, 16), start_year: 2000, end_year: 20000, prefix: "date[first]", order: [:month, :day, :year]) }
assert_raise(ArgumentError) { select_date(Time.mktime(2003, 8, 16), start_year: Date.today.year - 100.years, end_year: 2000, prefix: "date[first]", order: [:month, :day, :year]) }
assert_raise(ArgumentError) { select_date(Time.mktime(2003, 8, 16), start_year: 100, end_year: 2000, prefix: "date[first]", order: [:month, :day, :year]) }
end
def test_select_date_can_have_more_then_1000_years_interval_if_forced_via_parameter

View File

@ -1,3 +1,55 @@
* Deprecate implicit coercion of `ActiveSupport::Duration`
Currently `ActiveSupport::Duration` implicitly converts to a seconds
value when used in a calculation except for the explicit examples of
addition and subtraction where the duration is the receiver, e.g:
>> 2 * 1.day
=> 172800
This results in lots of confusion especially when using durations
with dates because adding/subtracting a value from a date treats
integers as a day and not a second, e.g:
>> Date.today
=> Wed, 01 Mar 2017
>> Date.today + 2 * 1.day
=> Mon, 10 Apr 2490
To fix this we're implementing `coerce` so that we can provide a
deprecation warning with the intent of removing the implicit coercion
in Rails 5.2, e.g:
>> 2 * 1.day
DEPRECATION WARNING: Implicit coercion of ActiveSupport::Duration
to a Numeric is deprecated and will raise a TypeError in Rails 5.2.
=> 172800
In Rails 5.2 it will raise `TypeError`, e.g:
>> 2 * 1.day
TypeError: ActiveSupport::Duration can't be coerced into Integer
This is the same behavior as with other types in Ruby, e.g:
>> 2 * "foo"
TypeError: String can't be coerced into Integer
>> "foo" * 2
=> "foofoo"
As part of this deprecation add `*` and `/` methods to `AS::Duration`
so that calculations that keep the duration as the receiver work
correctly whether the final receiver is a `Date` or `Time`, e.g:
>> Date.today
=> Wed, 01 Mar 2017
>> Date.today + 1.day * 2
=> Fri, 03 Mar 2017
Fixes #27457.
*Andrew White*
* Update `DateTime#change` to support `:usec` and `:nsec` options.
Adding support for these options now allows us to update the `DateTime#end_of`

View File

@ -156,7 +156,7 @@ module ActiveSupport
expires_in = options[:expires_in].to_i
if expires_in > 0 && !options[:raw]
# Set the memcache expire a few minutes in the future to support race condition ttls on read
expires_in += 5.minutes
expires_in += 300
end
rescue_error_with false do
@data.send(method, key, value, expires_in, options)

View File

@ -1,5 +1,7 @@
require "active_support/core_ext/array/conversions"
require "active_support/core_ext/object/acts_like"
require "active_support/core_ext/string/filters"
require "active_support/deprecation"
module ActiveSupport
# Provides accurate date and time measurements using Date#advance and
@ -88,6 +90,25 @@ module ActiveSupport
@parts.default = 0
end
def coerce(other) #:nodoc:
ActiveSupport::Deprecation.warn(<<-MSG.squish)
Implicit coercion of ActiveSupport::Duration to a Numeric
is deprecated and will raise a TypeError in Rails 5.2.
MSG
[other, value]
end
# Compares one Duration with another or a Numeric to this Duration.
# Numeric values are treated as seconds.
def <=>(other)
if Duration === other
value <=> other.value
elsif Numeric === other
value <=> other
end
end
# Adds another Duration or a Numeric to this Duration. Numeric values
# are treated as seconds.
def +(other)
@ -109,6 +130,24 @@ module ActiveSupport
self + (-other)
end
# Multiplies this Duration by a Numeric and returns a new Duration.
def *(other)
if Numeric === other
Duration.new(value * other, parts.map { |type, number| [type, number * other] })
else
value * other
end
end
# Devides this Duration by a Numeric and returns a new Duration.
def /(other)
if Numeric === other
Duration.new(value / other, parts.map { |type, number| [type, number / other] })
else
value / other
end
end
def -@ #:nodoc:
Duration.new(-value, parts.map { |type, number| [type, -number] })
end
@ -212,8 +251,6 @@ module ActiveSupport
ISO8601Serializer.new(self, precision: precision).serialize
end
delegate :<=>, to: :value
private
def sum(sign, time = ::Time.current)

View File

@ -84,8 +84,46 @@ class DurationTest < ActiveSupport::TestCase
assert_nothing_raised { Date.today - Date.today }
end
def test_plus
assert_equal 2.seconds, 1.second + 1.second
assert_instance_of ActiveSupport::Duration, 1.second + 1.second
assert_equal 2.seconds, 1.second + 1
assert_instance_of ActiveSupport::Duration, 1.second + 1
end
def test_minus
assert_equal 1.second, 2.seconds - 1.second
assert_instance_of ActiveSupport::Duration, 2.seconds - 1.second
assert_equal 1.second, 2.seconds - 1
assert_instance_of ActiveSupport::Duration, 2.seconds - 1
end
def test_multiply
assert_equal 7.days, 1.day * 7
assert_instance_of ActiveSupport::Duration, 1.day * 7
assert_deprecated do
assert_equal 86400, 1.day * 1.second
end
end
def test_divide
assert_equal 1.day, 7.days / 7
assert_instance_of ActiveSupport::Duration, 7.days / 7
assert_deprecated do
assert_equal 1, 1.day / 1.day
end
end
def test_date_added_with_multiplied_duration
assert_equal Date.civil(2017, 1, 3), Date.civil(2017, 1, 1) + 1.day * 2
end
def test_plus_with_time
assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration"
assert_deprecated do
assert_equal 1 + 1.second, 1.second + 1, "Duration + Numeric should == Numeric + Duration"
end
end
def test_time_plus_duration_returns_same_time_datatype
@ -104,6 +142,13 @@ class DurationTest < ActiveSupport::TestCase
assert_equal 'expected a time or date, got ""', e.message, "ensure ArgumentError is not being raised by dependencies.rb"
end
def test_implicit_coercion_is_deprecated
assert_deprecated { 1 + 1.second }
assert_deprecated { 1 - 1.second }
assert_deprecated { 1 * 1.second }
assert_deprecated { 1 / 1.second }
end
def test_fractional_weeks
assert_equal((86400 * 7) * 1.5, 1.5.weeks)
assert_equal((86400 * 7) * 1.7, 1.7.weeks)
@ -241,13 +286,20 @@ class DurationTest < ActiveSupport::TestCase
def test_comparable
assert_equal(-1, (0.seconds <=> 1.second))
assert_equal(-1, (1.second <=> 1.minute))
assert_equal(-1, (1 <=> 1.minute))
assert_deprecated do
assert_equal(-1, (1 <=> 1.minute))
end
assert_equal(0, (0.seconds <=> 0.seconds))
assert_equal(0, (0.seconds <=> 0.minutes))
assert_equal(0, (1.second <=> 1.second))
assert_equal(1, (1.second <=> 0.second))
assert_equal(1, (1.minute <=> 1.second))
assert_equal(1, (61 <=> 1.minute))
assert_deprecated do
assert_equal(1, (61 <=> 1.minute))
end
end
def test_twelve_months_equals_one_year