Skip to content

Commit 735a46f

Browse files
author
Andy
authored
If parsing a function type fails, parseTypeReference() to ensure something is returned (microsoft#24567)
* If parsing a function type fails, parseTypeReference() to ensure something is returned * Avoid tryParse * Add missing semicolon * Don't check for undefined, check for missing type * Don't set parameters undefined, set to missingList and return false * Update API baselines * Code review
1 parent 9681796 commit 735a46f

17 files changed

Lines changed: 140 additions & 84 deletions

‎src/compiler/parser.ts‎

Lines changed: 48 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ namespace ts {
44
Yield = 1 << 0,
55
Await = 1 << 1,
66
Type = 1 << 2,
7-
RequireCompleteParameterList = 1 << 3,
87
IgnoreMissingOpenBrace = 1 << 4,
98
JSDoc = 1 << 5,
109
}
@@ -1257,8 +1256,8 @@ namespace ts {
12571256
new TokenConstructor(kind, p, p);
12581257
}
12591258

1260-
function createNodeWithJSDoc(kind: SyntaxKind): Node {
1261-
const node = createNode(kind);
1259+
function createNodeWithJSDoc(kind: SyntaxKind, pos?: number): Node {
1260+
const node = createNode(kind, pos);
12621261
if (scanner.getTokenFlags() & TokenFlags.PrecedingJSDocComment) {
12631262
addJSDocComment(<HasJSDoc>node);
12641263
}
@@ -2257,6 +2256,24 @@ namespace ts {
22572256
return finishNode(node);
22582257
}
22592258

2259+
// If true, we should abort parsing an error function.
2260+
function typeHasArrowFunctionBlockingParseError(node: TypeNode): boolean {
2261+
switch (node.kind) {
2262+
case SyntaxKind.TypeReference:
2263+
return nodeIsMissing((node as TypeReferenceNode).typeName);
2264+
case SyntaxKind.FunctionType:
2265+
case SyntaxKind.ConstructorType: {
2266+
const { parameters, type } = node as FunctionOrConstructorTypeNode;
2267+
// parameters.pos === parameters.end only if we used parseMissingList, else should contain at least `()`
2268+
return parameters.pos === parameters.end || typeHasArrowFunctionBlockingParseError(type);
2269+
}
2270+
case SyntaxKind.ParenthesizedType:
2271+
return typeHasArrowFunctionBlockingParseError((node as ParenthesizedTypeNode).type);
2272+
default:
2273+
return false;
2274+
}
2275+
}
2276+
22602277
function parseThisTypePredicate(lhs: ThisTypeNode): TypePredicateNode {
22612278
nextToken();
22622279
const node = createNode(SyntaxKind.TypePredicate, lhs.pos) as TypePredicateNode;
@@ -2343,7 +2360,7 @@ namespace ts {
23432360
return finishNode(parameter);
23442361
}
23452362

2346-
function parseJSDocType() {
2363+
function parseJSDocType(): TypeNode {
23472364
const dotdotdot = parseOptionalToken(SyntaxKind.DotDotDotToken);
23482365
let type = parseType();
23492366
if (dotdotdot) {
@@ -2451,6 +2468,7 @@ namespace ts {
24512468
}
24522469

24532470
/**
2471+
* Note: If returnToken is EqualsGreaterThanToken, `signature.type` will always be defined.
24542472
* @returns If return type parsing succeeds
24552473
*/
24562474
function fillSignature(
@@ -2460,12 +2478,12 @@ namespace ts {
24602478
if (!(flags & SignatureFlags.JSDoc)) {
24612479
signature.typeParameters = parseTypeParameters();
24622480
}
2463-
signature.parameters = parseParameterList(flags)!; // TODO: GH#18217
2481+
const parametersParsedSuccessfully = parseParameterList(signature, flags);
24642482
if (shouldParseReturnType(returnToken, !!(flags & SignatureFlags.Type))) {
24652483
signature.type = parseTypeOrTypePredicate();
2466-
return signature.type !== undefined;
2484+
if (typeHasArrowFunctionBlockingParseError(signature.type)) return false;
24672485
}
2468-
return true;
2486+
return parametersParsedSuccessfully;
24692487
}
24702488

24712489
function shouldParseReturnType(returnToken: SyntaxKind.ColonToken | SyntaxKind.EqualsGreaterThanToken, isType: boolean): boolean {
@@ -2485,7 +2503,8 @@ namespace ts {
24852503
return false;
24862504
}
24872505

2488-
function parseParameterList(flags: SignatureFlags) {
2506+
// Returns true on success.
2507+
function parseParameterList(signature: SignatureDeclaration, flags: SignatureFlags): boolean {
24892508
// FormalParameters [Yield,Await]: (modified)
24902509
// [empty]
24912510
// FormalParameterList[?Yield,Await]
@@ -2499,31 +2518,23 @@ namespace ts {
24992518
//
25002519
// SingleNameBinding [Yield,Await]:
25012520
// BindingIdentifier[?Yield,?Await]Initializer [In, ?Yield,?Await] opt
2502-
if (parseExpected(SyntaxKind.OpenParenToken)) {
2503-
const savedYieldContext = inYieldContext();
2504-
const savedAwaitContext = inAwaitContext();
2505-
2506-
setYieldContext(!!(flags & SignatureFlags.Yield));
2507-
setAwaitContext(!!(flags & SignatureFlags.Await));
2521+
if (!parseExpected(SyntaxKind.OpenParenToken)) {
2522+
signature.parameters = createMissingList<ParameterDeclaration>();
2523+
return false;
2524+
}
25082525

2509-
const result = parseDelimitedList(ParsingContext.Parameters, flags & SignatureFlags.JSDoc ? parseJSDocParameter : parseParameter);
2526+
const savedYieldContext = inYieldContext();
2527+
const savedAwaitContext = inAwaitContext();
25102528

2511-
setYieldContext(savedYieldContext);
2512-
setAwaitContext(savedAwaitContext);
2529+
setYieldContext(!!(flags & SignatureFlags.Yield));
2530+
setAwaitContext(!!(flags & SignatureFlags.Await));
25132531

2514-
if (!parseExpected(SyntaxKind.CloseParenToken) && (flags & SignatureFlags.RequireCompleteParameterList)) {
2515-
// Caller insisted that we had to end with a ) We didn't. So just return
2516-
// undefined here.
2517-
return undefined;
2518-
}
2532+
signature.parameters = parseDelimitedList(ParsingContext.Parameters, flags & SignatureFlags.JSDoc ? parseJSDocParameter : parseParameter);
25192533

2520-
return result;
2521-
}
2534+
setYieldContext(savedYieldContext);
2535+
setAwaitContext(savedAwaitContext);
25222536

2523-
// We didn't even have an open paren. If the caller requires a complete parameter list,
2524-
// we definitely can't provide that. However, if they're ok with an incomplete one,
2525-
// then just return an empty set of parameters.
2526-
return (flags & SignatureFlags.RequireCompleteParameterList) ? undefined : createMissingList<ParameterDeclaration>();
2537+
return parseExpected(SyntaxKind.CloseParenToken);
25272538
}
25282539

25292540
function parseTypeMemberSemicolon() {
@@ -2772,28 +2783,19 @@ namespace ts {
27722783
return finishNode(node);
27732784
}
27742785

2775-
function parseParenthesizedType(): ParenthesizedTypeNode {
2786+
function parseParenthesizedType(): TypeNode {
27762787
const node = <ParenthesizedTypeNode>createNode(SyntaxKind.ParenthesizedType);
27772788
parseExpected(SyntaxKind.OpenParenToken);
27782789
node.type = parseType();
2779-
if (!node.type) {
2780-
return undefined!; // TODO: GH#18217
2781-
}
27822790
parseExpected(SyntaxKind.CloseParenToken);
27832791
return finishNode(node);
27842792
}
27852793

2786-
function parseFunctionOrConstructorType(kind: SyntaxKind): FunctionOrConstructorTypeNode | undefined {
2787-
const node = <FunctionOrConstructorTypeNode>createNodeWithJSDoc(kind);
2788-
if (kind === SyntaxKind.ConstructorType) {
2789-
parseExpected(SyntaxKind.NewKeyword);
2790-
}
2791-
if (!fillSignature(SyntaxKind.EqualsGreaterThanToken, SignatureFlags.Type | (sourceFile.languageVariant === LanguageVariant.JSX ? SignatureFlags.RequireCompleteParameterList : 0), node)) {
2792-
return undefined;
2793-
}
2794-
if (!node.parameters) {
2795-
return undefined;
2796-
}
2794+
function parseFunctionOrConstructorType(): TypeNode {
2795+
const pos = getNodePos();
2796+
const kind = parseOptional(SyntaxKind.NewKeyword) ? SyntaxKind.ConstructorType : SyntaxKind.FunctionType;
2797+
const node = <FunctionOrConstructorTypeNode>createNodeWithJSDoc(kind, pos);
2798+
fillSignature(SyntaxKind.EqualsGreaterThanToken, SignatureFlags.Type, node);
27972799
return finishNode(node);
27982800
}
27992801

@@ -3134,11 +3136,8 @@ namespace ts {
31343136
}
31353137

31363138
function parseTypeWorker(noConditionalTypes?: boolean): TypeNode {
3137-
if (isStartOfFunctionType()) {
3138-
return parseFunctionOrConstructorType(SyntaxKind.FunctionType)!; // TODO: GH#18217
3139-
}
3140-
if (token() === SyntaxKind.NewKeyword) {
3141-
return parseFunctionOrConstructorType(SyntaxKind.ConstructorType)!;
3139+
if (isStartOfFunctionType() || token() === SyntaxKind.NewKeyword) {
3140+
return parseFunctionOrConstructorType();
31423141
}
31433142
const type = parseUnionTypeOrHigher();
31443143
if (!noConditionalTypes && !scanner.hasPrecedingLineBreak() && parseOptional(SyntaxKind.ExtendsKeyword)) {
@@ -3626,12 +3625,7 @@ namespace ts {
36263625
// a => (b => c)
36273626
// And think that "(b =>" was actually a parenthesized arrow function with a missing
36283627
// close paren.
3629-
if (!fillSignature(SyntaxKind.ColonToken, isAsync | (allowAmbiguity ? SignatureFlags.None : SignatureFlags.RequireCompleteParameterList), node)) {
3630-
return undefined;
3631-
}
3632-
3633-
// If we couldn't get parameters, we definitely could not parse out an arrow function.
3634-
if (!node.parameters) {
3628+
if (!fillSignature(SyntaxKind.ColonToken, isAsync, node) && !allowAmbiguity) {
36353629
return undefined;
36363630
}
36373631

‎src/compiler/types.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,11 +1098,16 @@ namespace ts {
10981098

10991099
export type FunctionOrConstructorTypeNode = FunctionTypeNode | ConstructorTypeNode;
11001100

1101-
export interface FunctionTypeNode extends TypeNode, SignatureDeclarationBase {
1101+
export interface FunctionOrConstructorTypeNodeBase extends TypeNode, SignatureDeclarationBase {
1102+
kind: SyntaxKind.FunctionType | SyntaxKind.ConstructorType;
1103+
type: TypeNode;
1104+
}
1105+
1106+
export interface FunctionTypeNode extends FunctionOrConstructorTypeNodeBase {
11021107
kind: SyntaxKind.FunctionType;
11031108
}
11041109

1105-
export interface ConstructorTypeNode extends TypeNode, SignatureDeclarationBase {
1110+
export interface ConstructorTypeNode extends FunctionOrConstructorTypeNodeBase {
11061111
kind: SyntaxKind.ConstructorType;
11071112
}
11081113

‎tests/baselines/reference/ArrowFunction3.errors.txt‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1-
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,14): error TS1110: Type expected.
1+
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,10): error TS2304: Cannot find name 'a'.
2+
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,12): error TS1005: ',' expected.
3+
tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,14): error TS1005: ';' expected.
24

35

4-
==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts (1 errors) ====
6+
==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts (3 errors) ====
57
var v = (a): => {
8+
~
9+
!!! error TS2304: Cannot find name 'a'.
10+
~
11+
!!! error TS1005: ',' expected.
612
~~
7-
!!! error TS1110: Type expected.
13+
!!! error TS1005: ';' expected.
814

915
};

‎tests/baselines/reference/ArrowFunction3.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,7 @@ var v = (a): => {
44
};
55

66
//// [ArrowFunction3.js]
7-
var v = function (a) {
8-
};
7+
var v = (a);
8+
{
9+
}
10+
;

‎tests/baselines/reference/ArrowFunction3.symbols‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
=== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts ===
22
var v = (a): => {
33
>v : Symbol(v, Decl(ArrowFunction3.ts, 0, 3))
4-
>a : Symbol(a, Decl(ArrowFunction3.ts, 0, 9))
54

65
};

‎tests/baselines/reference/ArrowFunction3.types‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
=== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts ===
22
var v = (a): => {
3-
>v : (a: any) => any
4-
>(a): => { } : (a: any) => any
3+
>v : any
4+
>(a) : any
55
>a : any
6-
> : No type information available!
76

87
};

‎tests/baselines/reference/api/tsserverlibrary.d.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -715,10 +715,14 @@ declare namespace ts {
715715
kind: SyntaxKind.ThisType;
716716
}
717717
type FunctionOrConstructorTypeNode = FunctionTypeNode | ConstructorTypeNode;
718-
interface FunctionTypeNode extends TypeNode, SignatureDeclarationBase {
718+
interface FunctionOrConstructorTypeNodeBase extends TypeNode, SignatureDeclarationBase {
719+
kind: SyntaxKind.FunctionType | SyntaxKind.ConstructorType;
720+
type: TypeNode;
721+
}
722+
interface FunctionTypeNode extends FunctionOrConstructorTypeNodeBase {
719723
kind: SyntaxKind.FunctionType;
720724
}
721-
interface ConstructorTypeNode extends TypeNode, SignatureDeclarationBase {
725+
interface ConstructorTypeNode extends FunctionOrConstructorTypeNodeBase {
722726
kind: SyntaxKind.ConstructorType;
723727
}
724728
interface NodeWithTypeArguments extends TypeNode {

‎tests/baselines/reference/api/typescript.d.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -715,10 +715,14 @@ declare namespace ts {
715715
kind: SyntaxKind.ThisType;
716716
}
717717
type FunctionOrConstructorTypeNode = FunctionTypeNode | ConstructorTypeNode;
718-
interface FunctionTypeNode extends TypeNode, SignatureDeclarationBase {
718+
interface FunctionOrConstructorTypeNodeBase extends TypeNode, SignatureDeclarationBase {
719+
kind: SyntaxKind.FunctionType | SyntaxKind.ConstructorType;
720+
type: TypeNode;
721+
}
722+
interface FunctionTypeNode extends FunctionOrConstructorTypeNodeBase {
719723
kind: SyntaxKind.FunctionType;
720724
}
721-
interface ConstructorTypeNode extends TypeNode, SignatureDeclarationBase {
725+
interface ConstructorTypeNode extends FunctionOrConstructorTypeNodeBase {
722726
kind: SyntaxKind.ConstructorType;
723727
}
724728
interface NodeWithTypeArguments extends TypeNode {

‎tests/baselines/reference/jsdocFunctionTypeFalsePositive.errors.txt‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/a.js(1,14): error TS1139: Type parameter declaration expected.
2+
/a.js(1,17): error TS1003: Identifier expected.
3+
/a.js(1,17): error TS1110: Type expected.
4+
/a.js(1,17): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
5+
/a.js(1,18): error TS1005: '>' expected.
6+
/a.js(1,18): error TS1005: '}' expected.
7+
8+
9+
==== /a.js (6 errors) ====
10+
/** @param {<} x */
11+
~
12+
!!! error TS1139: Type parameter declaration expected.
13+
14+
!!! error TS1003: Identifier expected.
15+
16+
!!! error TS1110: Type expected.
17+
18+
!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
19+
20+
!!! error TS1005: '>' expected.
21+
22+
!!! error TS1005: '}' expected.
23+
function f(x) {}
24+

‎tests/baselines/reference/jsdocFunctionTypeFalsePositive.symbols‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
=== /a.js ===
2+
/** @param {<} x */
3+
function f(x) {}
4+
>f : Symbol(f, Decl(a.js, 0, 0))
5+
>x : Symbol(x, Decl(a.js, 1, 11))
6+

0 commit comments

Comments
 (0)