From 36f19eda9262cd6fbc146bb030770f00f4792368 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 6 Nov 2022 22:14:09 +0100 Subject: [PATCH] Push actual annotation nodes This is to avoid having rules like UnusedImports fail to see an annotation usage. This makes the tree rather messy when there are type annotations though. --- pmd-java/etc/grammar/Java.jjt | 63 ++++++++----------- .../java/ast/ASTInstanceOfExpression.java | 2 +- .../pmd/lang/java/ast/ASTPatternTest.kt | 11 ++-- .../java16/PatternMatchingInstanceof.txt | 3 + .../ast/jdkversiontests/java16/Records.txt | 3 + 5 files changed, 37 insertions(+), 45 deletions(-) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 7a6508ee02..f50de9e97a 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -1383,7 +1383,7 @@ void RecordComponent(): { (RecordComponentModifier())* Type() - [ (AnnotationNoNode())* "..." {jjtThis.setVarargs();} ] + [ (TypeAnnotation())* "..." {jjtThis.setVarargs();} ] VariableDeclaratorId() } @@ -1489,7 +1489,7 @@ void VariableDeclaratorId() : { (LOOKAHEAD(2) t= "." { checkforBadExplicitReceiverParameter(); jjtThis.setExplicitReceiverParameter(); image=t.image + ".this"; } | t= { checkforBadExplicitReceiverParameter(); jjtThis.setExplicitReceiverParameter(); image = t.image;} - | t= { image = t.image; } ( (AnnotationNoNode())* "[" "]" { jjtThis.bumpArrayDepth(); })* + | t= { image = t.image; } ( (TypeAnnotation())* "[" "]" { jjtThis.bumpArrayDepth(); })* ) { checkForBadAssertUsage(image, "a variable name"); @@ -1532,7 +1532,7 @@ void MethodDeclarator() : checkForBadEnumUsage(t.image, "a method name"); jjtThis.setImage( t.image ); } - FormalParameters() ( (AnnotationNoNode())* "[" "]" )* + FormalParameters() ( (TypeAnnotation())* "[" "]" )* } @@ -1547,8 +1547,8 @@ void FormalParameter() : } { ( "final" {jjtThis.setFinal(true);} | Annotation() )* - Type() ("|" {checkForBadMultipleExceptionsCatching();} (AnnotationNoNode())* Type())* - [ (AnnotationNoNode())* "..." {checkForBadVariableArgumentsUsage();} {jjtThis.setVarargs();} ] + Type() ("|" {checkForBadMultipleExceptionsCatching();} (TypeAnnotation())* Type())* + [ (TypeAnnotation())* "..." {checkForBadVariableArgumentsUsage();} {jjtThis.setVarargs();} ] VariableDeclaratorId() } @@ -1593,16 +1593,16 @@ void Type(): Token t; } { - LOOKAHEAD( | PrimitiveType() (AnnotationNoNode())* "[" "]" ) ReferenceType() + LOOKAHEAD( | PrimitiveType() (TypeAnnotation())* "[" "]" ) ReferenceType() | PrimitiveType() } void ReferenceType(): {} { - PrimitiveType() (LOOKAHEAD((AnnotationNoNode())* "[" "]") (AnnotationNoNode())* "[" "]" { jjtThis.bumpArrayDepth(); })+ + PrimitiveType() (LOOKAHEAD((TypeAnnotation())* "[" "]") (TypeAnnotation())* "[" "]" { jjtThis.bumpArrayDepth(); })+ | - ClassOrInterfaceType() (LOOKAHEAD((AnnotationNoNode())* "[" "]") (AnnotationNoNode())* "[" "]" { jjtThis.bumpArrayDepth(); })* + ClassOrInterfaceType() (LOOKAHEAD((TypeAnnotation())* "[" "]") (TypeAnnotation())* "[" "]" { jjtThis.bumpArrayDepth(); })* } void ClassOrInterfaceType(): @@ -1613,7 +1613,7 @@ void ClassOrInterfaceType(): { t= {s.append(t.image);} [ LOOKAHEAD(2) TypeArguments() ] - ( LOOKAHEAD(2) "." (AnnotationNoNode())* t= {s.append('.').append(t.image);} [ LOOKAHEAD(2) TypeArguments() ] )* + ( LOOKAHEAD(2) "." (TypeAnnotation())* t= {s.append('.').append(t.image);} [ LOOKAHEAD(2) TypeArguments() ] )* {jjtThis.setImage(s.toString());} } @@ -1745,8 +1745,6 @@ void AssignmentOperator() : | "|=" {jjtThis.setImage("|="); jjtThis.setCompound();} } -// TODO Setting isTernary is unnecessary, since the node is only pushed on the stack if there is at least one child, -// ie if it's a ternary void ConditionalExpression() #ConditionalExpression(>1) : {} { @@ -1792,7 +1790,7 @@ void EqualityExpression() #EqualityExpression(>1): void Pattern() #void: {} { - LOOKAHEAD(ReferenceType() "(") RecordPattern() + LOOKAHEAD((Annotation())* ReferenceType() "(") RecordPattern() | LOOKAHEAD("(") ParenthesizedPattern() | TypePattern() [ GuardedPatternCondition() #GuardedPattern(2) {checkForGuardedPatterns();} ] } @@ -1820,7 +1818,7 @@ void TypePattern(): void RecordPattern(): { checkForRecordPatterns(); } { - ReferenceType() RecordStructurePattern() [ VariableDeclaratorId() ] + (Annotation())* ReferenceType() RecordStructurePattern() [ VariableDeclaratorId() ] } void RecordStructurePattern() #ComponentPatternList: @@ -1839,16 +1837,15 @@ void InstanceOfExpression() #InstanceOfExpression(>1): {} { RelationalExpression() - [ "instanceof" (AnnotationNoNode())* + [ "instanceof" ( - LOOKAHEAD("final" | "@") {checkforBadInstanceOfPattern();} Pattern() - | - LOOKAHEAD("(") Pattern() {checkForParenthesizedInstanceOfPattern();} - | - LOOKAHEAD(ReferenceType() "(") RecordPattern() - | - Type() - [ {checkforBadInstanceOfPattern();} VariableDeclaratorId() #TypePattern(2) ] + // Note: this can be simplified when support for java 18 preview is removed. + // Here the production Pattern is inlined to avoid that a following conditional && + // be parsed as a pattern guard. + LOOKAHEAD("final" | (Annotation())* Type() ) {checkforBadInstanceOfPattern();} TypePattern() + | LOOKAHEAD((Annotation())* ReferenceType() "(") {checkForRecordPatterns();} RecordPattern() + | LOOKAHEAD("(") {checkForParenthesizedInstanceOfPattern();} ParenthesizedPattern() + | (Annotation())* Type() ) ] } @@ -2004,7 +2001,7 @@ void PrimaryPrefix() : | AllocationExpression() | LOOKAHEAD( ResultType() "." "class" ) ResultType() "." "class" | LOOKAHEAD( Name() "::" ) Name() // followed by method reference in PrimarySuffix -| LOOKAHEAD( "@" | Type() "::" ) (AnnotationNoNode())* (LOOKAHEAD(2) ReferenceType()|PrimitiveType()) // followed by method reference in PrimarySuffix +| LOOKAHEAD( "@" | Type() "::" ) (Annotation())* (LOOKAHEAD(2) ReferenceType()|PrimitiveType()) // followed by method reference in PrimarySuffix | Name() } @@ -2033,7 +2030,7 @@ void LambdaParameter() #FormalParameter : { ( "final" {jjtThis.setFinal(true);} | Annotation() )* LambdaParameterType() - [(AnnotationNoNode())* "..." {checkForBadVariableArgumentsUsage();} {jjtThis.setVarargs();} ] + [(TypeAnnotation())* "..." {checkForBadVariableArgumentsUsage();} {jjtThis.setVarargs();} ] VariableDeclaratorId() } @@ -2098,7 +2095,7 @@ void AllocationExpression(): PrimitiveType() ArrayDimsAndInits() | [TypeArguments()] - (AnnotationNoNode())* + (TypeAnnotation())* ClassOrInterfaceType() ( ArrayDimsAndInits() @@ -2121,10 +2118,9 @@ void AllocationExpression(): void ArrayDimsAndInits() : {} { - LOOKAHEAD((TypeAnnotation())* "[" "]") - ((AnnotationNoNode())* "[" "]" {jjtThis.bumpArrayDepth();})+ ArrayInitializer() -| ( LOOKAHEAD((TypeAnnotation())* "[" UnaryExprNotPmStart() ) (AnnotationNoNode())* "[" Expression() "]" {jjtThis.bumpArrayDepth();} )+ - ( LOOKAHEAD((TypeAnnotation())* "[") (AnnotationNoNode())* "[" "]" {jjtThis.bumpArrayDepth();} )* + LOOKAHEAD((TypeAnnotation())* "[" "]") ((TypeAnnotation())* "[" "]" {jjtThis.bumpArrayDepth();})+ ArrayInitializer() +| ( LOOKAHEAD((TypeAnnotation())* "[" UnaryExprNotPmStart() ) (TypeAnnotation())* "[" Expression() "]" {jjtThis.bumpArrayDepth();} )+ + ( LOOKAHEAD((TypeAnnotation())* "[") (TypeAnnotation())* "[" "]" {jjtThis.bumpArrayDepth();} )* } @@ -2549,15 +2545,6 @@ void RSIGNEDSHIFT(): // TODO 7.0.0 make #void /* Annotation syntax follows. */ - -// Todo PMD7 remove this, this is transitional in the PMD6 branch. -void AnnotationNoNode() #void: -{checkForBadTypeAnnotations();} -{ - Annotation() - {jjtree.popNode();} -} - void Annotation(): {} { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTInstanceOfExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTInstanceOfExpression.java index e20c5917bc..2f14dad482 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTInstanceOfExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTInstanceOfExpression.java @@ -46,7 +46,7 @@ public class ASTInstanceOfExpression extends AbstractJavaTypeNode { * Gets the type against which the expression is tested. */ public ASTType getTypeNode() { - JavaNode child = getChild(1); + JavaNode child = getChild(getNumChildren() - 1); return child instanceof ASTType ? (ASTType) child : ((ASTTypePattern) child).getTypeNode(); } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTPatternTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTPatternTest.kt index 1050fea440..7862e26d66 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTPatternTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTPatternTest.kt @@ -54,12 +54,11 @@ class ASTPatternTest : ParserTestSpec({ "obj instanceof @Deprecated Class c" should matchExpr { unspecifiedChild() child { -// TODO PMD 7 reenable -// child(ignoreChildren = true) { -// it.annotationName shouldBe "Deprecated" -// } -// -// it.isAnnotationPresent("java.lang.Deprecated") shouldBe true + child(ignoreChildren = true) { + it.annotationName shouldBe "Deprecated" + } + + it.isAnnotationPresent("java.lang.Deprecated") shouldBe true it::getTypeNode typeShouldBe child(ignoreChildren = true) {} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/PatternMatchingInstanceof.txt b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/PatternMatchingInstanceof.txt index c10b239bb7..00f533dd0e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/PatternMatchingInstanceof.txt +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/PatternMatchingInstanceof.txt @@ -379,6 +379,9 @@ | | | | +- PrimaryPrefix[@SuperModifier = false, @ThisModifier = false] | | | | +- Name[@Image = "obj"] | | | +- TypePattern[@ParenthesisDepth = 0] + | | | +- Annotation[@AnnotationName = "Deprecated"] + | | | | +- MarkerAnnotation[@AnnotationName = "Deprecated"] + | | | | +- Name[@Image = "Deprecated"] | | | +- Type[@Array = false, @ArrayDepth = 0, @ArrayType = false, @TypeImage = "String"] | | | | +- ReferenceType[@Array = false, @ArrayDepth = 0] | | | | +- ClassOrInterfaceType[@AnonymousClass = false, @Array = false, @ArrayDepth = 0, @Image = "String", @ReferenceToClassSameCompilationUnit = false] diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/Records.txt b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/Records.txt index d90f725a9e..25289146b8 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/Records.txt +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java16/Records.txt @@ -198,6 +198,9 @@ | | +- Type[@Array = false, @ArrayDepth = 0, @ArrayType = false, @TypeImage = "String"] | | | +- ReferenceType[@Array = false, @ArrayDepth = 0] | | | +- ClassOrInterfaceType[@AnonymousClass = false, @Array = false, @ArrayDepth = 0, @Image = "String", @ReferenceToClassSameCompilationUnit = false] + | | +- Annotation[@AnnotationName = "Nullable"] + | | | +- MarkerAnnotation[@AnnotationName = "Nullable"] + | | | +- Name[@Image = "Nullable"] | | +- VariableDeclaratorId[@Array = false, @ArrayDepth = 0, @ArrayType = false, @ExceptionBlockParameter = false, @ExplicitReceiverParameter = false, @Field = false, @Final = true, @ForeachVariable = false, @FormalParameter = false, @Image = "x", @LambdaParameter = false, @LocalVariable = false, @Name = "x", @PatternBinding = false, @ResourceDeclaration = false, @TypeInferred = false, @VariableName = "x"] | +- RecordBody[] +- ClassOrInterfaceBodyDeclaration[@AnonymousInnerClass = false, @EnumChild = false, @Kind = DeclarationKind.RECORD]