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.

Sign up for GitHub

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

Jump to bottom

9k regression on indirect java_class.annotation(java_class) method #4432

Closed
byteit101 opened this issue Jan 10, 2017 · 2 comments
Closed

9k regression on indirect java_class.annotation(java_class) method #4432

byteit101 opened this issue Jan 10, 2017 · 2 comments
Assignees
@kares
Labels
java integration JRuby 9000
Milestone
Won't Fix

Comments

@byteit101
Copy link
Member

Environment

jruby 1.7.19 (1.9.3p551) 2015-01-29 20786bd on Java HotSpot(TM) 64-Bit Server VM 1.8.0_111-b14 +jit [linux-amd64]
and
jruby 9.1.6.0 (2.3.1) 2016-11-09 0150a76 Java HotSpot(TM) 64-Bit Server VM 25.111-b14 on 1.8.0_111-b14 +jit [linux-x86_64]

import java.lang.annotation.*;
@Target({ ElementType.FIELD, ElementType.METHOD, ElementType.TYPE })
@Retention(RetentionPolicy.RUNTIME)
public @interface MyAnnotation {
    String value() default "";
}
import java.util.*;
@MyAnnotation
class MyClass {
	public static Class[] getAll(){
	   return new Class[] { MyClass.class };
    }
}

Expected Behavior

jruby-1.7.19 :002 > java_import 'MyClass'
 => [Java::Default::MyClass] 
jruby-1.7.19 :003 > java_import 'MyAnnotation'
 => [Java::Default::MyAnnotation] 
jruby-1.7.19 :004 > MyClass.all[0].annotation(MyAnnotation.java_class)
 => #<Java::Default::$Proxy24:0x212bf671> 
jruby-1.7.19 :005 > MyClass.all[0].annotation(MyAnnotation.java_class).value
 => ""

Actual Behavior

jruby-9.1.6.0 :003 > java_import 'MyClass'
 => [Java::Default::MyClass] 
jruby-9.1.6.0 :004 > java_import 'MyAnnotation'
 => [Java::Default::MyAnnotation]
jruby-9.1.6.0 :005 > MyClass.all[0].annotation(MyAnnotation.java_class)
ArgumentError: wrong number of arguments (1 for 0)
	from (irb):9:in `<eval>'
	from org/jruby/RubyKernel.java:998:in `eval'
	from org/jruby/RubyKernel.java:1299:in `loop'
	from org/jruby/RubyKernel.java:1118:in `catch'
	from org/jruby/RubyKernel.java:1118:in `catch'

jruby-9.1.6.0 :008 > MyClass.all[0].annotation
 => false

All 1.7 that i've used (since 1.7.3) works as the first, while all the 9.x series that I've tried do the latter. This looks like some funky proxy screw up, as directly referencing MyClass works on 1.7 and 9k:

jruby-9.1.6.0 :005 > MyClass.java_class.annotation(MyAnnotation.java_class)
 => #<Java::Default::$Proxy25:0x6a03bcb1> 
jruby-9.1.6.0 :006 > MyClass.all[0]
 => class MyClass 
jruby-9.1.6.0 :007 > MyClass.java_class
 => class MyClass 
jruby-9.1.6.0 :008 > MyClass.all[0].annotation(MyAnnotation.java_class)
ArgumentError: wrong number of arguments (1 for 0)
@kares
Copy link
Member

kares commented Jan 10, 2017

believe this is a collision on setting up ruby aliases for Java methods.

the first case works since it returns a Java::JavaClass wrapper,
the seconds returns a 'true' java.lang.Class proxy - has getAnnotation(arg) which gets alias-ed as annotation but there's also isAnnotation alias-ed as annotation? and annotation (if not yet defined).
we have a deterministic Java method order-ing in 9K while in 1.7 its whatever java.lang.Class#getMethods returns.

@kares kares self-assigned this Jan 10, 2017
kares added a commit to kares/jruby that referenced this issue Jan 10, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
dd1ec4b 
... 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 (this is quite unlikely)

resolves jruby#4432
kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
581db8b 
... 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 jruby#3470

due compatibility we can not fix jruby#4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't really a getter (and we're avoding a clash due jruby#3262)
kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
a51f61d 
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 jruby#3470

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

kares commented Jan 11, 2017

this lead me to realize there's still non-determinism with get/is aliases although this does not clasify for that.

the previous 1.7 behaviour was unfortunate since it would lead to confusing cases such as #3262
that is why in 9K things got changed and only 'real' get-ers have aliases without the get/is prefix, in this particular case:

  • getAnnotation(param) is not considered a getter since it receives a parameter
  • isAnnotation() is considered a getter and since there's no getAnnotation() annotation alias is set-up

this might sound weird but its better than 1.7 (believe alias-ing might just stay order dependent there).

you should refactor your code to use get_annotation(param)

interestingly the order of methods locally seems pretty deterministic (hard to get a failing test) but on travis-ci I was able to reproduce the issue with getFoo/isFoo Ruby aliasing (shall put that in another report).

@kares kares removed the regression label Jan 11, 2017
@kares kares added this to the Won't Fix milestone Jan 11, 2017
kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
dc78827 
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 jruby#3470

due compatibility we can not fix jruby#4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due jruby#3262)
kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
cdf5956 
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 jruby#3470

due compatibility we can not fix jruby#4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due jruby#3262)
kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
17eb6cf 
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 jruby#3470

due compatibility we can not fix jruby#4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due jruby#3262)
kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares
[ji] deal with conflicting get/is Java method alias (for real)
2e08645 
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 jruby#3470

due compatibility we can not fix jruby#4432
isAnnotation() + getAnnotation(param) case is different since the get
method isn't a (real) getter (and we're avoding a clash due jruby#3262)
@kares kares mentioned this issue Jan 11, 2017
Merged
@kares kares closed this as completed in 74c8725 Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kares kares

Labels
java integration JRuby 9000
Projects
None yet
Milestone
Won't Fix
Development

No branches or pull requests
2 participants
@kares @byteit101