Skip to content

Commit 2767197

Browse files
jakebaileyjakebailey
authored
Allow arrow function parameter parsing to bail out during speculation, redo (#48493)
* Allow arrow function parameter parsing to bail out during speculation, redo * Add correct baselines * Allow await/yield keywords for more graceful error reporting * Add test for other parser issue * Address some PR feedback * Add extra comment * Add async variants
1 parent 493ddc2 commit 2767197

9 files changed

+548
-14
lines changed

‎src/compiler/parser.ts

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2374,7 +2374,7 @@ namespace ts {
23742374
return createNodeArray(list, listPos);
23752375
}
23762376

2377-
function parseListElement<T extends Node>(parsingContext: ParsingContext, parseElement: () => T): T {
2377+
function parseListElement<T extends Node | undefined>(parsingContext: ParsingContext, parseElement: () => T): T {
23782378
const node = currentNode(parsingContext);
23792379
if (node) {
23802380
return consumeNode(node) as T;
@@ -2719,7 +2719,9 @@ namespace ts {
27192719
}
27202720

27212721
// Parses a comma-delimited list of elements
2722-
function parseDelimitedList<T extends Node>(kind: ParsingContext, parseElement: () => T, considerSemicolonAsDelimiter?: boolean): NodeArray<T> {
2722+
function parseDelimitedList<T extends Node>(kind: ParsingContext, parseElement: () => T, considerSemicolonAsDelimiter?: boolean): NodeArray<T>;
2723+
function parseDelimitedList<T extends Node | undefined>(kind: ParsingContext, parseElement: () => T, considerSemicolonAsDelimiter?: boolean): NodeArray<NonNullable<T>> | undefined;
2724+
function parseDelimitedList<T extends Node | undefined>(kind: ParsingContext, parseElement: () => T, considerSemicolonAsDelimiter?: boolean): NodeArray<NonNullable<T>> | undefined {
27232725
const saveParsingContext = parsingContext;
27242726
parsingContext |= 1 << kind;
27252727
const list = [];
@@ -2729,7 +2731,11 @@ namespace ts {
27292731
while (true) {
27302732
if (isListElement(kind, /*inErrorRecovery*/ false)) {
27312733
const startPos = scanner.getStartPos();
2732-
list.push(parseListElement(kind, parseElement));
2734+
const result = parseListElement(kind, parseElement);
2735+
if (!result) {
2736+
return undefined;
2737+
}
2738+
list.push(result as NonNullable<T>);
27332739
commaStart = scanner.getTokenPos();
27342740

27352741
if (parseOptional(SyntaxKind.CommaToken)) {
@@ -3239,15 +3245,24 @@ namespace ts {
32393245
return name;
32403246
}
32413247

3242-
function parseParameterInOuterAwaitContext() {
3243-
return parseParameterWorker(/*inOuterAwaitContext*/ true);
3248+
function isParameterNameStart() {
3249+
// Be permissive about await and yield by calling isBindingIdentifier instead of isIdentifier; disallowing
3250+
// them during a speculative parse leads to many more follow-on errors than allowing the function to parse then later
3251+
// complaining about the use of the keywords.
3252+
return isBindingIdentifier() || token() === SyntaxKind.OpenBracketToken || token() === SyntaxKind.OpenBraceToken;
3253+
}
3254+
3255+
function parseParameter(inOuterAwaitContext: boolean): ParameterDeclaration {
3256+
return parseParameterWorker(inOuterAwaitContext);
32443257
}
32453258

3246-
function parseParameter(): ParameterDeclaration {
3247-
return parseParameterWorker(/*inOuterAwaitContext*/ false);
3259+
function parseParameterForSpeculation(inOuterAwaitContext: boolean): ParameterDeclaration | undefined {
3260+
return parseParameterWorker(inOuterAwaitContext, /*allowAmbiguity*/ false);
32483261
}
32493262

3250-
function parseParameterWorker(inOuterAwaitContext: boolean): ParameterDeclaration {
3263+
function parseParameterWorker(inOuterAwaitContext: boolean): ParameterDeclaration;
3264+
function parseParameterWorker(inOuterAwaitContext: boolean, allowAmbiguity: false): ParameterDeclaration | undefined;
3265+
function parseParameterWorker(inOuterAwaitContext: boolean, allowAmbiguity = true): ParameterDeclaration | undefined {
32513266
const pos = getNodePos();
32523267
const hasJSDoc = hasPrecedingJSDocComment();
32533268

@@ -3277,13 +3292,20 @@ namespace ts {
32773292

32783293
const savedTopLevel = topLevel;
32793294
topLevel = false;
3295+
32803296
const modifiers = parseModifiers();
3297+
const dotDotDotToken = parseOptionalToken(SyntaxKind.DotDotDotToken);
3298+
3299+
if (!allowAmbiguity && !isParameterNameStart()) {
3300+
return undefined;
3301+
}
3302+
32813303
const node = withJSDoc(
32823304
finishNode(
32833305
factory.createParameterDeclaration(
32843306
decorators,
32853307
modifiers,
3286-
parseOptionalToken(SyntaxKind.DotDotDotToken),
3308+
dotDotDotToken,
32873309
parseNameOfParameter(modifiers),
32883310
parseOptionalToken(SyntaxKind.QuestionToken),
32893311
parseTypeAnnotation(),
@@ -3322,7 +3344,9 @@ namespace ts {
33223344
return false;
33233345
}
33243346

3325-
function parseParametersWorker(flags: SignatureFlags) {
3347+
function parseParametersWorker(flags: SignatureFlags, allowAmbiguity: true): NodeArray<ParameterDeclaration>;
3348+
function parseParametersWorker(flags: SignatureFlags, allowAmbiguity: false): NodeArray<ParameterDeclaration> | undefined;
3349+
function parseParametersWorker(flags: SignatureFlags, allowAmbiguity: boolean): NodeArray<ParameterDeclaration> | undefined {
33263350
// FormalParameters [Yield,Await]: (modified)
33273351
// [empty]
33283352
// FormalParameterList[?Yield,Await]
@@ -3344,7 +3368,7 @@ namespace ts {
33443368

33453369
const parameters = flags & SignatureFlags.JSDoc ?
33463370
parseDelimitedList(ParsingContext.JSDocParameters, parseJSDocParameter) :
3347-
parseDelimitedList(ParsingContext.Parameters, savedAwaitContext ? parseParameterInOuterAwaitContext : parseParameter);
3371+
parseDelimitedList(ParsingContext.Parameters, () => allowAmbiguity ? parseParameter(savedAwaitContext) : parseParameterForSpeculation(savedAwaitContext));
33483372

33493373
setYieldContext(savedYieldContext);
33503374
setAwaitContext(savedAwaitContext);
@@ -3370,7 +3394,7 @@ namespace ts {
33703394
return createMissingList<ParameterDeclaration>();
33713395
}
33723396

3373-
const parameters = parseParametersWorker(flags);
3397+
const parameters = parseParametersWorker(flags, /*allowAmbiguity*/ true);
33743398
parseExpected(SyntaxKind.CloseParenToken);
33753399
return parameters;
33763400
}
@@ -3463,7 +3487,7 @@ namespace ts {
34633487
}
34643488

34653489
function parseIndexSignatureDeclaration(pos: number, hasJSDoc: boolean, decorators: NodeArray<Decorator> | undefined, modifiers: NodeArray<Modifier> | undefined): IndexSignatureDeclaration {
3466-
const parameters = parseBracketedList(ParsingContext.Parameters, parseParameter, SyntaxKind.OpenBracketToken, SyntaxKind.CloseBracketToken);
3490+
const parameters = parseBracketedList<ParameterDeclaration>(ParsingContext.Parameters, () => parseParameter(/*inOuterAwaitContext*/ false), SyntaxKind.OpenBracketToken, SyntaxKind.CloseBracketToken);
34673491
const type = parseTypeAnnotation();
34683492
parseTypeMemberSemicolon();
34693493
const node = factory.createIndexSignature(decorators, modifiers, parameters, type);
@@ -4641,7 +4665,16 @@ namespace ts {
46414665
parameters = createMissingList<ParameterDeclaration>();
46424666
}
46434667
else {
4644-
parameters = parseParametersWorker(isAsync);
4668+
if (!allowAmbiguity) {
4669+
const maybeParameters = parseParametersWorker(isAsync, allowAmbiguity);
4670+
if (!maybeParameters) {
4671+
return undefined;
4672+
}
4673+
parameters = maybeParameters;
4674+
}
4675+
else {
4676+
parameters = parseParametersWorker(isAsync, allowAmbiguity);
4677+
}
46454678
if (!parseExpected(SyntaxKind.CloseParenToken) && !allowAmbiguity) {
46464679
return undefined;
46474680
}

‎tests/baselines/reference/arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
//// [arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts]
2+
// regression test for https://github.com/microsoft/TypeScript/issues/32914
3+
declare var value: boolean;
4+
declare var a: any;
5+
6+
const test = () => ({
7+
// "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space.
8+
prop: !value, // remove ! to see that errors will be gone
9+
run: () => { //replace arrow function with regular function to see that errors will be gone
10+
// comment next line or remove "()" to see that errors will be gone
11+
if(!a.b()) { return 'special'; }
12+
13+
return 'default';
14+
}
15+
});
16+
17+
18+
//// [arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.js]
19+
var test = function () { return ({
20+
// "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space.
21+
prop: !value,
22+
run: function () {
23+
// comment next line or remove "()" to see that errors will be gone
24+
if (!a.b()) {
25+
return 'special';
26+
}
27+
return 'default';
28+
}
29+
}); };

‎tests/baselines/reference/arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.symbols

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
=== tests/cases/compiler/arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts ===
2+
// regression test for https://github.com/microsoft/TypeScript/issues/32914
3+
declare var value: boolean;
4+
>value : Symbol(value, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 1, 11))
5+
6+
declare var a: any;
7+
>a : Symbol(a, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 2, 11))
8+
9+
const test = () => ({
10+
>test : Symbol(test, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 4, 5))
11+
12+
// "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space.
13+
prop: !value, // remove ! to see that errors will be gone
14+
>prop : Symbol(prop, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 4, 21))
15+
>value : Symbol(value, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 1, 11))
16+
17+
run: () => { //replace arrow function with regular function to see that errors will be gone
18+
>run : Symbol(run, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 6, 17))
19+
20+
// comment next line or remove "()" to see that errors will be gone
21+
if(!a.b()) { return 'special'; }
22+
>a : Symbol(a, Decl(arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts, 2, 11))
23+
24+
return 'default';
25+
}
26+
});
27+

‎tests/baselines/reference/arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.types

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
=== tests/cases/compiler/arrowFunctionParsingDoesNotConfuseParenthesizedObjectForArrowHead.ts ===
2+
// regression test for https://github.com/microsoft/TypeScript/issues/32914
3+
declare var value: boolean;
4+
>value : boolean
5+
6+
declare var a: any;
7+
>a : any
8+
9+
const test = () => ({
10+
>test : () => { prop: boolean; run: () => "special" | "default"; }
11+
>() => ({ // "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space. prop: !value, // remove ! to see that errors will be gone run: () => { //replace arrow function with regular function to see that errors will be gone // comment next line or remove "()" to see that errors will be gone if(!a.b()) { return 'special'; } return 'default'; }}) : () => { prop: boolean; run: () => "special" | "default"; }
12+
>({ // "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space. prop: !value, // remove ! to see that errors will be gone run: () => { //replace arrow function with regular function to see that errors will be gone // comment next line or remove "()" to see that errors will be gone if(!a.b()) { return 'special'; } return 'default'; }}) : { prop: boolean; run: () => "special" | "default"; }
13+
>{ // "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space. prop: !value, // remove ! to see that errors will be gone run: () => { //replace arrow function with regular function to see that errors will be gone // comment next line or remove "()" to see that errors will be gone if(!a.b()) { return 'special'; } return 'default'; }} : { prop: boolean; run: () => "special" | "default"; }
14+
15+
// "Identifier expected." error on "!" and two "Duplicate identifier '(Missing)'." errors on space.
16+
prop: !value, // remove ! to see that errors will be gone
17+
>prop : boolean
18+
>!value : boolean
19+
>value : boolean
20+
21+
run: () => { //replace arrow function with regular function to see that errors will be gone
22+
>run : () => "special" | "default"
23+
>() => { //replace arrow function with regular function to see that errors will be gone // comment next line or remove "()" to see that errors will be gone if(!a.b()) { return 'special'; } return 'default'; } : () => "special" | "default"
24+
25+
// comment next line or remove "()" to see that errors will be gone
26+
if(!a.b()) { return 'special'; }
27+
>!a.b() : boolean
28+
>a.b() : any
29+
>a.b : any
30+
>a : any
31+
>b : any
32+
>'special' : "special"
33+
34+
return 'default';
35+
>'default' : "default"
36+
}
37+
});
38+

‎tests/baselines/reference/arrowFunctionParsingGenericInObject.js

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//// [arrowFunctionParsingGenericInObject.ts]
2+
const fn1 = () => ({
3+
test: <T = undefined>(value: T): T => value,
4+
extraValue: () => {},
5+
})
6+
7+
const fn1async = () => ({
8+
test: async <T = undefined>(value: T): Promise<T> => value,
9+
extraValue: () => {},
10+
})
11+
12+
const fn2 = () => ({
13+
test: <T>(value: T): T => value,
14+
extraValue: () => {},
15+
})
16+
17+
const fn2async = () => ({
18+
test: async <T>(value: T): Promise<T> => value,
19+
extraValue: () => {},
20+
})
21+
22+
const fn3 = () => ({
23+
extraValue: () => {},
24+
test: <T = undefined>(value: T): T => value,
25+
})
26+
27+
const fn3async = () => ({
28+
extraValue: () => {},
29+
test: async <T = undefined>(value: T): Promise<T> => value,
30+
})
31+
32+
const fn4 = () => ({
33+
extraValue: '',
34+
test: <T = undefined>(value: T): T => value,
35+
})
36+
37+
const fn4async = () => ({
38+
extraValue: '',
39+
test: async <T = undefined>(value: T): Promise<T> => value,
40+
})
41+
42+
43+
//// [arrowFunctionParsingGenericInObject.js]
44+
const fn1 = () => ({
45+
test: (value) => value,
46+
extraValue: () => { },
47+
});
48+
const fn1async = () => ({
49+
test: async (value) => value,
50+
extraValue: () => { },
51+
});
52+
const fn2 = () => ({
53+
test: (value) => value,
54+
extraValue: () => { },
55+
});
56+
const fn2async = () => ({
57+
test: async (value) => value,
58+
extraValue: () => { },
59+
});
60+
const fn3 = () => ({
61+
extraValue: () => { },
62+
test: (value) => value,
63+
});
64+
const fn3async = () => ({
65+
extraValue: () => { },
66+
test: async (value) => value,
67+
});
68+
const fn4 = () => ({
69+
extraValue: '',
70+
test: (value) => value,
71+
});
72+
const fn4async = () => ({
73+
extraValue: '',
74+
test: async (value) => value,
75+
});

0 commit comments

Comments
 (0)