From 60e0a9d48ee185fa5a4c0238f3f888f4e61cfa07 Mon Sep 17 00:00:00 2001 From: Jordan Bolton Date: Wed, 14 Oct 2026 14:38:14 -0500 Subject: [PATCH 1/4] Avoid discarded primary-type speculation and bump to 3.1.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d5e5f8a-36d8-47aa-8526-9275e8355e2c --- README.md | 7 + package-lock.json | 4 +- package.json | 2 +- .../parser/parsers/naiveParseSteps.ts | 30 +++ .../parser/typeSpeculation.test.ts | 233 ++++++++++++++++++ 5 files changed, 273 insertions(+), 3 deletions(-) create mode 100644 src/test/libraryTest/parser/typeSpeculation.test.ts diff --git a/README.md b/README.md index f16405ef..030c6d38 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,13 @@ Cancellation still returns a `CommonError` wrapping `CancellationError`; polling The parser started off as a naive recursive descent parser with limited backtracking. It mostly followed the [official specification](https://docs.microsoft.com/en-us/powerquery-m/power-query-m-language-specification) released in October 2016. Deviations from the specification should be marked down in [specification.md](specification.md). A combinatorial parser has since been added which uses the naive parser as its base. +`readType` skips impossible primary-type attempts before falling back to a +primary expression. Strict primary-type reads retain their original diagnostics. +Prediction also applies with custom parser objects, cancellation tokens, and tracing. +Skipped attempts emit no traces or checkpoint calls; cancellation checks remain active. +EOF and reads without an active context retain the speculative path. +Return-type disambiguation is unchanged. + ### Style This project uses [prettier](https://github.com/prettier/prettier) as the primary source of style enforcement. Additional style requirements are located in [style.md](style.md). diff --git a/package-lock.json b/package-lock.json index 7316b79c..cbd2655c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@microsoft/powerquery-parser", - "version": "3.0.0", + "version": "3.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@microsoft/powerquery-parser", - "version": "3.0.0", + "version": "3.1.0", "license": "MIT", "dependencies": { "grapheme-splitter": "^1.0.4", diff --git a/package.json b/package.json index 17f57644..e2945332 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/powerquery-parser", - "version": "3.0.0", + "version": "3.1.0", "description": "A parser for the Power Query/M formula language.", "author": "Microsoft", "license": "MIT", diff --git a/src/powerquery-parser/parser/parsers/naiveParseSteps.ts b/src/powerquery-parser/parser/parsers/naiveParseSteps.ts index 571aab2d..6725c95f 100644 --- a/src/powerquery-parser/parser/parsers/naiveParseSteps.ts +++ b/src/powerquery-parser/parser/parsers/naiveParseSteps.ts @@ -1922,6 +1922,21 @@ export async function readType( state.cancellationToken?.throwIfCancelled(); + // Only readType discards a failed primary-type diagnostic in favor of an expression. + // Skipped attempts emit no traces or checkpoint calls; cancellation is still checked by the readers. + if ( + state.currentContextNode !== undefined && + state.tokenIndex < state.lexerSnapshot.tokens.length && + !canStartPrimaryType(state) + ) { + trace.exit({ + [NaiveTraceConstant.TokenIndex]: state.tokenIndex, + [TraceConstant.IsError]: true, + }); + + return parser.readPrimaryExpression(state, parser, trace.id); + } + const triedReadPrimaryType: TriedReadPrimaryType = await tryReadPrimaryType(state, parser, trace.id); if (ResultUtils.isOk(triedReadPrimaryType)) { @@ -2348,6 +2363,21 @@ export async function readFunctionType( return functionType; } +function canStartPrimaryType(state: ParseState): boolean { + const token: Token.Token | undefined = state.lexerSnapshot.tokens[state.tokenIndex]; + + return ( + token !== undefined && + (token.kind === TokenKind.LeftBracket || + token.kind === TokenKind.LeftBrace || + token.kind === TokenKind.KeywordType || + token.kind === TokenKind.NullLiteral || + (token.kind === TokenKind.Identifier && + (token.data === Constant.LanguageConstant.Nullable || + ConstantUtils.isPrimitiveTypeConstant(token.data)))) + ); +} + async function tryReadPrimaryType( state: ParseState, parser: Parser, diff --git a/src/test/libraryTest/parser/typeSpeculation.test.ts b/src/test/libraryTest/parser/typeSpeculation.test.ts new file mode 100644 index 00000000..ebe2e619 --- /dev/null +++ b/src/test/libraryTest/parser/typeSpeculation.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import "mocha"; +import { expect } from "chai"; +import { rejects } from "assert"; + +import { + CommonError, + CounterCancellationToken, + DefaultSettings, + NoOpCancellationToken, + Parser, + Task, + TaskUtils, +} from "../../.."; +import { Trace, TraceConstant, TraceManager } from "../../../powerquery-parser/common/trace"; +import { Ast } from "../../../powerquery-parser/language"; + +// Strict primary-type reads still perform speculation. Only an unconsumed primitive-type +// failure is discarded here, reproducing readType's fallback without using its prediction. +async function readTypeWithoutPrediction( + state: Parser.ParseState, + parser: Parser.Parser, + correlationId: number | undefined, +): Promise { + const tokenIndex: number = state.tokenIndex; + + try { + return await parser.readPrimaryType(state, parser, correlationId); + } catch (error: unknown) { + if ( + state.tokenIndex !== tokenIndex || + !( + error instanceof Parser.ParseError.InvalidPrimitiveTypeError || + error instanceof Parser.ParseError.ExpectedAnyTokenKindError + ) + ) { + throw error; + } + + return parser.readPrimaryExpression(state, parser, correlationId); + } +} + +class RecordingTraceManager extends TraceManager { + public readonly events: { trace: Trace; message: string }[] = []; + + public emit(trace: Trace, message: string): void { + this.events.push({ trace, message }); + } +} + +function newState(overrides?: Partial): Parser.ParseState { + const lex: Task.TriedLexTask = TaskUtils.tryLex(DefaultSettings, "unknown"); + TaskUtils.assertIsLexStageOk(lex); + const state: Parser.ParseState = Parser.ParseStateUtils.newState(lex.lexerSnapshot, overrides); + Parser.ParseStateUtils.startContext(state, Ast.NodeKind.ListType); + + return state; +} + +describe("Parser primary-type prediction", () => { + for (const parser of [Parser.CombinatorialParserV2, Parser.RecursiveDescentParser]) { + for (const text of [ + "let a = type unknown in a", + "let a = type (1 + 2) in a", + "let a = type @unknown in a", + "let a = type 1 in a", + "let a = type in a", + "let a = type table [x=", + "let a = type function (x as", + "let a = type function (x as number) as ", + "let a = type [x = number] in a", + "let a = type nullable number in a", + "(x as number) as nullable number => x", + ]) { + it(`preserves full parse results for ${text}`, async () => { + const actual: Task.TriedLexParseTask = await TaskUtils.tryLexParse( + { ...DefaultSettings, parser }, + text, + ); + + const expected: Task.TriedLexParseTask = await TaskUtils.tryLexParse( + { ...DefaultSettings, parser: { ...parser, readType: readTypeWithoutPrediction } }, + text, + ); + + expect(actual).to.deep.equal(expected); + + expect("nodeIdMapCollection" in actual ? actual.nodeIdMapCollection : undefined).to.deep.equal( + "nodeIdMapCollection" in expected ? expected.nodeIdMapCollection : undefined, + ); + + expect("parseState" in actual ? actual.parseState : undefined).to.deep.equal( + "parseState" in expected ? expected.parseState : undefined, + ); + }); + } + + it("skips speculation with a cancellation token and custom parser hooks", async () => { + const state: Parser.ParseState = newState({ cancellationToken: NoOpCancellationToken }); + let primaryReads: number = 0; + + const customParser: Parser.Parser = { + ...parser, + checkpoint: (): never => { + throw new Error("The skipped attempt must not create a checkpoint"); + }, + restoreCheckpoint: (): never => { + throw new Error("The skipped attempt must not restore a checkpoint"); + }, + readPrimaryExpression: async ( + state: Parser.ParseState, + parser: Parser.Parser, + correlationId: number | undefined, + ): Promise => { + primaryReads += 1; + + return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId); + }, + }; + + const result: Ast.TType = await customParser.readType(state, customParser, undefined); + expect(result.kind).to.equal(Ast.NodeKind.IdentifierExpression); + expect(primaryReads).to.equal(1); + }); + + it("traces only executed reads with matching entries, exits, and correlations", async () => { + const traceManager: RecordingTraceManager = new RecordingTraceManager(); + const state: Parser.ParseState = newState({ traceManager }); + await parser.readType(state, parser, undefined); + + const entries: Map = new Map(); + const exits: Set = new Set(); + + for (const { trace, message } of traceManager.events) { + if (message === TraceConstant.Entry) { + expect(entries.has(trace.id)).to.equal(false); + + if (trace.correlationId !== undefined) { + expect(entries.has(trace.correlationId)).to.equal(true); + } + + entries.set(trace.id, trace); + } else if (message === TraceConstant.Exit) { + expect(entries.has(trace.id)).to.equal(true); + expect(exits.has(trace.id)).to.equal(false); + exits.add(trace.id); + } + } + + const tasks: string[] = [...entries.values()].map((trace: Trace) => trace.task); + expect(tasks).to.include("readType"); + expect(tasks).to.include("readPrimaryExpression"); + expect(tasks).not.to.include("tryReadPrimaryType"); + expect(tasks).not.to.include("tryReadPrimitiveType"); + expect(exits.size).to.equal(entries.size); + }); + + it("honors cancellation before prediction", async () => { + const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(1) }); + const tokenIndex: number = state.tokenIndex; + const idCounter: number = state.contextState.idCounter; + await rejects(parser.readType(state, parser, undefined), CommonError.CancellationError); + expect(state.tokenIndex).to.equal(tokenIndex); + expect(state.contextState.idCounter).to.equal(idCounter); + }); + + it("honors cancellation when entering the primary-expression fallback", async () => { + const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(2) }); + let enteredFallback: boolean = false; + + const customParser: Parser.Parser = { + ...parser, + readPrimaryExpression: async ( + state: Parser.ParseState, + parser: Parser.Parser, + correlationId: number | undefined, + ): Promise => { + enteredFallback = true; + + return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId); + }, + }; + + await rejects(customParser.readType(state, customParser, undefined), CommonError.CancellationError); + expect(enteredFallback).to.equal(true); + expect(state.tokenIndex).to.equal(0); + }); + + it("retains strict primitive-type diagnostics and their checkpoint hooks", async () => { + let checkpoints: number = 0; + + const customParser: Parser.Parser = { + ...parser, + checkpoint: (state: Parser.ParseState): Promise => { + checkpoints += 1; + + return parser.checkpoint(state); + }, + }; + + await rejects( + customParser.readPrimaryType(newState(), customParser, undefined), + Parser.ParseError.InvalidPrimitiveTypeError, + ); + + expect(checkpoints).to.be.greaterThan(0); + }); + + it("retains EOF rollback and clears the stale cached token", async () => { + const state: Parser.ParseState = newState(); + state.tokenIndex = state.lexerSnapshot.tokens.length; + state.currentTokenKind = undefined; + expect(state.currentToken).not.to.equal(undefined); + let checkpoints: number = 0; + + const customParser: Parser.Parser = { + ...parser, + checkpoint: (state: Parser.ParseState): Promise => { + checkpoints += 1; + + return parser.checkpoint(state); + }, + }; + + await rejects(customParser.readType(state, customParser, undefined)); + expect(checkpoints).to.be.greaterThan(0); + expect(state.currentToken).to.equal(undefined); + }); + } +}); From ada734c7000f8da63a446c6614de8567edb09a14 Mon Sep 17 00:00:00 2001 From: Jordan Bolton Date: Wed, 14 Oct 2026 15:14:56 -0500 Subject: [PATCH 2/4] Explain why primary-type prediction safely skips speculation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d5e5f8a-36d8-47aa-8526-9275e8355e2c --- src/powerquery-parser/parser/parsers/naiveParseSteps.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/powerquery-parser/parser/parsers/naiveParseSteps.ts b/src/powerquery-parser/parser/parsers/naiveParseSteps.ts index 6725c95f..c3e9089c 100644 --- a/src/powerquery-parser/parser/parsers/naiveParseSteps.ts +++ b/src/powerquery-parser/parser/parsers/naiveParseSteps.ts @@ -1922,8 +1922,13 @@ export async function readType( state.cancellationToken?.throwIfCancelled(); - // Only readType discards a failed primary-type diagnostic in favor of an expression. - // Skipped attempts emit no traces or checkpoint calls; cancellation is still checked by the readers. + // canStartPrimaryType covers every prefix accepted by tryReadPrimaryType. If false, that + // reader only creates a PrimitiveType context, rejects the token without consuming it, + // and rolls the context back. readType discards that diagnostic and calls the same + // primary-expression reader below, so skipping the attempt loses no valid type or diagnostic. + // Keep speculation without an enclosing context or at EOF, where its root/cached-token + // bookkeeping is observable. Cancellation checks remain above and in the fallback; + // traces and checkpoint hooks for the discarded attempt are intentionally omitted. if ( state.currentContextNode !== undefined && state.tokenIndex < state.lexerSnapshot.tokens.length && From 7f0fe565efa4d731fe1eb8045ca930d499d54414 Mon Sep 17 00:00:00 2001 From: Jordan Bolton Date: Wed, 14 Oct 2026 17:27:14 -0500 Subject: [PATCH 3/4] Refine primary-type prediction documentation and test placement Move prediction coverage into parser behavior tests, remove the README addition, document token-to-AST mappings, and bump the package to 3.2.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d5e5f8a-36d8-47aa-8526-9275e8355e2c --- README.md | 7 - package-lock.json | 4 +- package.json | 2 +- .../parser/parsers/naiveParseSteps.ts | 9 + .../libraryTest/parser/parseBehavior.test.ts | 230 ++++++++++++++++- .../parser/typeSpeculation.test.ts | 233 ------------------ 6 files changed, 241 insertions(+), 244 deletions(-) delete mode 100644 src/test/libraryTest/parser/typeSpeculation.test.ts diff --git a/README.md b/README.md index 030c6d38..f16405ef 100644 --- a/README.md +++ b/README.md @@ -45,13 +45,6 @@ Cancellation still returns a `CommonError` wrapping `CancellationError`; polling The parser started off as a naive recursive descent parser with limited backtracking. It mostly followed the [official specification](https://docs.microsoft.com/en-us/powerquery-m/power-query-m-language-specification) released in October 2016. Deviations from the specification should be marked down in [specification.md](specification.md). A combinatorial parser has since been added which uses the naive parser as its base. -`readType` skips impossible primary-type attempts before falling back to a -primary expression. Strict primary-type reads retain their original diagnostics. -Prediction also applies with custom parser objects, cancellation tokens, and tracing. -Skipped attempts emit no traces or checkpoint calls; cancellation checks remain active. -EOF and reads without an active context retain the speculative path. -Return-type disambiguation is unchanged. - ### Style This project uses [prettier](https://github.com/prettier/prettier) as the primary source of style enforcement. Additional style requirements are located in [style.md](style.md). diff --git a/package-lock.json b/package-lock.json index cbd2655c..02a8f1db 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@microsoft/powerquery-parser", - "version": "3.1.0", + "version": "3.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@microsoft/powerquery-parser", - "version": "3.1.0", + "version": "3.2.0", "license": "MIT", "dependencies": { "grapheme-splitter": "^1.0.4", diff --git a/package.json b/package.json index e2945332..74d6222e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/powerquery-parser", - "version": "3.1.0", + "version": "3.2.0", "description": "A parser for the Power Query/M formula language.", "author": "Microsoft", "license": "MIT", diff --git a/src/powerquery-parser/parser/parsers/naiveParseSteps.ts b/src/powerquery-parser/parser/parsers/naiveParseSteps.ts index c3e9089c..c47d24f1 100644 --- a/src/powerquery-parser/parser/parsers/naiveParseSteps.ts +++ b/src/powerquery-parser/parser/parsers/naiveParseSteps.ts @@ -2371,6 +2371,15 @@ export async function readFunctionType( function canStartPrimaryType(state: ParseState): boolean { const token: Token.Token | undefined = state.lexerSnapshot.tokens[state.tokenIndex]; + // No token means no primary-type prefix. The token checks correspond to: + // LeftBracket: Ast.RecordType, such as [x = number]. + // LeftBrace: Ast.ListType, such as {number}. + // KeywordType: Ast.PrimitiveType with primitive kind Type, not Ast.TypePrimaryType. + // NullLiteral: Ast.PrimitiveType with primitive kind Null, not Ast.LiteralExpression. + // Identifier "nullable": Ast.NullableType, such as nullable number. + // Other primitive-type identifiers: Ast.PrimitiveType (number, text, etc.). + // "table" and "function" also admit Ast.TableType and Ast.FunctionType; + // tryReadPrimaryType uses subsequent tokens to distinguish these from primitive types. return ( token !== undefined && (token.kind === TokenKind.LeftBracket || diff --git a/src/test/libraryTest/parser/parseBehavior.test.ts b/src/test/libraryTest/parser/parseBehavior.test.ts index fc22bac1..2c35cb05 100644 --- a/src/test/libraryTest/parser/parseBehavior.test.ts +++ b/src/test/libraryTest/parser/parseBehavior.test.ts @@ -3,10 +3,23 @@ import "mocha"; import { expect } from "chai"; +import { rejects } from "assert"; import * as ParserTestUtils from "./parserTestUtils"; -import { Assert, DefaultSettings, Language, Task, TaskUtils } from "../../../powerquery-parser"; +import { + Assert, + CommonError, + CounterCancellationToken, + DefaultSettings, + Language, + NoOpCancellationToken, + Parser, + Task, + TaskUtils, +} from "../../../powerquery-parser"; +import { Trace, TraceConstant, TraceManager } from "../../../powerquery-parser/common/trace"; import { AssertTestUtils } from "../../testUtils"; +import { Ast } from "../../../powerquery-parser/language"; import { NodeKind } from "../../../powerquery-parser/language/ast/ast"; import { ParseBehavior } from "../../../powerquery-parser/parser/parseBehavior"; import { ParseError } from "../../../powerquery-parser/parser"; @@ -205,3 +218,218 @@ in ).to.deep.equal(["text", "number"]); }); }); + +// Strict primary-type reads still perform speculation. Only an unconsumed primitive-type +// failure is discarded here, reproducing readType's fallback without using its prediction. +async function readTypeWithoutPrediction( + state: Parser.ParseState, + parser: Parser.Parser, + correlationId: number | undefined, +): Promise { + const tokenIndex: number = state.tokenIndex; + + try { + return await parser.readPrimaryType(state, parser, correlationId); + } catch (error: unknown) { + if ( + state.tokenIndex !== tokenIndex || + !( + error instanceof Parser.ParseError.InvalidPrimitiveTypeError || + error instanceof Parser.ParseError.ExpectedAnyTokenKindError + ) + ) { + throw error; + } + + return parser.readPrimaryExpression(state, parser, correlationId); + } +} + +class RecordingTraceManager extends TraceManager { + public readonly events: { trace: Trace; message: string }[] = []; + + public emit(trace: Trace, message: string): void { + this.events.push({ trace, message }); + } +} + +function newState(overrides?: Partial): Parser.ParseState { + const lex: Task.TriedLexTask = TaskUtils.tryLex(DefaultSettings, "unknown"); + TaskUtils.assertIsLexStageOk(lex); + const state: Parser.ParseState = Parser.ParseStateUtils.newState(lex.lexerSnapshot, overrides); + Parser.ParseStateUtils.startContext(state, Ast.NodeKind.ListType); + + return state; +} + +describe("Parser primary-type prediction", () => { + for (const parser of [Parser.CombinatorialParserV2, Parser.RecursiveDescentParser]) { + for (const text of [ + "let a = type unknown in a", + "let a = type (1 + 2) in a", + "let a = type @unknown in a", + "let a = type 1 in a", + "let a = type in a", + "let a = type table [x=", + "let a = type function (x as", + "let a = type function (x as number) as ", + "let a = type [x = number] in a", + "let a = type nullable number in a", + "(x as number) as nullable number => x", + ]) { + it(`preserves full parse results for ${text}`, async () => { + const actual: Task.TriedLexParseTask = await TaskUtils.tryLexParse( + { ...DefaultSettings, parser }, + text, + ); + + const expected: Task.TriedLexParseTask = await TaskUtils.tryLexParse( + { ...DefaultSettings, parser: { ...parser, readType: readTypeWithoutPrediction } }, + text, + ); + + expect(actual).to.deep.equal(expected); + + expect("nodeIdMapCollection" in actual ? actual.nodeIdMapCollection : undefined).to.deep.equal( + "nodeIdMapCollection" in expected ? expected.nodeIdMapCollection : undefined, + ); + + expect("parseState" in actual ? actual.parseState : undefined).to.deep.equal( + "parseState" in expected ? expected.parseState : undefined, + ); + }); + } + + it("skips speculation with a cancellation token and custom parser hooks", async () => { + const state: Parser.ParseState = newState({ cancellationToken: NoOpCancellationToken }); + let primaryReads: number = 0; + + const customParser: Parser.Parser = { + ...parser, + checkpoint: (): never => { + throw new Error("The skipped attempt must not create a checkpoint"); + }, + restoreCheckpoint: (): never => { + throw new Error("The skipped attempt must not restore a checkpoint"); + }, + readPrimaryExpression: async ( + state: Parser.ParseState, + parser: Parser.Parser, + correlationId: number | undefined, + ): Promise => { + primaryReads += 1; + + return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId); + }, + }; + + const result: Ast.TType = await customParser.readType(state, customParser, undefined); + expect(result.kind).to.equal(Ast.NodeKind.IdentifierExpression); + expect(primaryReads).to.equal(1); + }); + + it("traces only executed reads with matching entries, exits, and correlations", async () => { + const traceManager: RecordingTraceManager = new RecordingTraceManager(); + const state: Parser.ParseState = newState({ traceManager }); + await parser.readType(state, parser, undefined); + + const entries: Map = new Map(); + const exits: Set = new Set(); + + for (const { trace, message } of traceManager.events) { + if (message === TraceConstant.Entry) { + expect(entries.has(trace.id)).to.equal(false); + + if (trace.correlationId !== undefined) { + expect(entries.has(trace.correlationId)).to.equal(true); + } + + entries.set(trace.id, trace); + } else if (message === TraceConstant.Exit) { + expect(entries.has(trace.id)).to.equal(true); + expect(exits.has(trace.id)).to.equal(false); + exits.add(trace.id); + } + } + + const tasks: string[] = [...entries.values()].map((trace: Trace) => trace.task); + expect(tasks).to.include("readType"); + expect(tasks).to.include("readPrimaryExpression"); + expect(tasks).not.to.include("tryReadPrimaryType"); + expect(tasks).not.to.include("tryReadPrimitiveType"); + expect(exits.size).to.equal(entries.size); + }); + + it("honors cancellation before prediction", async () => { + const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(1) }); + const tokenIndex: number = state.tokenIndex; + const idCounter: number = state.contextState.idCounter; + await rejects(parser.readType(state, parser, undefined), CommonError.CancellationError); + expect(state.tokenIndex).to.equal(tokenIndex); + expect(state.contextState.idCounter).to.equal(idCounter); + }); + + it("honors cancellation when entering the primary-expression fallback", async () => { + const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(2) }); + let enteredFallback: boolean = false; + + const customParser: Parser.Parser = { + ...parser, + readPrimaryExpression: async ( + state: Parser.ParseState, + parser: Parser.Parser, + correlationId: number | undefined, + ): Promise => { + enteredFallback = true; + + return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId); + }, + }; + + await rejects(customParser.readType(state, customParser, undefined), CommonError.CancellationError); + expect(enteredFallback).to.equal(true); + expect(state.tokenIndex).to.equal(0); + }); + + it("retains strict primitive-type diagnostics and their checkpoint hooks", async () => { + let checkpoints: number = 0; + + const customParser: Parser.Parser = { + ...parser, + checkpoint: (state: Parser.ParseState): Promise => { + checkpoints += 1; + + return parser.checkpoint(state); + }, + }; + + await rejects( + customParser.readPrimaryType(newState(), customParser, undefined), + Parser.ParseError.InvalidPrimitiveTypeError, + ); + + expect(checkpoints).to.be.greaterThan(0); + }); + + it("retains EOF rollback and clears the stale cached token", async () => { + const state: Parser.ParseState = newState(); + state.tokenIndex = state.lexerSnapshot.tokens.length; + state.currentTokenKind = undefined; + expect(state.currentToken).not.to.equal(undefined); + let checkpoints: number = 0; + + const customParser: Parser.Parser = { + ...parser, + checkpoint: (state: Parser.ParseState): Promise => { + checkpoints += 1; + + return parser.checkpoint(state); + }, + }; + + await rejects(customParser.readType(state, customParser, undefined)); + expect(checkpoints).to.be.greaterThan(0); + expect(state.currentToken).to.equal(undefined); + }); + } +}); diff --git a/src/test/libraryTest/parser/typeSpeculation.test.ts b/src/test/libraryTest/parser/typeSpeculation.test.ts deleted file mode 100644 index ebe2e619..00000000 --- a/src/test/libraryTest/parser/typeSpeculation.test.ts +++ /dev/null @@ -1,233 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import "mocha"; -import { expect } from "chai"; -import { rejects } from "assert"; - -import { - CommonError, - CounterCancellationToken, - DefaultSettings, - NoOpCancellationToken, - Parser, - Task, - TaskUtils, -} from "../../.."; -import { Trace, TraceConstant, TraceManager } from "../../../powerquery-parser/common/trace"; -import { Ast } from "../../../powerquery-parser/language"; - -// Strict primary-type reads still perform speculation. Only an unconsumed primitive-type -// failure is discarded here, reproducing readType's fallback without using its prediction. -async function readTypeWithoutPrediction( - state: Parser.ParseState, - parser: Parser.Parser, - correlationId: number | undefined, -): Promise { - const tokenIndex: number = state.tokenIndex; - - try { - return await parser.readPrimaryType(state, parser, correlationId); - } catch (error: unknown) { - if ( - state.tokenIndex !== tokenIndex || - !( - error instanceof Parser.ParseError.InvalidPrimitiveTypeError || - error instanceof Parser.ParseError.ExpectedAnyTokenKindError - ) - ) { - throw error; - } - - return parser.readPrimaryExpression(state, parser, correlationId); - } -} - -class RecordingTraceManager extends TraceManager { - public readonly events: { trace: Trace; message: string }[] = []; - - public emit(trace: Trace, message: string): void { - this.events.push({ trace, message }); - } -} - -function newState(overrides?: Partial): Parser.ParseState { - const lex: Task.TriedLexTask = TaskUtils.tryLex(DefaultSettings, "unknown"); - TaskUtils.assertIsLexStageOk(lex); - const state: Parser.ParseState = Parser.ParseStateUtils.newState(lex.lexerSnapshot, overrides); - Parser.ParseStateUtils.startContext(state, Ast.NodeKind.ListType); - - return state; -} - -describe("Parser primary-type prediction", () => { - for (const parser of [Parser.CombinatorialParserV2, Parser.RecursiveDescentParser]) { - for (const text of [ - "let a = type unknown in a", - "let a = type (1 + 2) in a", - "let a = type @unknown in a", - "let a = type 1 in a", - "let a = type in a", - "let a = type table [x=", - "let a = type function (x as", - "let a = type function (x as number) as ", - "let a = type [x = number] in a", - "let a = type nullable number in a", - "(x as number) as nullable number => x", - ]) { - it(`preserves full parse results for ${text}`, async () => { - const actual: Task.TriedLexParseTask = await TaskUtils.tryLexParse( - { ...DefaultSettings, parser }, - text, - ); - - const expected: Task.TriedLexParseTask = await TaskUtils.tryLexParse( - { ...DefaultSettings, parser: { ...parser, readType: readTypeWithoutPrediction } }, - text, - ); - - expect(actual).to.deep.equal(expected); - - expect("nodeIdMapCollection" in actual ? actual.nodeIdMapCollection : undefined).to.deep.equal( - "nodeIdMapCollection" in expected ? expected.nodeIdMapCollection : undefined, - ); - - expect("parseState" in actual ? actual.parseState : undefined).to.deep.equal( - "parseState" in expected ? expected.parseState : undefined, - ); - }); - } - - it("skips speculation with a cancellation token and custom parser hooks", async () => { - const state: Parser.ParseState = newState({ cancellationToken: NoOpCancellationToken }); - let primaryReads: number = 0; - - const customParser: Parser.Parser = { - ...parser, - checkpoint: (): never => { - throw new Error("The skipped attempt must not create a checkpoint"); - }, - restoreCheckpoint: (): never => { - throw new Error("The skipped attempt must not restore a checkpoint"); - }, - readPrimaryExpression: async ( - state: Parser.ParseState, - parser: Parser.Parser, - correlationId: number | undefined, - ): Promise => { - primaryReads += 1; - - return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId); - }, - }; - - const result: Ast.TType = await customParser.readType(state, customParser, undefined); - expect(result.kind).to.equal(Ast.NodeKind.IdentifierExpression); - expect(primaryReads).to.equal(1); - }); - - it("traces only executed reads with matching entries, exits, and correlations", async () => { - const traceManager: RecordingTraceManager = new RecordingTraceManager(); - const state: Parser.ParseState = newState({ traceManager }); - await parser.readType(state, parser, undefined); - - const entries: Map = new Map(); - const exits: Set = new Set(); - - for (const { trace, message } of traceManager.events) { - if (message === TraceConstant.Entry) { - expect(entries.has(trace.id)).to.equal(false); - - if (trace.correlationId !== undefined) { - expect(entries.has(trace.correlationId)).to.equal(true); - } - - entries.set(trace.id, trace); - } else if (message === TraceConstant.Exit) { - expect(entries.has(trace.id)).to.equal(true); - expect(exits.has(trace.id)).to.equal(false); - exits.add(trace.id); - } - } - - const tasks: string[] = [...entries.values()].map((trace: Trace) => trace.task); - expect(tasks).to.include("readType"); - expect(tasks).to.include("readPrimaryExpression"); - expect(tasks).not.to.include("tryReadPrimaryType"); - expect(tasks).not.to.include("tryReadPrimitiveType"); - expect(exits.size).to.equal(entries.size); - }); - - it("honors cancellation before prediction", async () => { - const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(1) }); - const tokenIndex: number = state.tokenIndex; - const idCounter: number = state.contextState.idCounter; - await rejects(parser.readType(state, parser, undefined), CommonError.CancellationError); - expect(state.tokenIndex).to.equal(tokenIndex); - expect(state.contextState.idCounter).to.equal(idCounter); - }); - - it("honors cancellation when entering the primary-expression fallback", async () => { - const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(2) }); - let enteredFallback: boolean = false; - - const customParser: Parser.Parser = { - ...parser, - readPrimaryExpression: async ( - state: Parser.ParseState, - parser: Parser.Parser, - correlationId: number | undefined, - ): Promise => { - enteredFallback = true; - - return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId); - }, - }; - - await rejects(customParser.readType(state, customParser, undefined), CommonError.CancellationError); - expect(enteredFallback).to.equal(true); - expect(state.tokenIndex).to.equal(0); - }); - - it("retains strict primitive-type diagnostics and their checkpoint hooks", async () => { - let checkpoints: number = 0; - - const customParser: Parser.Parser = { - ...parser, - checkpoint: (state: Parser.ParseState): Promise => { - checkpoints += 1; - - return parser.checkpoint(state); - }, - }; - - await rejects( - customParser.readPrimaryType(newState(), customParser, undefined), - Parser.ParseError.InvalidPrimitiveTypeError, - ); - - expect(checkpoints).to.be.greaterThan(0); - }); - - it("retains EOF rollback and clears the stale cached token", async () => { - const state: Parser.ParseState = newState(); - state.tokenIndex = state.lexerSnapshot.tokens.length; - state.currentTokenKind = undefined; - expect(state.currentToken).not.to.equal(undefined); - let checkpoints: number = 0; - - const customParser: Parser.Parser = { - ...parser, - checkpoint: (state: Parser.ParseState): Promise => { - checkpoints += 1; - - return parser.checkpoint(state); - }, - }; - - await rejects(customParser.readType(state, customParser, undefined)); - expect(checkpoints).to.be.greaterThan(0); - expect(state.currentToken).to.equal(undefined); - }); - } -}); From 6379644e6a7e3ba75285afe9bc4fc133c77efeae Mon Sep 17 00:00:00 2001 From: "Jordan Bolton (jobolton)" Date: Thu, 15 Oct 2026 15:27:12 -0500 Subject: [PATCH 4/4] Remove package version changes from parser speculation fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bfa84c62-b343-4449-a0d6-ecfdecea16f1 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index c54dcb3a..cbd2655c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@microsoft/powerquery-parser", - "version": "3.3.0", + "version": "3.1.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@microsoft/powerquery-parser", - "version": "3.3.0", + "version": "3.1.0", "license": "MIT", "dependencies": { "grapheme-splitter": "^1.0.4", diff --git a/package.json b/package.json index c9098481..e2945332 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@microsoft/powerquery-parser", - "version": "3.3.0", + "version": "3.1.0", "description": "A parser for the Power Query/M formula language.", "author": "Microsoft", "license": "MIT",