Community
Participate
Working Groups
build I20070918-0010 1) create pack/AClass.java package pack; public class AClass<T> { public static int zork; } 2) create Test.java import static pack.AClass.zork public class Test { public static void main(String[] args) { pack.AClass.zork } } 3) do ctrl+space after zork in the import statement getDeclarationSignature() of the proposal returns 'Lpack.AClass;' 4) do ctrl+space after zork in the main method getDeclarationSignature() of the proposal returns 'Lpack.AClass<TT;>;' The declaration signature should be the same in both case. I think that the generic signature is better because it give more information.
Created attachment 160811 [details] proposed fix v0.5 + regression tests The different signature in case of the proposal for the static import was due to its calculation from a raw type binding for AClass. So the fix just involves taking the erasure to find the exact source type binding corresponding to AClass, which will yield the signature as AClass<T>. Note that the added tests verify this through the generated proposal string's second parameter within the curly braces. There's no way to directly call the getDeclarationSign() method for the generated proposal and verify the result since the proposals are private to CompletionTestRequestor2 class.
Call the erasure() method sounds not to be the good solution to me... In this case, you have a raw type (not 100% sure but I assume that's the only possibility in the import reference) and you want to get the generic type to have the type parameters information in the signature, hence the following fix would look more appropriate (IMO): FieldBinding[] fields; if (ref.isRawType()) { fields = ((RawTypeBinding) ref).genericType().availableFields(); } else { fields = ref.availableFields(); } Moreover, it would avoid a cast which always get me a little bit nervous when I'm not 100% sure that the object is a kind of the casted type... Note also that I really do not like when a method parameter is reassigned (but this is a personal inhabit from a time when I was a C/C++ developer...)
Created attachment 160913 [details] proposed fix v1.0 + regression tests Updated patch incorporating the above suggestion. In my defense, I'd thought the cast would be safe since if one goes to the place where findImportsOfStaticFields() is called, one can see that the parameter 'ref' itself was obtained after a cast to ReferenceBinding on CompletionEngine:line 1784. But yes I agree that calling genericType is better than calling erasure().
(In reply to comment #3) > Created an attachment (id=160913) [details] > proposed fix v1.0 + regression tests > The patch looks good to me. > Updated patch incorporating the above suggestion. > > In my defense, I'd thought the cast would be safe since if one goes to the > place where findImportsOfStaticFields() is called, one can see that the > parameter 'ref' itself was obtained after a cast to ReferenceBinding on > CompletionEngine:line 1784. > I totally agree (as I also verified) but adding a cast exposes the code to a CCE in the future if someone decides to call this method without looking at its implementation and gives a TypeBinding which is not a ReferenceBinding! That's why I was a little bit nervous about it... but not too much though ;-) I just applied one my security principle: when there's a safer solution than a cast, use it... :-) > But yes I agree that calling genericType is better than calling erasure().
Frederic, could you please release this for M7? Thanks!
Verified for 3.6M7 using build I20100424-200
