(This is testable with the jar in the wiki)

Main changes

• Many nodes are now interfaces and not separate nodes in the tree
• PrimaryExpression grammar is completely rewritten
• [java] About operator nodes #1661 is implemented
• Fixes [java] Add new node for anonymous class declaration #905
• Type grammar is substantially changed
  • This adds support for some Java 8 constructs our parser doesn't recognise yet (refs [java] Java8 parsing corner case with annotated array types  #997, fixes [java] Parsing error on annotated inner class #1367)

There are many things still to do to have a consistent AST, eg the new ArrayTypeDims should be used in the other places it's meant for, Dimensionable should be removed, etc. This is already big enough (keep in mind, about half of the line changes are tests).

All the changes to the AST structure are already described on this wiki page in the category "Done".

There's one change that was not in the preview version you've already seen: the parameters of lambda expressions are now their own node, which removes inconsistencies between the case when there's a single VariableDeclaratorId and the case where there's a full FormalParameter.

Temporary measures

• I didn't fix any tests of rules, I just kept them compilable, which sometimes involved shamelessly commenting out some code
• The only tests I made sure did pass are the kotlin tests of the ast package
• Nodes that are not pushed anymore by the parser are not removed, just deprecated (for the rules to compile).
• They're not on the visitor interface anymore but there are some deprecated methods on the adapter and AbstractJavaRule to keep implementors compilable

PrimaryExpression

JavaCC doesn't support left-recursion, so the left recursive nodes are built by manipulating the jjtree stack.

You can see in alljavacc.xml that the method extendLeft is inserted in JJTreeJavaParserState.java. This bumps the arity of the current node (the one being built, ie is not on the stack) by one, which means that if the node ends up being closed, the node that was pushed immediately before the current one is added as a child of the current node.

The productions PrimaryPrefix and PrimarySuffix are kept but made #void.

• The first parses the expressions that can only occur in start position, eg array allocation. It tries its best to not push an AmbiguousName with some lookaheads
• The second parses a segment that needs a left-hand side, e.g. .foo() or ::new. The extendLeft method is called systematically by all of those productions to enclose the previous node on the stack, which necessarily exists because we've already parsed a PrimaryPrefix (or another suffix).

Disambiguation

The parser needs to parse AmbiguousNames to keep lookaheads to a minimum. The disambiguation that uses only syntactic context is implemented at various points in the parser and the initialization code of the nodes

• The javadoc of ASTAmbiguousName gives an overview of that with relevant links.

Since this version already needs to reclassify some ambiguous names, the machinery to actually replace a name with an unambiguous version is already in place. A later PR can add a naive disambiguation pass that uses the current symbol table in order to remove most of the ambiguous names

Operator nodes (#1661)

That's implemented using basically the same template for all the relevant nodes. See AbstractLrBinaryExpr and eg the production ShiftExpression

Optional vs @Nullable

One thing I decided which may be controversial is not to use java.util.Optional in the AST API. Instead I went with JSR 305's @Nullable and @Nonnull annotations, for several reasons:

• The Optional type was initially added as a form of documentation, specifically for return types (eg not for fields or method parameters). Nullable annotations achieve the same goal but may be used in all places where it's useful, without having to explicitly wrap and unwrap optionals everywhere.
• Using annotations makes it possible to incrementally migrate the codebase to this better documented form, without introducing API breaking changes. Many existing methods that have a nullable result won't need to be broken.
• Using annotations improves the Kotlin compatibility by a long range. The AST API can be deemed idiomatic in both Java and Kotlin.
• The Optional API is very incomplete anyway, eg it lacks an isEmpty method, ifPresentOrElse, or or (added in Java 11, 9 and 9 respectively), there's still no ifEmpty. Many times you just want to check whether a node is present, in which case dealing with Optional instead of doing a simple null check is obnoxious. When you do want to use Optional features like map or stuff then Optional.ofNullable is easy to add.
• The difference that made me switch is that it's possible to override a Nullable method with a Nonnull method, which is of course not possible with Optional. This allows more things to be abstracted. E.g. you can have an interface QualifiableExpr { @Nullable getLhs(); } and implement it in ASTArrayAccess with a @Nonnull, instead of having to return a never-empty Optional.

@NoAttribute

This is used to remove the XPath attributes of ASTAmbiguousName to the minimum. It has many irrelevant attributes because it implements ASTExpression and ASTType (maybe that should be changed though). We can eg also use it to remove the @Image attribute from the new nodes to avoid people using them. It's pretty simple but not tested yet.

Other stuff

• I made it so that the children table is never null (just use an empty array). That makes it easier to work with and also ensures that when trying to fetch a child that doesn't exist, only ArrayIndexOutOfBoundsException is thrown, and never NullPointerException
• The test DSL now asserts the text bounds of nodes are ordered by inclusion (which is messed up by touching with the jjtree stack). What's currently done on master with SwitchLabeledRule breaks this invariant, so that I can't extract those changes into a separate PR without bringing in some more changes from here.
• I removed the SemanticCheck attribute from AmbiguousName, we can reuse the removed code to do the temporary disambiguation pass later
• JavaParserVisitorAdapter now delegates by default to abstract supertypes of nodes. So eg overriding visit(ASTExpression) effectively visits all expressions.
  • This is error prone to maintain manually so we should probably generate the delegation code somehow (would be nice to have it in the default interface methods so that AbstractJavaRule can have the same behaviour without duplication).
  • I deprecated JavaParserVisitorReducedAdapter because it doesn't serve any purpose now. MethodLikeNode introduces a discrepancy because now LambdaExpression delegates to visit(ASTExpression) and not visit(ASTMethodLikeNode). I think it should be removed entirely, it makes LambdaExpression implement AccessNode (!). This can go with a simplification of the metrics framework. There's no reason that metrics be constrained to be computed on methods, most of them can actually be computed on any block of code...