add implementation of respond_to? for ActiveSupport::Duration#16574
add implementation of respond_to? for ActiveSupport::Duration#16574jeremy merged 1 commit intorails:masterfrom
Conversation
lsylvester
force-pushed
the
duration-respond_to
branch
from
299f668 to
428a569
Compare
There was a problem hiding this comment.
Can this just be super || @value.respond_to?(method, include_private)
There was a problem hiding this comment.
Because Duration is a subclass of ProxyObject it doesn't have a respond_to? method in its superclass, so calling super goes straight to method_missing.
There was a problem hiding this comment.
>> class O < BasicObject; HAX = ::Object.method(:respond_to?).unbind.bind(self); def respond_to?(*args) HAX.call(*args) end end
=> :respond_to?
>> O.new.respond_to? :a
=> falseReimplementing Ruby's respond_to? + respond_to_missing? protocol will lead to breakage down the line.
Is there a better way?
There was a problem hiding this comment.
@jeremy I think implementation of respond_to? returns true based on if the class has the methods defined instead of the the instance.
I don't think that it is possible to bind a method to a ProxyObject from Object as I get an error when I try to do so.
Object.instance_method(:respond_to?).bind(BasicObject.new)
#=> bind argument must be an instance of Object (TypeError)Maybe Duration should be a subclass of Object so it can get the correct respond_to? behaviour for free.
The only issue identified by the tests when Durations superclass is changed to Object is that it can now be duplicated, which can be fixed by defining duplicable? and undefining dup.
Do you know any reasons why Durations superclass shouldn't be changed to Object.
There was a problem hiding this comment.
@lsylvester See the code snippet above 😁 You have to #unbind the Method first, then #bind the UnboundMethod to the class.
Agree that Duration should simply be an Object. It's not really a transparent proxy anymore.
There was a problem hiding this comment.
See #11794 for another case where being an Object would help. We don't have hash-equality with #eql? currently.
|
Need it strongly in release of 4.2. Thanks! |
|
cc @pixeltrix @robin850 @jeremy can one of you take a look 😄 |
There was a problem hiding this comment.
Then let use assert_not_respond_to
|
Isn't this really a problem with the |
lsylvester
force-pushed
the
duration-respond_to
branch
2 times, most recently
from
443607f to
eb5bb05
Compare
lsylvester
force-pushed
the
duration-respond_to
branch
from
eb5bb05 to
06e20fa
Compare
|
I have changed this to make Duration a subclass of Object, so that it gets the correct I have made it non duplicatable, not sure if this is required or it should be removed or deprecated. Also, |
|
Nice @lsylvester - that feels a lot better! The object is duplicable, so that bit can go.
|
lsylvester
force-pushed
the
duration-respond_to
branch
2 times, most recently
from
b2b17da to
32f3115
Compare
lsylvester
force-pushed
the
duration-respond_to
branch
from
32f3115 to
f976751
Compare
|
The duplicatable bit is gone. |
add implementation of respond_to? for ActiveSupport::Duration
|
Thank you @lsylvester ! |
- Reference : rails#17493 (comment) - Duration stopped inheriting from ProxyObject in rails#16574
8 participants
In rails 4.2.beta, calling respond_to? on a ActiveSupport::Duration no longer returns true for the methods defined in ActiveSupport::Duration. ie
This is because the
since,agoand similar methods were removed from Numeric.This adds a custom respond_to? method to currently return if the duration responds to these methods.