-
Notifications
You must be signed in to change notification settings - Fork 858
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
NativeJavaClass: Add ".class" to access the associated Java Class #1679
base: master
Are you sure you want to change the base?
Conversation
Looks like at least one of the testcases you added fails |
aaf0ccf
to
0ceaea0
Compare
I think that should be fixed now. I guess it was running it multiple times and so the static count variable was higher than expected |
The code looks OK and I see the issue, but will people understand what we changed here? I think that at the very least the PR or commit should describe what we did and what people can do now -- since I'm not sure where the LiveConnect "spec" or docs are, I'm not sure how people will understand what is or isn't possible unless we update our docs. |
Renamed the PR and added a more detailed description on what was changed. Not sure where else to put this (other than the commit description). |
IMO, this is basically adding a new feature to LiveConnect, which looks useful to me, but I could use some more opinions. What do regular users of the Java interop think about this? |
Did you see the discussion on #757? This makes sense to me, to simplify access to the class |
Hello, we use LC a lot. But until now I can't remember that we were missing this feature. If a Java method expected a class as parameter, you could just call Nevertheless, I can imagine use cases where it would be necessary.
So, I would have nothing against simply replacing BTW: It was added 17 years ago with this commit d623313 and then changed from |
Yeah the main reason I see it being useful is for reflection. |
This seems useful but given that so much of this stuff isn't documented we'll I'm kind of waiting to see if we have more opinions. I am trying to resurrect some of the old docs, once I do that perhaps you can all take a look and see if that's a good place to update. |
@gbrail which old docs are you referring to? Thought we did resurrect all (relevant) docs from the Mozilla website into rhino.github.io already |
First time making tests so let me know if there is some case that isn't covered or whatever else I should change.
This PR adds support for class literal syntax for NativeJavaClasses. Before this, an example would be trying to get the
Class
associated withjava.lang.Object
, where the user would have to door
This makes it so that
works as well, just like it looks like in Java. The work around I had to add was this, due to how Rhino automatically creates bean properties associated with the getters and setters of the same name. If there is a class that has a static setClass() method, a
class
field is created, which would conflict with the goal of this PR. Instead, we just ignore that case if it exists, since there can't be a staticclass
getter due togetClass()
being defined as an instance method insideObject
, so there is no staticclass
getter associated to the bean property.Closes #757