[PR #1328] [MERGED] fix newline checks in parser #1020

New Issue

2025-12-30T01:28:19+01:00

adam commented

2025-12-30 01:28:19 +01:00

📋 Pull Request Information

Original PR: https://github.com/apple/pkl/pull/1328
Author: @spyoungtech
Created: 11/20/2025
Status: ✅ Merged
Merged: 11/24/2025
Merged by: @stackoverflow

Base: main ← Head: fix-parsing-newline-checks

📝 Commits (1)

d76b1cb fix newline checks in parser

📊 Changes

4 files changed (+15 additions, -1 deletions)

View changed files

➕ pkl-core/src/test/files/LanguageSnippetTests/input/errors/binopDifferentLine.pkl (+5 -0)
➕ pkl-core/src/test/files/LanguageSnippetTests/output/errors/binopDifferentLine.err (+6 -0)
📝 pkl-parser/src/main/java/org/pkl/parser/Parser.java (+3 -1)
📝 pkl-parser/src/test/kotlin/org/pkl/parser/ParserComparisonTest.kt (+1 -0)

📄 Description

In places where the parser tries to check or enforce that productions occur on the same line, 'affix' tokens present on subsequent lines, when followed by the expected/enforced production can confuse the parser. For example, this code is rejected by the old ANT parser, but accepted with the new parser in 0.28+:

// block comments help you violate "same line" rules 
// such as for type constraints, subscription, binary expressions etc
binresult: Int = 1

/*comment*/ - 

/*comment*/ 1

Execution details

In versions <0.28:

–– Pkl Error ––
Extraneous input: `-`. Expected one of: <EOF>, `abstract`, `class`, `const`, `external`, `fixed`, `function`, `hidden`, `local`, `open`, `typealias`, `@`, Identifier, DocComment

3 | /*comment*/-
               ^
at binopDifferentLine (file:///path/to/binopDifferentLine.pkl, line 3)

In pkl 0.28+ this succeeds erroneously:

binresult: 0

This is because of how the lexer (re)sets newLinesBetween when it encounters new tokens, including those often ignored by the parser. When those ignored tokens (specifically line comments, block comments, and semicolons) are encountered in, the count is reset and checks like lookahead.newLinesBetween == 0 can fail to assert the necessary/intended state.

Because checks on newLinesBetween generally are for ==0 and coincide with !precededBySemicolon in practice, I think only block comments can cause this issue, even though semicolons and line comments also reset the count.

This may also have an impact on how expect logically arrives at the error location in which to point.

The proposed fix modifies forceNext to keep track of newlines between tokens, including any newlines between tokens that are skipped, and accumulating the total into the presented FullToken.

This may also be needed for forceNextComment but not sure.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/apple/pkl/pull/1328 **Author:** [@spyoungtech](https://github.com/spyoungtech) **Created:** 11/20/2025 **Status:** ✅ Merged **Merged:** 11/24/2025 **Merged by:** [@stackoverflow](https://github.com/stackoverflow) **Base:** `main` ← **Head:** `fix-parsing-newline-checks` --- ### 📝 Commits (1) - [`d76b1cb`](https://github.com/apple/pkl/commit/d76b1cbeb9ce75f1da6d643c3b5e6194640bdb34) fix newline checks in parser ### 📊 Changes **4 files changed** (+15 additions, -1 deletions) <details> <summary>View changed files</summary> ➕ `pkl-core/src/test/files/LanguageSnippetTests/input/errors/binopDifferentLine.pkl` (+5 -0) ➕ `pkl-core/src/test/files/LanguageSnippetTests/output/errors/binopDifferentLine.err` (+6 -0) 📝 `pkl-parser/src/main/java/org/pkl/parser/Parser.java` (+3 -1) 📝 `pkl-parser/src/test/kotlin/org/pkl/parser/ParserComparisonTest.kt` (+1 -0) </details> ### 📄 Description In places where the parser tries to check or enforce that productions occur on the same line, 'affix' tokens present on subsequent lines, when followed by the expected/enforced production can confuse the parser. For example, this code is rejected by the old ANT parser, but accepted with the new parser in 0.28+: ```pkl // block comments help you violate "same line" rules // such as for type constraints, subscription, binary expressions etc binresult: Int = 1 /*comment*/ - /*comment*/ 1 ``` <details><summary>Execution details</summary> In versions <0.28: ```pkl –– Pkl Error –– Extraneous input: `-`. Expected one of: <EOF>, `abstract`, `class`, `const`, `external`, `fixed`, `function`, `hidden`, `local`, `open`, `typealias`, `@`, Identifier, DocComment 3 | /*comment*/- ^ at binopDifferentLine (file:///path/to/binopDifferentLine.pkl, line 3) ``` In pkl 0.28+ this succeeds erroneously: ```yaml binresult: 0 ``` </details> This is because of how the lexer (re)sets `newLinesBetween` when it encounters new tokens, including those often ignored by the parser. When those ignored tokens (specifically line comments, block comments, and semicolons) are encountered in, the count is reset and checks like `lookahead.newLinesBetween == 0` can fail to assert the necessary/intended state. Because checks on newLinesBetween generally are for `==0` and coincide with `!precededBySemicolon` in practice, I think only block comments can cause this issue, even though semicolons and line comments also reset the count. This _may_ also have an impact on how `expect` logically arrives at the error location in which to point. The proposed fix modifies `forceNext` to keep track of newlines between tokens, including any newlines between tokens that are skipped, and accumulating the total into the presented `FullToken`. This may also be needed for `forceNextComment` but not sure. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

adam added the pull-request label 2025-12-30 01:28:19 +01:00

adam closed this issue

2025-12-30 01:28:19 +01:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/pkl#1020