Skip to content

Commit 60e0a9d

Browse files
Jordan BoltonCopilot
andcommitted
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
1 parent fbdc9ba commit 60e0a9d

5 files changed

Lines changed: 273 additions & 3 deletions

File tree

‎README.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ Cancellation still returns a `CommonError` wrapping `CancellationError`; polling
4545

4646
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.
4747

48+
`readType` skips impossible primary-type attempts before falling back to a
49+
primary expression. Strict primary-type reads retain their original diagnostics.
50+
Prediction also applies with custom parser objects, cancellation tokens, and tracing.
51+
Skipped attempts emit no traces or checkpoint calls; cancellation checks remain active.
52+
EOF and reads without an active context retain the speculative path.
53+
Return-type disambiguation is unchanged.
54+
4855
### Style
4956

5057
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).

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@microsoft/powerquery-parser",
3-
"version": "3.0.0",
3+
"version": "3.1.0",
44
"description": "A parser for the Power Query/M formula language.",
55
"author": "Microsoft",
66
"license": "MIT",

‎src/powerquery-parser/parser/parsers/naiveParseSteps.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,6 +1922,21 @@ export async function readType(
19221922

19231923
state.cancellationToken?.throwIfCancelled();
19241924

1925+
// Only readType discards a failed primary-type diagnostic in favor of an expression.
1926+
// Skipped attempts emit no traces or checkpoint calls; cancellation is still checked by the readers.
1927+
if (
1928+
state.currentContextNode !== undefined &&
1929+
state.tokenIndex < state.lexerSnapshot.tokens.length &&
1930+
!canStartPrimaryType(state)
1931+
) {
1932+
trace.exit({
1933+
[NaiveTraceConstant.TokenIndex]: state.tokenIndex,
1934+
[TraceConstant.IsError]: true,
1935+
});
1936+
1937+
return parser.readPrimaryExpression(state, parser, trace.id);
1938+
}
1939+
19251940
const triedReadPrimaryType: TriedReadPrimaryType = await tryReadPrimaryType(state, parser, trace.id);
19261941

19271942
if (ResultUtils.isOk(triedReadPrimaryType)) {
@@ -2348,6 +2363,21 @@ export async function readFunctionType(
23482363
return functionType;
23492364
}
23502365

2366+
function canStartPrimaryType(state: ParseState): boolean {
2367+
const token: Token.Token | undefined = state.lexerSnapshot.tokens[state.tokenIndex];
2368+
2369+
return (
2370+
token !== undefined &&
2371+
(token.kind === TokenKind.LeftBracket ||
2372+
token.kind === TokenKind.LeftBrace ||
2373+
token.kind === TokenKind.KeywordType ||
2374+
token.kind === TokenKind.NullLiteral ||
2375+
(token.kind === TokenKind.Identifier &&
2376+
(token.data === Constant.LanguageConstant.Nullable ||
2377+
ConstantUtils.isPrimitiveTypeConstant(token.data))))
2378+
);
2379+
}
2380+
23512381
async function tryReadPrimaryType(
23522382
state: ParseState,
23532383
parser: Parser,
Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
import "mocha";
5+
import { expect } from "chai";
6+
import { rejects } from "assert";
7+
8+
import {
9+
CommonError,
10+
CounterCancellationToken,
11+
DefaultSettings,
12+
NoOpCancellationToken,
13+
Parser,
14+
Task,
15+
TaskUtils,
16+
} from "../../..";
17+
import { Trace, TraceConstant, TraceManager } from "../../../powerquery-parser/common/trace";
18+
import { Ast } from "../../../powerquery-parser/language";
19+
20+
// Strict primary-type reads still perform speculation. Only an unconsumed primitive-type
21+
// failure is discarded here, reproducing readType's fallback without using its prediction.
22+
async function readTypeWithoutPrediction(
23+
state: Parser.ParseState,
24+
parser: Parser.Parser,
25+
correlationId: number | undefined,
26+
): Promise<Ast.TType> {
27+
const tokenIndex: number = state.tokenIndex;
28+
29+
try {
30+
return await parser.readPrimaryType(state, parser, correlationId);
31+
} catch (error: unknown) {
32+
if (
33+
state.tokenIndex !== tokenIndex ||
34+
!(
35+
error instanceof Parser.ParseError.InvalidPrimitiveTypeError ||
36+
error instanceof Parser.ParseError.ExpectedAnyTokenKindError
37+
)
38+
) {
39+
throw error;
40+
}
41+
42+
return parser.readPrimaryExpression(state, parser, correlationId);
43+
}
44+
}
45+
46+
class RecordingTraceManager extends TraceManager {
47+
public readonly events: { trace: Trace; message: string }[] = [];
48+
49+
public emit(trace: Trace, message: string): void {
50+
this.events.push({ trace, message });
51+
}
52+
}
53+
54+
function newState(overrides?: Partial<Parser.ParseState>): Parser.ParseState {
55+
const lex: Task.TriedLexTask = TaskUtils.tryLex(DefaultSettings, "unknown");
56+
TaskUtils.assertIsLexStageOk(lex);
57+
const state: Parser.ParseState = Parser.ParseStateUtils.newState(lex.lexerSnapshot, overrides);
58+
Parser.ParseStateUtils.startContext(state, Ast.NodeKind.ListType);
59+
60+
return state;
61+
}
62+
63+
describe("Parser primary-type prediction", () => {
64+
for (const parser of [Parser.CombinatorialParserV2, Parser.RecursiveDescentParser]) {
65+
for (const text of [
66+
"let a = type unknown in a",
67+
"let a = type (1 + 2) in a",
68+
"let a = type @unknown in a",
69+
"let a = type 1 in a",
70+
"let a = type in a",
71+
"let a = type table [x=",
72+
"let a = type function (x as",
73+
"let a = type function (x as number) as ",
74+
"let a = type [x = number] in a",
75+
"let a = type nullable number in a",
76+
"(x as number) as nullable number => x",
77+
]) {
78+
it(`preserves full parse results for ${text}`, async () => {
79+
const actual: Task.TriedLexParseTask = await TaskUtils.tryLexParse(
80+
{ ...DefaultSettings, parser },
81+
text,
82+
);
83+
84+
const expected: Task.TriedLexParseTask = await TaskUtils.tryLexParse(
85+
{ ...DefaultSettings, parser: { ...parser, readType: readTypeWithoutPrediction } },
86+
text,
87+
);
88+
89+
expect(actual).to.deep.equal(expected);
90+
91+
expect("nodeIdMapCollection" in actual ? actual.nodeIdMapCollection : undefined).to.deep.equal(
92+
"nodeIdMapCollection" in expected ? expected.nodeIdMapCollection : undefined,
93+
);
94+
95+
expect("parseState" in actual ? actual.parseState : undefined).to.deep.equal(
96+
"parseState" in expected ? expected.parseState : undefined,
97+
);
98+
});
99+
}
100+
101+
it("skips speculation with a cancellation token and custom parser hooks", async () => {
102+
const state: Parser.ParseState = newState({ cancellationToken: NoOpCancellationToken });
103+
let primaryReads: number = 0;
104+
105+
const customParser: Parser.Parser = {
106+
...parser,
107+
checkpoint: (): never => {
108+
throw new Error("The skipped attempt must not create a checkpoint");
109+
},
110+
restoreCheckpoint: (): never => {
111+
throw new Error("The skipped attempt must not restore a checkpoint");
112+
},
113+
readPrimaryExpression: async (
114+
state: Parser.ParseState,
115+
parser: Parser.Parser,
116+
correlationId: number | undefined,
117+
): Promise<Ast.TPrimaryExpression> => {
118+
primaryReads += 1;
119+
120+
return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId);
121+
},
122+
};
123+
124+
const result: Ast.TType = await customParser.readType(state, customParser, undefined);
125+
expect(result.kind).to.equal(Ast.NodeKind.IdentifierExpression);
126+
expect(primaryReads).to.equal(1);
127+
});
128+
129+
it("traces only executed reads with matching entries, exits, and correlations", async () => {
130+
const traceManager: RecordingTraceManager = new RecordingTraceManager();
131+
const state: Parser.ParseState = newState({ traceManager });
132+
await parser.readType(state, parser, undefined);
133+
134+
const entries: Map<number, Trace> = new Map();
135+
const exits: Set<number> = new Set();
136+
137+
for (const { trace, message } of traceManager.events) {
138+
if (message === TraceConstant.Entry) {
139+
expect(entries.has(trace.id)).to.equal(false);
140+
141+
if (trace.correlationId !== undefined) {
142+
expect(entries.has(trace.correlationId)).to.equal(true);
143+
}
144+
145+
entries.set(trace.id, trace);
146+
} else if (message === TraceConstant.Exit) {
147+
expect(entries.has(trace.id)).to.equal(true);
148+
expect(exits.has(trace.id)).to.equal(false);
149+
exits.add(trace.id);
150+
}
151+
}
152+
153+
const tasks: string[] = [...entries.values()].map((trace: Trace) => trace.task);
154+
expect(tasks).to.include("readType");
155+
expect(tasks).to.include("readPrimaryExpression");
156+
expect(tasks).not.to.include("tryReadPrimaryType");
157+
expect(tasks).not.to.include("tryReadPrimitiveType");
158+
expect(exits.size).to.equal(entries.size);
159+
});
160+
161+
it("honors cancellation before prediction", async () => {
162+
const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(1) });
163+
const tokenIndex: number = state.tokenIndex;
164+
const idCounter: number = state.contextState.idCounter;
165+
await rejects(parser.readType(state, parser, undefined), CommonError.CancellationError);
166+
expect(state.tokenIndex).to.equal(tokenIndex);
167+
expect(state.contextState.idCounter).to.equal(idCounter);
168+
});
169+
170+
it("honors cancellation when entering the primary-expression fallback", async () => {
171+
const state: Parser.ParseState = newState({ cancellationToken: new CounterCancellationToken(2) });
172+
let enteredFallback: boolean = false;
173+
174+
const customParser: Parser.Parser = {
175+
...parser,
176+
readPrimaryExpression: async (
177+
state: Parser.ParseState,
178+
parser: Parser.Parser,
179+
correlationId: number | undefined,
180+
): Promise<Ast.TPrimaryExpression> => {
181+
enteredFallback = true;
182+
183+
return await Parser.NaiveParseSteps.readPrimaryExpression(state, parser, correlationId);
184+
},
185+
};
186+
187+
await rejects(customParser.readType(state, customParser, undefined), CommonError.CancellationError);
188+
expect(enteredFallback).to.equal(true);
189+
expect(state.tokenIndex).to.equal(0);
190+
});
191+
192+
it("retains strict primitive-type diagnostics and their checkpoint hooks", async () => {
193+
let checkpoints: number = 0;
194+
195+
const customParser: Parser.Parser = {
196+
...parser,
197+
checkpoint: (state: Parser.ParseState): Promise<Parser.ParseStateCheckpoint> => {
198+
checkpoints += 1;
199+
200+
return parser.checkpoint(state);
201+
},
202+
};
203+
204+
await rejects(
205+
customParser.readPrimaryType(newState(), customParser, undefined),
206+
Parser.ParseError.InvalidPrimitiveTypeError,
207+
);
208+
209+
expect(checkpoints).to.be.greaterThan(0);
210+
});
211+
212+
it("retains EOF rollback and clears the stale cached token", async () => {
213+
const state: Parser.ParseState = newState();
214+
state.tokenIndex = state.lexerSnapshot.tokens.length;
215+
state.currentTokenKind = undefined;
216+
expect(state.currentToken).not.to.equal(undefined);
217+
let checkpoints: number = 0;
218+
219+
const customParser: Parser.Parser = {
220+
...parser,
221+
checkpoint: (state: Parser.ParseState): Promise<Parser.ParseStateCheckpoint> => {
222+
checkpoints += 1;
223+
224+
return parser.checkpoint(state);
225+
},
226+
};
227+
228+
await rejects(customParser.readType(state, customParser, undefined));
229+
expect(checkpoints).to.be.greaterThan(0);
230+
expect(state.currentToken).to.equal(undefined);
231+
});
232+
}
233+
});

0 commit comments

Comments
 (0)