Fix grammar

The BlockStatement production is rewritten for performance
and clarity. There was a problem with ModifierLists floating around

pmd-java/etc/grammar/Java.jjt

@@ -275,6 +275,10 @@ class JavaParserImpl {
     return (jdkVersion == 14 || jdkVersion == 15) && preview;
   }
 
+  private boolean localTypesSupported() {
+    return isRecordTypeSupported();
+  }
+
   private boolean isSealedClassSupported() {
       return jdkVersion == 15 && preview;
   }
@@ -294,7 +298,7 @@ class JavaParserImpl {
   // This is a semantic LOOKAHEAD to determine if we're dealing with an assert
   // Note that this can't be replaced with a syntactic lookahead
   // since "assert" isn't a string literal token
-  private boolean isNextTokenAnAssert() {
+  private boolean isAssertStart() {
     if (jdkVersion <= 3) {
       return false;
     }
@@ -302,6 +306,14 @@ class JavaParserImpl {
     return getToken(1).getImage().equals("assert");
   }
 
+  private boolean isRecordStart() {
+    return isRecordTypeSupported() && isKeyword("record");
+  }
+
+  private boolean isEnumStart() {
+    return jdkVersion >= 5 && isKeyword("enum");
+  }
+
   /**
    * Semantic lookahead to check if the next identifier is a
    * specific restricted keyword.
@@ -357,13 +369,22 @@ class JavaParserImpl {
         || isSealedClassSupported() && isNonSealedModifier();
   }
 
-  private boolean localTypeDeclLookahead() {
+  private boolean localTypeDeclAfterModifiers() {
       Token next = getToken(1);
       return next.kind == CLASS
-        || isRecordTypeSupported() && next.kind == INTERFACE
+        || localTypesSupported() && (
-        || isRecordTypeSupported() && next.kind == AT && isToken(2, INTERFACE)
+              next.kind == INTERFACE
-        || isRecordTypeSupported() && next.kind == IDENTIFIER && next.getImage().equals("enum")
+           || next.kind == AT && isToken(2, INTERFACE)
-        || isRecordTypeSupported() && next.kind == IDENTIFIER && next.image.equals("record");
+           || next.kind == IDENTIFIER && next.getImage().equals("enum")
+           ||
+           next.kind == IDENTIFIER && next.image.equals("record")
+      );
+  }
+
+  private boolean localTypeDeclGivenNextIsIdent() {
+      return localTypesSupported() && (
+          isNonSealedModifier() || isRecordStart() || isEnumStart()
+      );
   }
 
   /**
@@ -436,6 +457,11 @@ class JavaParserImpl {
     node.setImage(getToken(0).getImage());
   }
 
+  private void fixLastToken() {
+       AbstractJavaNode top = (AbstractJavaNode) jjtree.peekNode();
+       top.setLastToken(getToken(0));
+  }
+
   private void forceExprContext() {
        AbstractJavaNode top = jjtree.peekNode();
 
@@ -2127,8 +2153,20 @@ void ArrayDimExpr() #void:
 void Statement() #void:
 {}
 {
-  Block()
+  StatementNoIdent()
+// testing the hard cases last optimises the code gen
+// all the previous cases are trivial for the parser
+// because they start with a different token
 | LOOKAHEAD( { isYieldStart() } ) YieldStatement()
+| LOOKAHEAD( { isAssertStart() } ) AssertStatement()
+| LOOKAHEAD(2) LabeledStatement()
+| ( StatementExpression() ";" ) #ExpressionStatement
+}
+
+void StatementNoIdent() #void:
+{}
+{
+  Block()
 | EmptyStatement()
 | SwitchStatement()
 | IfStatement()
@@ -2141,12 +2179,6 @@ void Statement() #void:
 | ThrowStatement()
 | SynchronizedStatement()
 | TryStatement()
-// testing the hard cases last optimises the code gen
-// all the previous cases are trivial for the parser
-// because they start with a different token
-| LOOKAHEAD( { isNextTokenAnAssert() } ) AssertStatement()
-| LOOKAHEAD(2) LabeledStatement()
-| ( StatementExpression() ";" ) #ExpressionStatement
 }
 
 void LabeledStatement() :
@@ -2162,45 +2194,60 @@ void Block() :
 }
 
 void BlockStatement() #void:
-{}
+{} // Note: this has been written this way to minimize lookaheads
+   // This generates a table switch with very few lookaheads
 {
-  LOOKAHEAD( { isNextTokenAnAssert() } ) AssertStatement()
+  LOOKAHEAD(1, "@" | "final" )
-| LOOKAHEAD( { isYieldStart() } ) YieldStatement()
-| LOOKAHEAD( "@" | "final" )
     // this eagerly parses all modifiers and annotations. After that, either a local type declaration
     // or a local variable declaration follows.
     // This allows more modifiers for local variables than actually allowed
-    // and the annotations for local variables need to be moved in the AST down again.
+
-    ModifierList()
+    // The ModifierList is adopted by the next class to open
-    (
+    ModifierList() (
-        LOOKAHEAD({localTypeDeclLookahead()}) LocalTypeDecl()
+        LOOKAHEAD({localTypeDeclAfterModifiers()}) LocalTypeDecl()
-        |
+      | LOOKAHEAD({true})                          LocalVariableDeclaration() ";"    { fixLastToken(); }
-        LocalVariableDeclaration() ";"
     )
-| LOOKAHEAD({classModifierLookahead() || localTypeDeclLookahead()})
+| LOOKAHEAD(1, <IDENTIFIER>)
-    ModifierList() LocalTypeDecl()
+ (
-| LOOKAHEAD(Type() <IDENTIFIER>)
+      LOOKAHEAD({ localTypeDeclGivenNextIsIdent() }) ModifierList() LocalTypeDecl()
-    LocalVariableDeclaration() ";" {
+    | LOOKAHEAD({ isAssertStart() })                 AssertStatement()
-      // make it so that the LocalVariableDeclaration's last token is the semicolon
+    | LOOKAHEAD({ isYieldStart() })                  YieldStatement()
-      AbstractJavaNode top = (AbstractJavaNode) jjtree.peekNode();
+    | LOOKAHEAD({ getToken(2).kind == COLON })       LabeledStatement()
-      top.setLastToken(getToken(0));
+    | LOOKAHEAD(ClassOrInterfaceType() <IDENTIFIER>) LocalVariableDeclaration() ";"  { fixLastToken(); }
-  } {}
+    | LOOKAHEAD({true})                              ExpressionStatement()
-|
+ )
-  // we need to lookahead until the "class" token,
+| LOOKAHEAD(1, LocalTypeStartNoIdent()) ModifierList() LocalTypeDecl()
-  // because a method ref may be annotated
+| LOOKAHEAD(1)                          StatementNoIdent()
-  // -> so Expression, and hence Statement, may start with "@"
+| LOOKAHEAD(Type() <IDENTIFIER>)        LocalVariableDeclaration() ";" { fixLastToken(); }
-  LOOKAHEAD(ModifierList() "class") LocalTypeDecl()
+| LOOKAHEAD({true})                     ExpressionStatement()
-|
-  Statement()
 }
 
-void LocalTypeDecl() #LocalClassStatement:
+private void LocalTypeStartNoIdent() #void: // A lookahead
 {}
-{
+{ // notice: not default
-    ClassOrInterfaceDeclaration()
+    "public" | "static" | "protected" | "private" | "final"
+  | "abstract" | "synchronized" | "native" | "transient"
+  | "volatile" | "strictfp"
+
+  | "class" | "interface"
+}
+
+void LocalTypeDecl() #void:
+{} // At the point this is called, a ModifierList is on the top of the stack,
+{  // waiting for the next node to open. We want that node to be the type declaration,
+   // not the wrapper statement node.
+  ( ClassOrInterfaceDeclaration()
   | AnnotationTypeDeclaration()
   | LOOKAHEAD({isKeyword("record")}) RecordDeclaration()
   | LOOKAHEAD({isKeyword("enum")}) EnumDeclaration()
+  ) {
+    // Wrap the type decl into a statement
+    // This can't be done with regular jjtree constructs, as the ModifierList
+    // is adopted by the first node to be opened.
+    ASTAnyTypeDeclaration type = (ASTAnyTypeDeclaration) jjtree.popNode();
+    ASTLocalClassStatement stmt = new ASTLocalClassStatement(type);
+    jjtree.pushNode(stmt);
+  }
 }
 
 /*
@@ -2241,6 +2288,12 @@ void EmptyDeclaration() :
   ";"
 }
 
+void ExpressionStatement():
+{}
+{
+    StatementExpression() ";"
+}
+
 void StatementExpression() #void:
 {AssignmentOp op = null;}
 {
@@ -2268,12 +2321,6 @@ void SwitchBlock() #void:
     "}"
 }
 
-void SwitchArrowLahead() #void:
-{}
-{
-    SwitchLabel() "->"
-}
-
 void SwitchArrowBranch():
 {}
 {
@@ -2671,3 +2718,4 @@ void VariableAccess(): {} { <IDENTIFIER> }
 // those are created manually
 void TypeExpression(): {} { <IDENTIFIER> }
 void PatternExpression(): {} { <IDENTIFIER> }
+void LocalClassStatement(): {} { TypeDeclaration() }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLocalClassStatement.java

@@ -12,7 +12,7 @@ import org.checkerframework.checker.nullness.qual.NonNull;
  *
  * <pre class="grammar">
  *
- * LocalClassStatement ::= {@link ASTClassOrInterfaceDeclaration ClassDeclaration}
+ * LocalClassStatement ::= {@link ASTAnyTypeDeclaration TypeDeclaration}
  *
  * </pre>
  */
@@ -22,6 +22,12 @@ public final class ASTLocalClassStatement extends AbstractStatement {
         super(id);
     }
 
+    ASTLocalClassStatement(ASTAnyTypeDeclaration tdecl) {
+        super(JavaParserImplTreeConstants.JJTLOCALCLASSSTATEMENT);
+        assert tdecl != null;
+        addChild((AbstractJavaNode) tdecl, 0);
+    }
+
     @Override
     protected <P, R> R acceptVisitor(JavaVisitor<? super P, ? extends R> visitor, P data) {
         return visitor.visit(this, data);
@@ -31,8 +37,7 @@ public final class ASTLocalClassStatement extends AbstractStatement {
     /**
      * Returns the contained declaration.
      */
-    @NonNull
+    public @NonNull ASTAnyTypeDeclaration getDeclaration() {
-    public ASTClassOrInterfaceDeclaration getDeclaration() {
+        return (ASTAnyTypeDeclaration) getChild(0);
-        return (ASTClassOrInterfaceDeclaration) getChild(0);
     }
 }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SymbolTableResolver.java

@@ -17,7 +17,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnonymousClassDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTBlock;
 import net.sourceforge.pmd.lang.java.ast.ASTCatchClause;
-import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration;
 import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit;
 import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall;
 import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration;
@@ -282,7 +281,7 @@ public final class SymbolTableResolver {
                 if (st instanceof ASTLocalVariableDeclaration) {
                     pushed += pushOnStack(f.localVarSymTable(top(), ((ASTLocalVariableDeclaration) st).getVarIds()));
                 } else if (st instanceof ASTLocalClassStatement) {
-                    ASTClassOrInterfaceDeclaration local = ((ASTLocalClassStatement) st).getDeclaration();
+                    ASTAnyTypeDeclaration local = ((ASTLocalClassStatement) st).getDeclaration();
                     pushed += pushOnStack(f.localTypeSymTable(top(), local.getSymbol()));
                     processTypeHeader(local);
                 }