[java] Move annotations inside the node they apply to

[oowekyala]

Annotations are now part of the node they apply to, which fixes a lot of inconsistencies, where sometimes the annotations are inside the node, and sometimes just somewhere in the parent, with no real structure.

Implementation is another jjtree tweak, which allows avoiding looking ahead past the annotations. The annotations are just marked as "pending", and then adopted by the next node to be opened (be it a class or method declaration, whatever).

This makes some nodes completely redundant, which can later be turned into interfaces or removed: TypeDeclaration, ClassOrInterfaceBodyDeclaration, AnnotationTypeBodyDeclaration, and BlockStatement (which for now contains the annotations of a local class for example)

Some examples:

Code	Old AST	Current 7.0 AST	New AST after this PR
Top-level type declaration @A class C {}	+TypeDeclaration +MarkerAnnotation "A" +ClassOrInterfaceDeclaration +ClassOrInterfaceBody	Same	+TypeDeclaration - +MarkerAnnotation "A" +ClassOrInterfaceDeclaration + +MarkerAnnotation "A" +ClassOrInterfaceBody
Cast expression (@A T.@B S) expr	N/A (Parse error)	+CastExpression +MarkerAnnotation "A" +ClassOrInterfaceType "S" +MarkerAnnotation "B" +AmbiguousName "T" +(Expression `expr`)	+CastExpression - +MarkerAnnotation "A" +ClassOrInterfaceType "S" +MarkerAnnotation "B" - +AmbiguousName "T" + +ClassOrInterfaceType "T" + +MarkerAnnotation "A" +(Expression `expr`) Notice how T is not ambiguous anymore.
Cast expression with intersection (@A T & S) expr	+CastExpression +MarkerAnnotation "A" +ClassOrInterfaceType "T" +ClassOrInterfaceType "S" +(Expression `expr`)	+CastExpression +MarkerAnnotation "A" + +IntersectionType +ClassOrInterfaceType "T" +ClassOrInterfaceType "S" +(Expression `expr`)	+CastExpression - +MarkerAnnotation "A" +IntersectionType +ClassOrInterfaceType "T" + +MarkerAnnotation "A" +ClassOrInterfaceType "S" +(Expression `expr`) Notice @A binds to T, not T & S
Constructor call new @A T()	+AllocationExpression +MarkerAnnotation "A" +Type +ReferenceType +ClassOrInterfaceType "T" +Arguments	+ConstructorCall +MarkerAnnotation "A" +ClassOrInterfaceType "T" +ArgumentsList	+ConstructorCall +ClassOrInterfaceType "T" +MarkerAnnotation "A" +ArgumentsList
Array allocation new @A int[0]	+AllocationExpression +MarkerAnnotation "A" +Type +PrimitiveType "int" +ArrayDimsAndInits +Expression +PrimaryExpression +Literal "0"	+ArrayAllocation +MarkerAnnotation "A" +PrimitiveType "int" +ArrayAllocationDims +ArrayDimExpr +NumericLiteral "0"	+ArrayAllocation - +MarkerAnnotation "A" +PrimitiveType "int" + +MarkerAnnotation "A" +ArrayAllocationDims +ArrayDimExpr +NumericLiteral "0"
Array type @A int @B[]	N/A (parse error)	+MarkerAnnotation "A" +ArrayType +PrimitiveType "int" +ArrayTypeDims +ArrayTypeDim +MarkerAnnotation "B"	-+MarkerAnnotation "A" +ArrayType +PrimitiveType "int" + +MarkerAnnotation "A" +ArrayTypeDims +ArrayTypeDim +MarkerAnnotation "B" Notice @A binds to int, not int[]
Type parameters <@A T, @B S extends @C Object>	+TypeParameters +MarkerAnnotation "A" +TypeParameter "T" +MarkerAnnotation "B" +TypeParameter "S" +MarkerAnnotation "C" +TypeBound +ReferenceType +ClassOrInterfaceType "Object"	+TypeParameters +MarkerAnnotation "A" +TypeParameter "T" +MarkerAnnotation "B" +TypeParameter "S" +MarkerAnnotation "C" +TypeBound - +ReferenceType +ClassOrInterfaceType "Object"	+TypeParameters - +MarkerAnnotation "A" +TypeParameter "T" + +MarkerAnnotation "A" - +MarkerAnnotation "B" +TypeParameter "S" + +MarkerAnnotation "B" - +MarkerAnnotation "C" - +TypeBound +ClassOrInterfaceType "Object" + +MarkerAnnotation "C" TypeParameters now only can have TypeParameter as a child Annotations that apply to the param are in the param Annotations that apply to the bound are in the type This removes the need for TypeBound, because annotations are cleanly placed.
Enum constants enum { @A E1, @B E2 }	+EnumBody +MarkerAnnotation "A" +EnumConstant "E1" +MarkerAnnotation "B" +EnumConstant "E2"	+EnumBody +MarkerAnnotation "A" +EnumConstant "E1" + +VariableDeclaratorId "E1" +MarkerAnnotation "B" +EnumConstant "E2" + +VariableDeclaratorId "E1"	+EnumBody - +MarkerAnnotation "A" +EnumConstant "E1" + +MarkerAnnotation "A" +VariableDeclaratorId "E1" - +MarkerAnnotation "B" +EnumConstant "E2" + +MarkerAnnotation "B" +VariableDeclaratorId "E1" Annotations are not just randomly in the enum body anymore

