[java] Use single node for annotations #2282
Conversation
Generated by 🚫 Danger |
| {String name = null;} | ||
| { | ||
| "@" name=VoidName() {n.setImage(name);} | ||
| "@" jjtThis.name=VoidName() [ AnnotationMemberList() ] |
There was a problem hiding this comment.
Interesting - this time without a setter. I've no strong opinion whether we should use jjtThis.setName(VoidName()) or jjtThis.name=VoidName()- whatever we do, we should do it the same in all other AST classes. Using a setter would provide us a bit more flexibility (although I don't know, whether we need/should use that - a setter should not contain too much logic...).
There was a problem hiding this comment.
This is something I tried to make the grammar cleaner. But maybe, using a setter would be better, especially if the field is shared (this could be the image field)
| @@ -140,3 +140,6 @@ class ASTCastExpressionTest : ParserTestSpec({ | |||
|
|
|||
|
|
|||
| }) | |||
|
|
|||
| val Annotatable.declaredAnnotationsList: List<ASTAnnotation> | |||
There was a problem hiding this comment.
Does this work, if it is declared here in ASTCastExpressionTest? I could imagine, this test needs to run before the others... This should probably be moved to the DSL in TestExtensions.kt
There was a problem hiding this comment.
Toplevel functions and properties in kotlin are translated to static members of a class:
https://kotlinlang.org/docs/reference/java-to-kotlin-interop.html#package-level-functions
So this doesn't need to run. But for clarity, since this is used in several tests, it should indeed be put into TestExtensions.kt
Grammar change is described below. This also makes Annotatable use node streams.
Annotations
@Aand@A()are semantically equivalent, yet they were parsed as MarkerAnnotation resp. NormalAnnotation. Similarly,@A("")and@A(value="")were parsed as SingleMemberAnnotation resp. NormalAnnotation. This also makes parsing much simpler.