From 875fac857c10fbd553cd56158ae8a8f62516df92 Mon Sep 17 00:00:00 2001 From: Daniel Chao Date: Tue, 3 Nov 2026 16:31:40 -0800 Subject: [PATCH] Generic parser fixes (#1959) This fixes some issues with the handling of separators in GenericParser --- .../org/pkl/parser/GenericParserImpl.java | 44 ++++---- .../org/pkl/parser/GenericParserTest.kt | 100 ++++++++++++++++++ .../org/pkl/parser/ParserComparisonTest.kt | 7 -- .../test/kotlin/org/pkl/parser/TestUtils.kt | 23 ++++ 4 files changed, 146 insertions(+), 28 deletions(-) create mode 100644 pkl-parser/src/test/kotlin/org/pkl/parser/TestUtils.kt diff --git a/pkl-parser/src/main/java/org/pkl/parser/GenericParserImpl.java b/pkl-parser/src/main/java/org/pkl/parser/GenericParserImpl.java index e0dd53ac2..ab633e84f 100644 --- a/pkl-parser/src/main/java/org/pkl/parser/GenericParserImpl.java +++ b/pkl-parser/src/main/java/org/pkl/parser/GenericParserImpl.java @@ -755,37 +755,43 @@ class GenericParserImpl { private void parseAccess(List children) { children.add(parseIdentifier()); - var typeArgs = tryParseTypeArgumentList(); + var typeArgs = tryParseTypeArgumentList(children); if (typeArgs != null) { // `tryParseTypeArgumentList` guarantees that an argument list follows children.add(typeArgs); ff(children); children.add(parseArgumentList()); - } else if (lookahead() == Token.LPAREN - && noSemicolonInbetween() - && _lookahead.newLinesBetween == 0) { - ff(children); - children.add(parseArgumentList()); + } else { + var fullLookahead = fullLookahead(); + if (fullLookahead.tk.token == Token.LPAREN + && !fullLookahead.hasSemicolon + && fullLookahead.tk.newLinesBetween == 0) { + ff(children); + children.add(parseArgumentList()); + } } } - private @Nullable Node tryParseTypeArgumentList() { - if (lookahead() != Token.LT - || !noSemicolonInbetween() - || _lookahead.newLinesBetween != 0 - || failedTypeArgAttempts.contains(spanLookahead.charIndex()) - || !isTypeArgCandidate(spanLookahead.charIndex())) { + private @Nullable Node tryParseTypeArgumentList(List children) { + var fullLookahead = fullLookahead(); + if (fullLookahead.tk.token != Token.LT + || fullLookahead.hasSemicolon + || fullLookahead.tk.newLinesBetween != 0 + || failedTypeArgAttempts.contains(fullLookahead.tk.span.charIndex()) + || !isTypeArgCandidate(fullLookahead.tk.span.charIndex())) { return null; } var savedCursor = cursor; - var savedCharIndex = spanLookahead.charIndex(); + var savedCharIndex = fullLookahead.tk.span.charIndex(); var savedIsSpeculativeParse = isSpeculativeParse; isSpeculativeParse = true; try { + var affixes = new ArrayList(); + ff(affixes); var typeArgs = parseTypeArgumentList(); - if (lookahead() == Token.LPAREN - && noSemicolonInbetween() - && _lookahead.newLinesBetween == 0) { + var fla = fullLookahead(); + if (fla.tk.token == Token.LPAREN && !fla.hasSemicolon && fla.tk.newLinesBetween == 0) { + children.addAll(affixes); return typeArgs; } } catch (GenericParserError ignored) { @@ -1310,7 +1316,7 @@ class GenericParserImpl { } // constrained types: have to start in the same line as the type var fla = fullLookahead(); - if (fla.tk.token == Token.LPAREN && noSemicolonInbetween() && fla.tk.newLinesBetween == 0) { + if (fla.tk.token == Token.LPAREN && !fla.hasSemicolon && fla.tk.newLinesBetween == 0) { ff(children); var constraint = new ArrayList(); constraint.add(makeTerminal(next())); @@ -1565,10 +1571,6 @@ class GenericParserImpl { return tmp; } - private boolean noSemicolonInbetween() { - return tokens.get(cursor - 1).token != Token.SEMICOLON; - } - private void backtrackTo(int point) { cursor = point; var tmp = tokens.get(cursor); diff --git a/pkl-parser/src/test/kotlin/org/pkl/parser/GenericParserTest.kt b/pkl-parser/src/test/kotlin/org/pkl/parser/GenericParserTest.kt index 9cffde9bf..9f1bb8986 100644 --- a/pkl-parser/src/test/kotlin/org/pkl/parser/GenericParserTest.kt +++ b/pkl-parser/src/test/kotlin/org/pkl/parser/GenericParserTest.kt @@ -45,4 +45,104 @@ class GenericParserTest { // 1 second should still catch if we see exponential regressions assertTimeoutPreemptively(Duration.ofSeconds(1)) { GenericParser().parseModule(code) } } + + // TODO: move these into ParserSnippetTests + @Test + fun `parse separator correctly -- semicolon`() { + val code = + """ + result = new Listing { + a; + (b) { c = 1 } + } + """ + .trimIndent() + assertThat(renderGenericCode(code)) + .isEqualTo( + """ + (module + (classProperty + (identifier) + (newExpr + (declaredType + (qualifiedIdentifier + (identifier))) + (objectBody + (objectElement + (unqualifiedAccessExpr + (identifier))) + (objectElement + (amendsExpr + (parenthesizedExpr + (unqualifiedAccessExpr + (identifier))) + (objectBody + (objectProperty + (identifier) + (intLiteralExpr))))))))) + """ + .trimIndent() + ) + } + + @Test + fun `parse separator correctly -- line comment in between`() { + val code = + """ + result = new Listing { + a // some comment + (b) { c = 1 } + } + """ + .trimIndent() + assertThat(renderGenericCode(code)) + .isEqualTo( + """ + (module + (classProperty + (identifier) + (newExpr + (declaredType + (qualifiedIdentifier + (identifier))) + (objectBody + (objectElement + (unqualifiedAccessExpr + (identifier))) + (objectElement + (amendsExpr + (parenthesizedExpr + (unqualifiedAccessExpr + (identifier))) + (objectBody + (objectProperty + (identifier) + (intLiteralExpr))))))))) + """ + .trimIndent() + ) + } + + @Test + fun `parse type arg list -- comment in between`() { + val code = "foo = a /* some comment */ (c)" + assertThat(renderGenericCode(code)) + .isEqualTo( + """ + (module + (classProperty + (identifier) + (unqualifiedAccessExpr + (identifier) + (typeArgumentList + (declaredType + (qualifiedIdentifier + (identifier)))) + (argumentList + (unqualifiedAccessExpr + (identifier)))))) + """ + .trimIndent() + ) + } } diff --git a/pkl-parser/src/test/kotlin/org/pkl/parser/ParserComparisonTest.kt b/pkl-parser/src/test/kotlin/org/pkl/parser/ParserComparisonTest.kt index 10781aaeb..29f2a71c3 100644 --- a/pkl-parser/src/test/kotlin/org/pkl/parser/ParserComparisonTest.kt +++ b/pkl-parser/src/test/kotlin/org/pkl/parser/ParserComparisonTest.kt @@ -64,13 +64,6 @@ class ParserComparisonTest { return renderer.render(mod) } - private fun renderGenericCode(code: String): String { - val parser = GenericParser() - val mod = parser.parseModule(code) - val renderer = GenericSexpRenderer(code) - return renderer.render(mod) - } - // tests that are not syntactically valid Pkl private val exceptions = setOf( diff --git a/pkl-parser/src/test/kotlin/org/pkl/parser/TestUtils.kt b/pkl-parser/src/test/kotlin/org/pkl/parser/TestUtils.kt new file mode 100644 index 000000000..9a148b7ca --- /dev/null +++ b/pkl-parser/src/test/kotlin/org/pkl/parser/TestUtils.kt @@ -0,0 +1,23 @@ +/* + * Copyright © 2026 Apple Inc. and the Pkl project authors. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.pkl.parser + +fun renderGenericCode(code: String): String { + val parser = GenericParser() + val mod = parser.parseModule(code) + val renderer = GenericSexpRenderer(code) + return renderer.render(mod) +}