[oowekyala]

I added a lengthy test case for type annotations that includes all contexts where type annotations may appear for exhaustiveness. It's based off the spec of type annotations.

There are a few cases where type annotations are legal that this PR fixes

• On method references: @Nonnull String::length
• On instanceof expressions: o instanceof @Readonly String
• On all bounds of an intersection type: @A U1 & @B U2. I originally misread the spec and thought only the first type could be annotated.

There are two cases which we currently don't handle:

• On the types of a throws clause: void foo() throws @A Exception, java.io.@B IOException
  • This is easy to fix and there's already a TODO on the wiki, about replacing NameList with ThrowsList, which would be a list of ClassOrInterfaceType
• On the varargs ellipsis ...: void varargs(String @A ... last);
  • In that case, the ... is treated just like array dimensions and annotations apply exactly as if the param was declared as String @A [] last
  • The sensible way to parse that would probably be to use ArrayTypeDims in some way... I'm not sure yet.
  • Eg maybe we should parse String... just as if it was String[], so use an ArrayType.
    • This causes problems because ArrayType is not fully recursive, so that the component type of an ArrayType is not always represented by a node.
      • Eg in String[], the component type is represented by the ClassOrInterfaceType
      • But in String[][], there is no node representing the component type String[] , since [][] are flat.
      • So maybe we should represent array types like the following:

Code	Old AST	Current 7.0 AST	Proposed representation
String[][]	+Type +ReferenceType[@ArrayDepth=2] +ClassOrInterfaceType "String"	+ArrayType +ClassOrInterfaceType "String" +ArrayTypeDims +ArrayTypeDim +ArrayTypeDim	+ArrayType +ArrayType +ClassOrInterfaceType "String"
String @A [] @B []	N/A (Parse error)	+ArrayType +ClassOrInterfaceType +ArrayTypeDims +ArrayTypeDim +Annotation "A" +ArrayTypeDim +Annotation "B"	+ArrayType +ArrayType +ClassOrInterfaceType +Annotation "A" +Annotation "B"
With varargs: String @A [] @B ...	N/A (Parse error)	N/A (Parse error)	+ArrayType[@Varargs = true()] +ArrayType +ClassOrInterfaceType +Annotation "A" +Annotation "B"

Note that this representation is both more economic in number of nodes (especially in the most common case where there are no annotations on the array dimensions), and also preserves every intermediary array type, which is nice.

[adangel]

Is it important, that the interface starts with the prefix "J"? We don't usually do this...

[oowekyala]

I only did this, because this interface is equivalent to one that is introduced in #1622, and I want to avoid the name clash. When that PR is merged we can remove this one.

[adangel]

We should probably rename this class to avoid confusion 😄

Do we have annotated method declarations covered? Like

@Override
public void run() { }

[adangel]

@oowekyala
Sorry for the delay. I've merged it now into java-grammar. I've updated https://github.com/pmd/pmd/wiki/PMD-7.0.0-Java and https://github.com/pmd/pmd/wiki/Java_clean_changes#annotation-nesting