From cf43a6f18e38ed3e81e839d5fa6fbaf3881d007d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 22 Jul 2019 12:31:59 +0200 Subject: [PATCH] Fix text bounds of parenthesized expressions --- pmd-java/etc/grammar/Java.jjt | 11 ++++++++-- .../pmd/lang/java/ast/KotlinTestingDsl.kt | 6 ++++++ .../pmd/lang/java/ast/ParenthesesTest.kt | 21 ++++++++++++++++++- .../pmd/lang/java/ast/TestExtensions.kt | 17 +++++++++++---- 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 97a1e57eac..2df3351ade 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2540,7 +2540,7 @@ void PrimaryExpression() #void : Expressions that may be present at the start of a primary expression. */ void PrimaryPrefix() #void : -{Token t;} +{Token savedStart;} { Literal() | "this" #ThisExpression @@ -2560,7 +2560,14 @@ void PrimaryPrefix() #void : | LOOKAHEAD(LambdaLahead()) LambdaExpression() -| "(" Expression() ")" { AstImplUtil.bumpParenDepth((ASTExpression) jjtree.peekNode()); } +// Parenthesized expr, we need to adjust start/end tokens +| "(" {savedStart = getToken(0);} Expression() ")" + { + AstImplUtil.bumpParenDepth((ASTExpression) jjtree.peekNode()); + AbstractJavaNode top = (AbstractJavaNode) jjtree.peekNode(); + top.jjtSetFirstToken(savedStart); + top.jjtSetLastToken(getToken(0)); + } // If it is an ambiguous name, then we may go on // into the next step, which is less restricted // than PrimarySuffix diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/KotlinTestingDsl.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/KotlinTestingDsl.kt index 5cb63c5d4f..8366e28e37 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/KotlinTestingDsl.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/KotlinTestingDsl.kt @@ -76,6 +76,7 @@ object CustomTreePrinter : KotlintestBeanTreePrinter(NodeTreeLikeAdapter) } +// invariants that should be preserved always private val javaImplicitAssertions: Assertions = { DefaultMatchingConfig.implicitAssertions(it) @@ -86,6 +87,11 @@ private val javaImplicitAssertions: Assertions = { it::isBooleanLiteral shouldBe (it is ASTBooleanLiteral) it::isNullLiteral shouldBe (it is ASTNullLiteral) } + + if (it is ASTExpression) run { + it::isParenthesized shouldBe (it.parenthesisDepth > 0) + } + } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParenthesesTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParenthesesTest.kt index e267aef060..379ab801fb 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParenthesesTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParenthesesTest.kt @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.ast +import io.kotlintest.shouldBe import net.sourceforge.pmd.lang.ast.test.shouldBe import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType.INT import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.StatementParsingCtx @@ -46,6 +47,8 @@ class ParenthesesTest : ParserTestSpec({ it::getInitializer shouldBe int(3) { it::getParenthesisDepth shouldBe 2 it::isParenthesized shouldBe true + + it.tokenList().map { it.image } shouldBe listOf("(", "(", "3", ")", ")") } } } @@ -60,6 +63,8 @@ class ParenthesesTest : ParserTestSpec({ it::getLhsExpression shouldBe variableRef("a") { it::getParenthesisDepth shouldBe 2 it::isParenthesized shouldBe true + + it.tokenList().map { it.image } shouldBe listOf("(", "(", "a", ")", ")") } } } @@ -71,9 +76,13 @@ class ParenthesesTest : ParserTestSpec({ it::getParenthesisDepth shouldBe 1 it::isParenthesized shouldBe true + it.tokenList().map { it.image } shouldBe listOf("(", "(", "a", ")", ".", "f", ")") + it::getLhsExpression shouldBe variableRef("a") { it::getParenthesisDepth shouldBe 1 it::isParenthesized shouldBe true + + it.tokenList().map { it.image } shouldBe listOf("(", "a", ")") } } } @@ -87,15 +96,25 @@ class ParenthesesTest : ParserTestSpec({ it::getParenthesisDepth shouldBe 1 it::isParenthesized shouldBe true + it.tokenList().map { it.image } shouldBe + listOf("(", "(", "1", "+", "2", ")", "+", "f", ")") + additiveExpr(BinaryOp.ADD) { it::getParenthesisDepth shouldBe 1 it::isParenthesized shouldBe true + + it.tokenList().map { it.image } shouldBe + listOf("(", "1", "+", "2", ")") + int(1) int(2) } - variableRef("f") + variableRef("f") { + it::isParenthesized shouldBe false + it::getParenthesisDepth shouldBe 0 + } } } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt index bdb729ce3b..cc48bb0fac 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt @@ -1,11 +1,9 @@ package net.sourceforge.pmd.lang.java.ast import com.github.oowekyala.treeutils.matchers.TreeNodeWrapper +import net.sourceforge.pmd.lang.ast.GenericToken import net.sourceforge.pmd.lang.ast.Node -import net.sourceforge.pmd.lang.ast.test.NodeSpec -import net.sourceforge.pmd.lang.ast.test.ValuedNodeSpec -import net.sourceforge.pmd.lang.ast.test.shouldBe -import net.sourceforge.pmd.lang.ast.test.shouldMatch +import net.sourceforge.pmd.lang.ast.test.* import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType.* import java.util.* import kotlin.reflect.KCallable @@ -28,6 +26,17 @@ infix fun KCallable>.shouldBePresent(any: U) = this shoul ::get shouldBe any } +fun JavaNode.tokenList(): List { + val lst = mutableListOf() + var t = firstToken + lst += t + while (t != lastToken) { + t = t.next + lst += t + } + return lst +} + fun String.addArticle() = when (this[0].toLowerCase()) { 'a', 'e', 'i', 'o', 'u' -> "an $this" else -> "a $this"