-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Respond_to? returns private methods #4253
Comments
thanks, would be better to give it a go on JRuby 9.1.5.0 instead of 9.0.5.0 (that or a reduced test-case) |
9.1.5.0 has the same issue:
cc @headius |
cc @enebo |
@kirs this is fixed in 9.1.6.0. Can you confirm that? |
I confirm that ❤️ I think the issue can be closed. |
Ooops, I was wrong:
# post.rb
class Post
protected
def metadata
end
end
puts Post.new.respond_to?(:metadata).inspect
|
@kirs interestingly the bug we fixed in 9.1.6.0 was the exact opposite problem that we were not seeing true when we should have. Thanks for checking. |
Maybe you fixed it for private methods, but not for protected? On Wed, Nov 16, 2016 at 9:30 AM, Thomas E Enebo notifications@github.com
|
@kirs the fix was for protected methods and involved us not checking beneath a particular point due to the use of prepended modules. We were getting false back and not true due to us not searching past the prepended module. I read this issue backwards. |
Thanks for the background! In my case above there're no prepended modules involved. |
@enebo this is still an issue on master. Do you have any suggestion how I can fix it? It currently blocks us on the Rails side. |
@kirs I will see if I can get any insight into this. One weird thing is respond_to? even in a method of the same type returns true while mri will return false. So post will really respond to being sent metadata but the respond_to? method will still say false? |
@enebo not exactly. As a result, class Post
protected
def metadata
end
end
post = Post.new
if post.respond_to?(:metadata)
post.metadata
else
puts "no metadata available"
end
While on MRI that works as expected:
|
@kirs I think I have this fixed locally. Running tests now. If appears for dispatch we only care about private visibility but for respond_to? we care about both private and protected. It is a lack of symmetry. |
Awesome. I'm looking forward for updates from you. |
@kirs kick the tires on this one. I did actually find some tagged out specs on this and I feel fairly confident about the fix but nonetheless this is pretty low-level language mojo :) |
Thanks! ❤️ |
From Twitter: https://twitter.com/headius/status/788857979655323648
Environment
jruby 9.0.5.0 (2.2.3) 2016-01-26 7bee00d Java HotSpot(TM) 64-Bit Server VM 25.101-b13 on 1.8.0_101-b13 +jit [darwin-x86_64]
The test case:
Expected Behavior
raise
ActiveModel::UnknownAttributeError
Actual Behavior
This there is a comment about this at https://github.com/jruby/jruby/blame/21b9760280f07fe91aa5a12f23334579b5744024/core/src/main/java/org/jruby/ext/ripper/RubyRipper.java#L361
The text was updated successfully, but these errors were encountered: