Community
Participate
Working Groups
Build Identifier: m20090917-0800 Quoted from IBindings.getAnnotations(): "Type bindings - these are annotations on a class, interface, enum, or annotation type declaration. The result is the same regardless of whether the type is parameterized." However, when we call TypeBindings.getAnnotations() on an instance of a generified type with an annotation, we get null. If we instead call TypeBindings.getErasure().getAnnotations(), we get the annotations as intended. Unless I'm misunderstanding something, this breaks the statement ending "... regardless of whether the type is parameterized." Reproducible: Always Steps to Reproduce: 1. define @randomAnnotation class foo<T> { public void bar() {} } class baz { private foo<Integer> _a; void bee () { _a.bar(); } } 2. Define an ASTVisitor over the compilation unit including baz.bee() in visit(MethodDeclaraction md) { ITypeBinding baseBinding = md.getDeclaringClass(); IAnnotationBinding[] baseAnnotations = baseBinding.getAnnotations(); ITypeBinding erasedBindings = baseBinding.getErasure(); IAnnotationBinding[] erasedAnnotations = erasedBindings.getAnnotations(); } baseBindings will refer to foo<Integer>, baseAnnotations will be null. erasedBindings will refer to foo<T>, erasedAnnotations will not be null. Sorry if something is lost here, it's really hard to see if I included everything in this tiny space.
Walter, should not we retrieve first the declaration? When we have a parameterized type, I think we should first get the corresponding generic type to retrieve the annotations. Walter, any comment ?
(In reply to comment #1) > Walter, should not we retrieve first the declaration? > When we have a parameterized type, I think we should first get the > corresponding generic type to retrieve the annotations. > Walter, any comment ? In the APT API, there is a distinction between types and declarations; declarations refer to a particular piece of code and may have annotations, where types refer to an abstract entity created by the compiler and they cannot have annotations. This distinction is hard to understand and frequently obscure, and it is not present in the Eclipse compiler architecture, which was one of the challenges involved in implementing APT within Eclipse. If we had that distinction here, then we'd say that foo<Integer> is a _type_, which can't have annotations; the declaration that produced the type, though, is the declaration of foo<T> (in foo.java, presumably), and that _declaration_ is annotated with @randomAnnotation. Note that if baz had a generic method foo<T> quux(T t), the _type_ of the return value would be foo<T>, but it would still be just a type and not annotated. All the above has to do with the APT API, though, and not with Eclipse TypeBindings, which serve for both types and declarations. So, I am not sure how you want to handle this, within the Eclipse compiler's API.
I think the documentation is misleading. What it should say is that: "The result is the same regardless of whether the type is generic." instead of: "The result is the same regardless of whether the type is parameterized." The result that you are getting are expected as Walter pointed out. It makes sense that calling getErasure() allows to get annotation, since the erasure returns the generic type. So I would say we should only "fix" the documentation and the implementation itself is fine. Walter, would you suggest another sentence for that above statement ?
(in reply to comment #3): Though you're right, it should say "generic" instead of "parameterized", I don't think that helps fix the problem here. The problem is that the result _isn't_ the same. (in reply to comment #2): I think I understand what you're saying - and, indeed, I had similar problems with APT. But in APT, types (which I presume map to bindings) and declarations are different objects - and I don't believe there's even a call to get annotations from the type (though I'm very new to APT, I could be very wrong). As you say, TypeBindings has no such distinction - it has to represent both. Even if it did, I don't understand what there is to be gained by not carrying the annotation around everywhere - it seems to me to break the whole notion of generics - that they are simply artificial constructs to help type-checking; that under the hood, they are all the same thing. Since, with the current system, _any_ foo<concrete X) would have null annotations (since there's no way to specify annotations for only one parameterization of foo), why not just have it always return the annotations of the erasure? That seems the only useful thing getAnnotations could do in that case, isn't it?
(In reply to comment #3) > Walter, would you suggest another sentence for that above statement ? I think Nathan's argument is interesting (that is, that we should always retrieve the annotations from the erasure). I don't know if there are performance issues, though - maybe the binding to the erasure is not always available? Also, it is dangerous to change behavior of an API that has existed for some time. We should resolve that before deciding how to word the javadoc, probably.
I kind of disagree there. If you look at the three methods: org.eclipse.jdt.core.dom.ITypeBinding#isGenericType() org.eclipse.jdt.core.dom.ITypeBinding#isParameterizedType() org.eclipse.jdt.core.dom.ITypeBinding#isRawType() you can see that ITypeBinding also makes a distinction between reference to a generic type (raw or parameterized types) and its declaration (generic type). Annotations only make sense on a declaration. What you are trying to get is the annotations on the corresponding type declaration (same for a method). For the IMethodBinding, we are not consistent as we return the annotations of the declaration even if this is a method reference. We don't have an API on IMethodBinding to return the declaration of the method. I don't think we should introduce the same inconsistency for ITypeBinding.
(In reply to comment #6) Two issues with this logic: (1) If we accept your argument, shouldn't we get rid of getAnnotations() completely in the case of a reference? I don't see anything it could return in that case, so why have it at all? (2) This isn't simply a case of reference vs. declaration, at least not in the current implementation. In the reproduction steps, if removes the generification from foo, baseBindings.getAnnotations() returns @randomAnnotation, even though it is still merely a reference, not a declaration. The behavior in question is caused by the interaction of generics and the reference/declaration split.
(In reply to comment #7) > Two issues with this logic: > (1) If we accept your argument, shouldn't we get rid of getAnnotations() > completely in the case of a reference? I don't see anything it could return in > that case, so why have it at all? I think getAnnotations() should only return a value for a declaration (method, field, type). What would be the meaning of getAnnotations() for anything else than a declaration ? Now the fact that the current implementation doesn't respect this would be a bug. The documentation of getAnnotations() should replace "parameterized" with "generic" for both methods and types cases. Walter, what is your opinion on this?
(In reply to comment #7) > (2) This isn't simply a case of reference vs. declaration, at least not in the > current implementation. In the reproduction steps, if removes the > generification from foo, baseBindings.getAnnotations() returns > @randomAnnotation, even though it is still merely a reference, not a > declaration. The behavior in question is caused by the interaction of generics > and the reference/declaration split. Just to be clear, when I am talking about a reference, it is not necessarily from where the binding comes from. When you get a parameterized binding, it is always a reference. So in this case, I would expect the getAnnotations() call to return an empty list.
Another way to fix this would be to implement what the documentation describes even if I believe this is a bug in the documentation in this case. Since this was defined in 3.2, I don't think it really matters.
Ok, last word about this. In your example, I think you should not take the erasure even if it works in this case, but you should do this: baseBinding.getTypeDeclaration().getAnnotations(); Now I would like to get your explanation of what means getAnnotations() for something else than a declaration. A parameterized type binding cannot be a declaration and therefore I don't see this as a bug.
Created attachment 160817 [details] Proposed fix + regression tests
Released for 3.6M6. Added regression tests: org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test342 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test343
I disagree with this fix. (In reply to comment #11) > you should do this: baseBinding.getTypeDeclaration().getAnnotations(); Yes. > Now I would like to get your explanation of what means getAnnotations() for > something else than a declaration. > > A parameterized type binding cannot be a declaration and therefore I don't see > this as a bug. The problem is that up to the addition of generics, the same JDT ITypeBindings always stood for the declaration as well as the reference. Now, we have different bindings for generic types, but still the same for non-generic types. For client code, it's always easier if the reference bindings also carry all the attributes of the declaration bindings. Note that reference bindings already return the right values in other places, e.g. getModifiers(), isDeprecated(), getBinaryName(), isVarargs(), ... . I think getAnnotations() shouldn't be treated differently. For non-generic types, it already returns the right result even if the binding is from a reference, so it would be consistent to also return the annotation from a parameterized or raw reference binding.
Created attachment 160833 [details] Proposed fix + regression tests Same tests are added but slightly modified.
For me annotations make sense only for a declaration. I released the fix you wanted. Released for 3.6M6.
Verified for 3.6M6 using I20100307-2000 build.
