Summary: | DCR AST: EnumDeclaration: Separate field and body statement lists | ||
---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Martin Aeschlimann <martinae> |
Component: | Core | Assignee: | Olivier Thomann <Olivier_Thomann> |
Status: | VERIFIED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | dirk_baeumer |
Version: | 3.0 | ||
Target Milestone: | 3.1 M3 | ||
Hardware: | PC | ||
OS: | Windows XP | ||
Whiteboard: |
Description
Martin Aeschlimann
2004-10-13 13:50:48 EDT
I agree that the situation is not quite perfect. Let me explain how it came to be the way it is. EnumDeclaration is declared as a subclass of AbstractTypeDeclaration; this was necessary because there needed to be something like this to encompass all forms for type declarations. It's a good fit for AbstractTypeDeclaration to declare bodyDeclarations() because these are what normal types and annotation types have. This is, as you point out, a bit of a stretch for enum declarations, which really have a pair of lists. The first list is the enum constants (EnumConstantDeclaration), and the second list is the (optional) class body declarations. Rather than introducing a second list, I combined them into one. EnumConstantDeclarations are typically very simple (just an identifier), but in complex forms are almost indistinguishable from method declarations. This is why it did not feel like too much of a stretch. The convienience method getEnumConstants() makes it easy for clients to get at the constants. And I thought it would be easy for the DOM/AST implementation to put the ";" in the right place when serializing. The obvious alternative is to add a second list property: public List<EnumConstantDeclaration> enumConstants(); and spec bodyDeclarations() to exclude the enum constants. Do you think that change would make it any easier or harder for clients? When I first looked at the EnumDeclaration I studied the grammar and then wanted to find the enum constant list in the API. I first tought it was forgotten until I understood that the two lists have been merged. So, yes, I think separation would make the API clearer. Manipulations and access on the node would benefit, having a strong typing of the enum constant list, no worries about a illegal order of element (you can't have a normal body declaration between constants). It's more explicit and this is more the style of the rest of the ASTNode API's. I guess the conterargument is that the new list addition would break clients already using the EnumDeclaration. The damage would still be small, the nodes haven't been parsed until recently, so I would vote for it. FYI - I am currently changing the compiler internals to separate the enumConstants from their body declarations. In essence, an enum constant is homogeneous to a field, and it may additionally carry an anonymous type (in the same way as a qualified allocation expression). Ok. Added a second list property to EnumDeclaration: public List<EnumConstantDeclaration> enumConstants(); Deprecated method on EnumDeclaration (to be deleted after 3.1M3): public EnumConstantDeclaration[] getEnumConstants(); Changed spec for EnumDeclaration: bodyDeclarations() does not include enum constant declarations Olivier, please fix AST converter. I am investigating the changes. Fixed and released in HEAD. Tests have been updated to use the new API. ast rewrite updated, tests added > 20041015 Verified for 3.1M3 with build I20041101 |