[java] Improve try-with-resources grammar #1897
Conversation
oowekyala
commented
Jul 1, 2019
oowekyala
commented
- ASTResources is made void. There were two layers of nodes that represented the "resource list" abstraction, and that wasn't necessary
- Rename ResourceSpecification to ResourceList (this is more consistent with some other *List nodes we have: ExtendsList, ArgumentList, ImplementsList, soon-to-be ThrowsList)
- Add a LocalVariableDeclaration node inside Resource node, when they're not a concise resource
- This is because Resource in that case has an identical API/ represents the same concept
- This simplifies type resolution, since ClassTypeResolver can work on the VariableDeclarator's initializer just like in other LocalVariableDeclaration
- Remove the workaround in OccurenceFinder. This worked around the fact that concise resources were just a Name, however, it's now an expression, so is handled by the rest of the class
- Resource doesn't extend FormalParameter anymore, refs [java] AST inconsistencies around FormalParameter #998 point 3
- Add an attribute to ResourceList to expose whether there was a trailing semicolon or not -> we can make a rule later to check that
There was a problem hiding this comment.
So we renamed here ResourceSpecification into ResourceList. I don't remember, what we should do here in terms of documentation/API compatibility... we can't deprecate ResourceSpecification on PMD6 since the successor is only in PMD7 available. Maybe just mention it in the javadoc, like for the nodes that will be gone?
pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTTryStatementTest.kt
Show resolved
Hide resolved
Rename ResourceSpecification production to ResourceList, remove Resources production
|
@adangel I'm not sure what mentioning this in the javadocs right now would achieve... Maybe we should just think about this when we're done updating the grammar. We're keeping a tidy list so that seems easy |
Agreed. Let's just collect the removed nodes in the wiki. It's maybe better suited for a later "Upgrade guide". |
