Skip to content
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

Java::JavaPackage does not respond_to ancestors, instance_methods #5784

Open
jonasoberschweiber opened this issue Jul 6, 2019 · 7 comments

Comments

Projects
None yet
4 participants
@jonasoberschweiber
Copy link

commented Jul 6, 2019

Environment

JRuby version is jruby 9.2.7.0 (2.5.3) 2019-04-09 8a269e3 Java HotSpot(TM) 64-Bit Server VM 25.121-b13 on 1.8.0_121-b13 +jit [darwin-x86_64]
Operating System is Darwin jobembp.local 18.6.0 Darwin Kernel Version 18.6.0: Thu Apr 25 23:16:27 PDT 2019; root:xnu-4903.261.4~2/RELEASE_X86_64 x86_64

Expected Behavior

Since Java::JavaPackage is a Module:

irb(main):001:0> Java::JavaPackage.is_a?(Module)
=> true

I'd expect it to respond to constants and instance_methods.

Actual Behavior

Java::JavaPackage does not respond to constants and instance_methods:

irb(main):002:0> Java::JavaPackage.instance_methods
Traceback (most recent call last):
        6: from /Users/jobe/.rbenv/versions/jruby-9.2.7.0/bin/irb:13:in `<main>'
        5: from org/jruby/RubyKernel.java:1193:in `catch'
        4: from org/jruby/RubyKernel.java:1193:in `catch'
        3: from org/jruby/RubyKernel.java:1425:in `loop'
        2: from org/jruby/RubyKernel.java:1061:in `eval'
        1: from (irb):2:in `evaluate'
NoMethodError (undefined method `instance_methods' for Java::JavaPackage:Module)
irb(main):003:0> Java::JavaPackage.constants
Traceback (most recent call last):
        6: from /Users/jobe/.rbenv/versions/jruby-9.2.7.0/bin/irb:13:in `<main>'
        5: from org/jruby/RubyKernel.java:1193:in `catch'
        4: from org/jruby/RubyKernel.java:1193:in `catch'
        3: from org/jruby/RubyKernel.java:1425:in `loop'
        2: from org/jruby/RubyKernel.java:1061:in `eval'
        1: from (irb):3:in `evaluate'
NoMethodError (undefined method `constants' for Java::JavaPackage:Module)

I've hit this behaviour while trying to make Sorbet work with JRuby.

@enebo

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

@kares is it possible we could change our package instances to not be modules but instances of BasicObject or something like that?

@jonasoberschweiber @kares @headius I feel like if we add 'constants' back as a method we will for sure end up with bug reports on how people cannot access classes with that package name (e.g. com.foo.constants.NetworkConstants). The workaround is to not use this form (ala magic constants or stringified classes [not positive that works or not actually]) but I am sure a report will come in if we add this back. I would prefer us to make basic reflective methods work but I also want to balance that with getting obscure errors when trying to use JI. Maybe switching package to instances of BasicObject or wiped singletons is a way of having our cake and eating it too...That may be a pretty visible design change though....

@enebo

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

An additional note on this is that while I think it is reasonable to try and accommodate this I do feel libraries which introspect the universe of any code in Ruby cannot just blindly assume these methods will exist. If it could assume that methods were available then Ruby would not give the ability to remove them as methods. Whether we fix this or not I feel sorbet should probably be a bit more defensive here (and I know how sucky that is but we are not the only people removing methods from Modules in Ruby for reasons).

@kares

This comment has been minimized.

Copy link
Member

commented Jul 8, 2019

I am mostly +1 for the BasicObject experiment (in 9.3).
also agree with Tom on 'constants' being problematic and a no-go zone for JI.

instance_methods and friends could be added in 9.2 if it plays well with the rest of JavaPackage.
@jonasoberschweiber would it help Sorbet (wout having constants method) ?

@kares

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

believe for instance_methods not being on the list is due being compact.
if we add it we likely need to add all of these:
[:public_instance_methods, :instance_methods, :private_instance_methods, :protected_instance_methods, :protected_methods, :singleton_methods, :private_methods, :methods, :public_methods]

here methods might be slightly problematic (less than constants but still), so we would need to skip that

@kares

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

btw. somehow I also like that java packages are modules - kind of makes sense except for the dot magic.
having it be a BasicObject (instance) seems less intuitive but than maybe I am biased here ...

also an ugly (and unofficial work-around) for constants :

Module.class_eval { alias constants __constants__ }
# JavaPackage has __constants__ hooked so we can than use it for all modules
all_modules.each { |mod| mod.__constants__ ... }
@headius

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

I'm also +1 for making package modules just be BasicObject derivatives. I believe we had a reason for not making this change, but the last time this came up was probably around the time BasicObject was added in 1.9, and I don't think we've seriously revisited it since then.

This would also allow us to remove some hacky workarounds, like in our ObjectSpace.each_object logic when it stumbles over these weird package modules. I'm sure there's other places, plus the extra Java plumbing we attach to them that could mostly go away in favor of normalish const_missing and method_missing hooks.

@headius

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

One problem with making it a BasicObject would be breaking the :: format for accessing classes:

[] ~/projects/jruby $ jruby -e 'p java::lang::System'
Java::JavaLang::System

[] ~/projects/jruby $ jruby -e 'o = BasicObject.new; p o::System'
TypeError: org.jruby.RubyBasicObject@536aaa8d is not a type/class
  <main> at -e:1

(Also note the kinda goofy error message)

This is important because you can't reopen a class using the dot notation:

[] ~/projects/jruby $ jruby -e 'class java::lang::System; def foo; end; p instance_methods(false); end'
[:==, :"__jcreate!", :equals, :foo]

[] ~/projects/jruby $ jruby -e 'class java.lang.System; def foo; end; p instance_methods(false); end'
SyntaxError: -e:1: syntax error, unexpected ';'
class java.lang.System; def foo; end; p instance_methods(false); end

This may be the reason we've left them modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.