This project is archived and is in readonly mode.

#6062 open
Jeremy Evans

ActiveSupport assumes too much about how Date/DateTime are implemented, breaking some usage with home_run

Reported by Jeremy Evans | November 25th, 2010 @ 07:22 AM

ActiveSupport assumes that DateTime inherits certain methods from Date, such as -. This is true in the standard library (where dates are stored as datetimes), but not true in home_run. I think ActiveSupport should be changed so that any methods that are overridden in Date should not be assumed to be automatically overridden in DateTime, so that you are sure that DateTime objects have the same behavior. I believe that ActiveSupport overrides the following methods in Date and assumes they will be overridden in DateTime: +, -, >>.

Removing the methods from DateTime or defining them to call super will not work, because the behavior for DateTime could be different from Date (and is in home_run). However, just adding the following to the DateTime core extension should work:

class DateTime
  alias_method :plus_without_duration, :+
  alias_method :+, :plus_with_duration
  alias_method :minus_without_duration, :-
  alias_method :-, :minus_with_duration
end

For >>, you should probably do something similar, like defining a new method and aliasing instead of undefing, even if you don't call the previous method. That's less of an issue, though, as you aren't changing behavior. You aren't going to effect home_run's DateTime class, but as it doesn't suffer from the bug you are trying to fix, it doesn't matter much.

See https://gist.github.com/714891 for a gist of the problem. See https://github.com/jeremyevans/home_run/issues/issue/20 for the issue reported on home_run's bugtracker.

Comments and changes to this ticket

  • Jeremy Evans

    Jeremy Evans November 25th, 2010 @ 05:43 PM

    • Tag changed from date datetime home_run core_ext active_support to active_support, core_ext, date, datetime, home_run

    I'm attaching a tested patch that fixes DateTime#+ and DateTime#- to work both with and without home_run, including some new tests for it.

  • rails

    rails February 26th, 2011 @ 12:00 AM

    • State changed from “new” to “open”

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • rails

    rails February 26th, 2011 @ 12:00 AM

    • State changed from “open” to “stale”
  • Jeremy Evans

    Jeremy Evans February 26th, 2011 @ 02:48 AM

    • State changed from “stale” to “open”

    [state:open] This still hasn't been fixed, and the patch probably still applies. While it currently only affects people that use home_run, as soon as tadf (ruby core member) merges his switch_hitter patch (replacing the standard date/datetime library with a C extension, see http://redmine.ruby-lang.org/issues/show/4257), this is going to start breaking on ruby-head, so you are going to have to fix it sooner or later.

  • Jeremy Evans

    Jeremy Evans March 29th, 2011 @ 07:45 PM

    • Tag changed from active_support, core_ext, date, datetime, home_run to active_support, core_ext, date, datetime, home_run, ruby-head

    This currently affects ruby-head even without home_run, since tadf merged his switch_hitter patch:

    $ gem list | grep active
    activesupport (3.0.5)
    $ ruby -v -r date -ractive_support/all -e 'p(DateTime::parse("January 1 1943") + 1.year)'
    ruby 1.9.3dev (2011-02-28 trunk 30975) [x86_64-openbsd4.9]
    /home/jeremy/.rvm/rubies/ruby-head/lib/ruby/1.9.1/date.rb:1358:in `plus_r': expected numeric (TypeError)
            from -e:1:in `+'
            from -e:1:in `<main>'
    
  • Jeremy Evans

    Jeremy Evans March 29th, 2011 @ 07:52 PM

    Looks like tenderlove fixed this earlier this month (https://github.com/rails/rails/commit/33f222b9e1b20aa6264084ec2c8c3..., but didn't merge it into the stable branch. Could this be merged to 3-0-stable so it makes 3.0.6?

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Pages