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

Method aliases are not consistently mapped for a class with isFoo and getFoo #3470

Closed
benissimo opened this Issue Nov 19, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@benissimo

benissimo commented Nov 19, 2015

For a class

public class Ugh {

  public boolean isFoo() {
     return false;
  }

  public int getFoo() {
     return 3;
  }
}

calling .foo from Jruby results in inconsistent behavior.

Sometimes it behaves this way:

jruby-1.7.22 :001 > Java::Ugh.new.foo
 => 3 

Other times (exit jruby and fire up irb a new time, this time using, say, java 8 instead of java 7) it behaves this way:

jruby-1.7.22 :001 > Java::Ugh.new.foo
 => false 

Per https://github.com/jruby/jruby/wiki/CallingJavaFromJRuby
'something' -> 'getSomething'
'something?' -> 'isSomething'

However in practice 'something' gets mapped to both 'getSomething' and 'isSomething' (assuming the java class has both methods) and, as the order those methods get mapped appears to be nondeterministic, there's no consistency as to which one gets mapped last (clobbering the first). At least that's my best guess as to what's causing the issue here.

We stumbled across this when upgrading to java 8, initially thought it was related to that, but ultimately found there to be no consistent behavior (different developers got different results running the above test).

We suspect that ./core/src/main/java/org/jruby/javasupport/JavaUtil.java is involved.

Suggestion: do not map 'foo' to 'isFoo'. Only map 'foo?' to 'isFoo' (as the wiki says).
Or: ensure java methods are sorted in some consistent order before mapping them.

@benissimo benissimo changed the title from Alias names are not consistent for a class with isFoo and getFoo to Method aliases are not consistently mapped for a class with isFoo and getFoo Nov 19, 2015

@benissimo

This comment has been minimized.

Show comment
Hide comment
@benissimo

benissimo Nov 19, 2015

p.s. the workaround in these cases is to rely on get_foo and foo? instead of foo

benissimo commented Nov 19, 2015

p.s. the workaround in these cases is to rely on get_foo and foo? instead of foo

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 19, 2015

Member

Your suggestion is reasonable, and it now seems a little odd to me that we would map "boolean isFoo" to "foo" and not just "foo?".

I expect the issue is that different versions of JVM provide these methods do us in different orders. We could (perhaps should) make this more consistent by sorting the lists of methods before binding them, but I agree this case requires a bit of improvement.

Member

headius commented Nov 19, 2015

Your suggestion is reasonable, and it now seems a little odd to me that we would map "boolean isFoo" to "foo" and not just "foo?".

I expect the issue is that different versions of JVM provide these methods do us in different orders. We could (perhaps should) make this more consistent by sorting the lists of methods before binding them, but I agree this case requires a bit of improvement.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Nov 19, 2015

Member

@benissimo is this 9K only for you or 1.7 as well ?

Member

kares commented Nov 19, 2015

@benissimo is this 9K only for you or 1.7 as well ?

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Nov 19, 2015

Member

on both releases and foo is used as an alias for isFoo ;( ... too bad this didn't get spotted before 9K.

Member

kares commented Nov 19, 2015

on both releases and foo is used as an alias for isFoo ;( ... too bad this didn't get spotted before 9K.

@benissimo

This comment has been minimized.

Show comment
Hide comment
@benissimo

benissimo Nov 19, 2015

@kares I've only got jruby 1.7.22 installed, can't confirm re 9K, but sounds like you have already.

benissimo commented Nov 19, 2015

@kares I've only got jruby 1.7.22 installed, can't confirm re 9K, but sounds like you have already.

kares added a commit to kares/jruby that referenced this issue Nov 19, 2015

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Nov 19, 2015

Member

a bit of a hack but likely the simplest thing we could do: 63b65ef

Member

kares commented Nov 19, 2015

a bit of a hack but likely the simplest thing we could do: 63b65ef

@kares kares closed this Nov 19, 2015

@kares kares added this to the JRuby 1.7.23 milestone Nov 19, 2015

@benissimo

This comment has been minimized.

Show comment
Hide comment
@benissimo

benissimo Nov 20, 2015

Thanks for responding to this so quickly! Kudos!

benissimo commented Nov 20, 2015

Thanks for responding to this so quickly! Kudos!

kares added a commit to kares/jruby that referenced this issue Jan 11, 2017

[ji] deal with conflicting get/is Java method alias (for real)
... unfortunately there's no ordering for the methods thus we have
to also deal with foo being aliased for isFoo while processing getFoo

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't really a getter (and we're avoding a clash due #3262)

kares added a commit to kares/jruby that referenced this issue Jan 11, 2017

[ji] deal with conflicting get/is Java method alias (for real)
currently we're nondeterministic and depend on reflected method order

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due #3262)

kares added a commit to kares/jruby that referenced this issue Jan 11, 2017

[ji] deal with conflicting get/is Java method alias (for real)
currently we're nondeterministic and depend on reflected method order

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due #3262)

kares added a commit to kares/jruby that referenced this issue Jan 11, 2017

[ji] deal with conflicting get/is Java method alias (for real)
currently we're nondeterministic and depend on reflected method order

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due #3262)

kares added a commit to kares/jruby that referenced this issue Jan 11, 2017

[ji] deal with conflicting get/is Java method alias (for real)
currently we're nondeterministic and depend on reflected method order

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due #3262)

kares added a commit to kares/jruby that referenced this issue Jan 11, 2017

[ji] deal with conflicting get/is Java method alias (for real)
currently we're nondeterministic and depend on reflected method order

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due #3262)

kares added a commit to kares/jruby that referenced this issue Jan 12, 2017

[ji] deal with conflicting get/is Java method alias (for real)
currently we're nondeterministic and depend on reflected method order

foo -> getFoo method shall always win, since foo? is there for isFoo
in case both getFoo and isFoo are specified

the update should align nicely with expectations in #3470

due compatibility we can not fix #4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due #3262)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment