| Summary: | [1.8][dom] ParameterizedType's 'type' property could be AnnotatableType instead of Type | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Jay Arthanareeswaran <jarthana> |
| Component: | Core | Assignee: | Manoj N Palat <manoj.palat> |
| Status: | RESOLVED WORKSFORME | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | markus.kell.r, srikanth_sankaran |
| Version: | 4.4 | ||
| Target Milestone: | BETA J8 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | |||
|
Description
Jay Arthanareeswaran
Let us also explore whether the annotations on ParameterizedType should/could be attached to itself instead of being held in the Type property. i.e Should @NonNull List<@NonNull String> be viewed as: @NonNull (List<@NonNull String>) or (@NonNull List)<@NonNull String> Compiler follows the former model, while DOM/AST encodes the latter. Looking at the grammar changes for JSR308, I think we should go with the former model: Type: [Annotations] UnannType Such discrepancy could lead to issues if a client traverses to various parts of the tree and queries the annotations and their resolved types. Per javadoc on Type, the following types are not AnnotatableType's: ParameterizedType, UnionType, IntersectionType. The latter two should stay that way, while we need to consider making the former Annotatable. (In reply to Jayaprakash Arthanareeswaran from comment #0) > Currently ParameterizedType is not an AnnotatableType and hence we will have > to make the former a subclass of latter (to cover the scenario where the > type argument itself will be a ParameterizedType) I noticed that ArrayType is not an AnnotatableType either. ArrayTypes can be type arguments too. In the saga of modelling arrays, not sure we considered making ArrayType an AnnotatableType that stores *additional* dimensions in List<Dimension> and let the base dimension be implicit in the ArrayType itself. At the expense of sounding like a broken record: The AnnotatableType abstraction needs a relook. It can work/be made to work, I don't see a fundamental blocking problem with that, but as was pointed out in http://mail.openjdk.java.net/pipermail/type-annotations-spec-observers/2013-February/000092.html whether or not a type is annotated is a "has-a" relationship and not an "is a" relationship. FWIW, This is the approach we have taken in the compiler. I'll also mention here that Core/reflection taken the opposite approach and has come up with an hierarchy of types - AnnotatedType, AnnotatedArrayType, AnnotatedParameterizedType etc. AFAICT, AnnotatableType originated in https://bugs.eclipse.org/bugs/show_bug.cgi?id=391890#c7 due to concerns about Type#annotations() being available on all types. Elsewhere concerns have been raised about making ParameterizedType's annotatable because it offers multiple places to annotate. I think these could be solved by documentation and throwing of exceptions. Summary: If the client side is comfortable with these APIs as they are, that is fine by us. I agree should voiced this much earlier - blame it on too much to do and too little time :( (In reply to Srikanth Sankaran from comment #4) > AFAICT, AnnotatableType originated in > https://bugs.eclipse.org/bugs/show_bug.cgi?id=391890#c7 due to concerns > about Type#annotations() being available on > all types. Would it have worked if Type.annotations() simply returned an unmodifiable empty list and the subtypes that have the annotations property returned modifiable lists with actual, possibly empty list of annotations ? Markus and I discussed this at length and he explained the rationale behind why clients would prefer the current model. These observations are already recorded though in a scattered form across different bugs. Basically, for UI team the need to cast is less of an issue as they work with visitors and being able to block annotatability in multiple places right at compile time is a better approach for them. So I will close this as WORKSFORME. |