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

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

Projects

None yet

2 participants

@byteit101
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
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 kares added a commit to kares/jruby that referenced this issue Jan 10, 2017
@kares kares [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 (this is quite unlikely)

resolves #4432
dd1ec4b
@kares kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares kares [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)
581db8b
@kares kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares kares [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)
a51f61d
@kares
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 kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares kares [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)
dc78827
@kares kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares kares [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)
cdf5956
@kares kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares kares [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)
17eb6cf
@kares kares added a commit to kares/jruby that referenced this issue Jan 11, 2017
@kares kares [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)
2e08645
@kares kares added a commit that closed this issue Jan 17, 2017
@kares kares [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)
74c8725
@kares kares closed this in 74c8725 Jan 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment