From f3d887956da63eb256616ca62efc657e5b0e9995 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 23 Aug 2020 15:38:21 +0200 Subject: [PATCH] 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 | 142 ++++++++++++------ .../lang/java/ast/ASTLocalClassStatement.java | 13 +- .../table/internal/SymbolTableResolver.java | 3 +- 3 files changed, 105 insertions(+), 53 deletions(-) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 48d8ae0d44..c5b1aa04b3 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/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 - || isRecordTypeSupported() && next.kind == AT && isToken(2, INTERFACE) - || isRecordTypeSupported() && next.kind == IDENTIFIER && next.getImage().equals("enum") - || isRecordTypeSupported() && next.kind == IDENTIFIER && next.image.equals("record"); + || localTypesSupported() && ( + next.kind == INTERFACE + || next.kind == AT && isToken(2, INTERFACE) + || 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( { isYieldStart() } ) YieldStatement() -| LOOKAHEAD( "@" | "final" ) + LOOKAHEAD(1, "@" | "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() - ( - LOOKAHEAD({localTypeDeclLookahead()}) LocalTypeDecl() - | - LocalVariableDeclaration() ";" + + // The ModifierList is adopted by the next class to open + ModifierList() ( + LOOKAHEAD({localTypeDeclAfterModifiers()}) LocalTypeDecl() + | LOOKAHEAD({true}) LocalVariableDeclaration() ";" { fixLastToken(); } ) -| LOOKAHEAD({classModifierLookahead() || localTypeDeclLookahead()}) - ModifierList() LocalTypeDecl() -| LOOKAHEAD(Type() ) - LocalVariableDeclaration() ";" { - // make it so that the LocalVariableDeclaration's last token is the semicolon - AbstractJavaNode top = (AbstractJavaNode) jjtree.peekNode(); - top.setLastToken(getToken(0)); - } {} -| - // we need to lookahead until the "class" token, - // because a method ref may be annotated - // -> so Expression, and hence Statement, may start with "@" - LOOKAHEAD(ModifierList() "class") LocalTypeDecl() -| - Statement() +| LOOKAHEAD(1, ) + ( + LOOKAHEAD({ localTypeDeclGivenNextIsIdent() }) ModifierList() LocalTypeDecl() + | LOOKAHEAD({ isAssertStart() }) AssertStatement() + | LOOKAHEAD({ isYieldStart() }) YieldStatement() + | LOOKAHEAD({ getToken(2).kind == COLON }) LabeledStatement() + | LOOKAHEAD(ClassOrInterfaceType() ) LocalVariableDeclaration() ";" { fixLastToken(); } + | LOOKAHEAD({true}) ExpressionStatement() + ) +| LOOKAHEAD(1, LocalTypeStartNoIdent()) ModifierList() LocalTypeDecl() +| LOOKAHEAD(1) StatementNoIdent() +| LOOKAHEAD(Type() ) LocalVariableDeclaration() ";" { fixLastToken(); } +| LOOKAHEAD({true}) ExpressionStatement() } -void LocalTypeDecl() #LocalClassStatement: +private void LocalTypeStartNoIdent() #void: // A lookahead {} -{ - ClassOrInterfaceDeclaration() +{ // notice: not default + "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(): {} { } // those are created manually void TypeExpression(): {} { } void PatternExpression(): {} { } +void LocalClassStatement(): {} { TypeDeclaration() } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLocalClassStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLocalClassStatement.java index d3e2fe42b8..7d28ca04cf 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLocalClassStatement.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTLocalClassStatement.java @@ -12,7 +12,7 @@ import org.checkerframework.checker.nullness.qual.NonNull; * * 
  *
- * LocalClassStatement ::= {@link ASTClassOrInterfaceDeclaration ClassDeclaration}
+ * LocalClassStatement ::= {@link ASTAnyTypeDeclaration TypeDeclaration}
  *
  *
*/ @@ -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 R acceptVisitor(JavaVisitor visitor, P data) { return visitor.visit(this, data); @@ -31,8 +37,7 @@ public final class ASTLocalClassStatement extends AbstractStatement { /** * Returns the contained declaration. */ - @NonNull - public ASTClassOrInterfaceDeclaration getDeclaration() { - return (ASTClassOrInterfaceDeclaration) getChild(0); + public @NonNull ASTAnyTypeDeclaration getDeclaration() { + return (ASTAnyTypeDeclaration) getChild(0); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SymbolTableResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SymbolTableResolver.java index f62014de44..9105141004 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/table/internal/SymbolTableResolver.java +++ b/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); }