From 735a46f838e620e1ecbf59f8a5d2727178d56eb8 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 5 Jun 2018 10:24:37 -0700 Subject: [PATCH] If parsing a function type fails, parseTypeReference() to ensure something is returned (#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 --- src/compiler/parser.ts | 108 +++++++++--------- src/compiler/types.ts | 9 +- .../reference/ArrowFunction3.errors.txt | 12 +- tests/baselines/reference/ArrowFunction3.js | 6 +- .../reference/ArrowFunction3.symbols | 1 - .../baselines/reference/ArrowFunction3.types | 5 +- .../reference/api/tsserverlibrary.d.ts | 8 +- tests/baselines/reference/api/typescript.d.ts | 8 +- .../jsdocFunctionTypeFalsePositive.errors.txt | 24 ++++ .../jsdocFunctionTypeFalsePositive.symbols | 6 + .../jsdocFunctionTypeFalsePositive.types | 6 + .../parserX_ArrowFunction3.errors.txt | 12 +- .../reference/parserX_ArrowFunction3.js | 6 +- .../reference/parserX_ArrowFunction3.symbols | 1 - .../reference/parserX_ArrowFunction3.types | 5 +- .../jsdocFunctionTypeFalsePositive.ts | 7 ++ ...ionListNewIdentifierVariableDeclaration.ts | 6 - 17 files changed, 143 insertions(+), 87 deletions(-) create mode 100644 tests/baselines/reference/jsdocFunctionTypeFalsePositive.errors.txt create mode 100644 tests/baselines/reference/jsdocFunctionTypeFalsePositive.symbols create mode 100644 tests/baselines/reference/jsdocFunctionTypeFalsePositive.types create mode 100644 tests/cases/compiler/jsdocFunctionTypeFalsePositive.ts diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 74ec3d8593..026ec1a1c8 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -4,7 +4,6 @@ namespace ts { Yield = 1 << 0, Await = 1 << 1, Type = 1 << 2, - RequireCompleteParameterList = 1 << 3, IgnoreMissingOpenBrace = 1 << 4, JSDoc = 1 << 5, } @@ -1257,8 +1256,8 @@ namespace ts { new TokenConstructor(kind, p, p); } - function createNodeWithJSDoc(kind: SyntaxKind): Node { - const node = createNode(kind); + function createNodeWithJSDoc(kind: SyntaxKind, pos?: number): Node { + const node = createNode(kind, pos); if (scanner.getTokenFlags() & TokenFlags.PrecedingJSDocComment) { addJSDocComment(node); } @@ -2257,6 +2256,24 @@ namespace ts { return finishNode(node); } + // If true, we should abort parsing an error function. + function typeHasArrowFunctionBlockingParseError(node: TypeNode): boolean { + switch (node.kind) { + case SyntaxKind.TypeReference: + return nodeIsMissing((node as TypeReferenceNode).typeName); + case SyntaxKind.FunctionType: + case SyntaxKind.ConstructorType: { + const { parameters, type } = node as FunctionOrConstructorTypeNode; + // parameters.pos === parameters.end only if we used parseMissingList, else should contain at least `()` + return parameters.pos === parameters.end || typeHasArrowFunctionBlockingParseError(type); + } + case SyntaxKind.ParenthesizedType: + return typeHasArrowFunctionBlockingParseError((node as ParenthesizedTypeNode).type); + default: + return false; + } + } + function parseThisTypePredicate(lhs: ThisTypeNode): TypePredicateNode { nextToken(); const node = createNode(SyntaxKind.TypePredicate, lhs.pos) as TypePredicateNode; @@ -2343,7 +2360,7 @@ namespace ts { return finishNode(parameter); } - function parseJSDocType() { + function parseJSDocType(): TypeNode { const dotdotdot = parseOptionalToken(SyntaxKind.DotDotDotToken); let type = parseType(); if (dotdotdot) { @@ -2451,6 +2468,7 @@ namespace ts { } /** + * Note: If returnToken is EqualsGreaterThanToken, `signature.type` will always be defined. * @returns If return type parsing succeeds */ function fillSignature( @@ -2460,12 +2478,12 @@ namespace ts { if (!(flags & SignatureFlags.JSDoc)) { signature.typeParameters = parseTypeParameters(); } - signature.parameters = parseParameterList(flags)!; // TODO: GH#18217 + const parametersParsedSuccessfully = parseParameterList(signature, flags); if (shouldParseReturnType(returnToken, !!(flags & SignatureFlags.Type))) { signature.type = parseTypeOrTypePredicate(); - return signature.type !== undefined; + if (typeHasArrowFunctionBlockingParseError(signature.type)) return false; } - return true; + return parametersParsedSuccessfully; } function shouldParseReturnType(returnToken: SyntaxKind.ColonToken | SyntaxKind.EqualsGreaterThanToken, isType: boolean): boolean { @@ -2485,7 +2503,8 @@ namespace ts { return false; } - function parseParameterList(flags: SignatureFlags) { + // Returns true on success. + function parseParameterList(signature: SignatureDeclaration, flags: SignatureFlags): boolean { // FormalParameters [Yield,Await]: (modified) // [empty] // FormalParameterList[?Yield,Await] @@ -2499,31 +2518,23 @@ namespace ts { // // SingleNameBinding [Yield,Await]: // BindingIdentifier[?Yield,?Await]Initializer [In, ?Yield,?Await] opt - if (parseExpected(SyntaxKind.OpenParenToken)) { - const savedYieldContext = inYieldContext(); - const savedAwaitContext = inAwaitContext(); - - setYieldContext(!!(flags & SignatureFlags.Yield)); - setAwaitContext(!!(flags & SignatureFlags.Await)); - - const result = parseDelimitedList(ParsingContext.Parameters, flags & SignatureFlags.JSDoc ? parseJSDocParameter : parseParameter); - - setYieldContext(savedYieldContext); - setAwaitContext(savedAwaitContext); - - if (!parseExpected(SyntaxKind.CloseParenToken) && (flags & SignatureFlags.RequireCompleteParameterList)) { - // Caller insisted that we had to end with a ) We didn't. So just return - // undefined here. - return undefined; - } - - return result; + if (!parseExpected(SyntaxKind.OpenParenToken)) { + signature.parameters = createMissingList(); + return false; } - // We didn't even have an open paren. If the caller requires a complete parameter list, - // we definitely can't provide that. However, if they're ok with an incomplete one, - // then just return an empty set of parameters. - return (flags & SignatureFlags.RequireCompleteParameterList) ? undefined : createMissingList(); + const savedYieldContext = inYieldContext(); + const savedAwaitContext = inAwaitContext(); + + setYieldContext(!!(flags & SignatureFlags.Yield)); + setAwaitContext(!!(flags & SignatureFlags.Await)); + + signature.parameters = parseDelimitedList(ParsingContext.Parameters, flags & SignatureFlags.JSDoc ? parseJSDocParameter : parseParameter); + + setYieldContext(savedYieldContext); + setAwaitContext(savedAwaitContext); + + return parseExpected(SyntaxKind.CloseParenToken); } function parseTypeMemberSemicolon() { @@ -2772,28 +2783,19 @@ namespace ts { return finishNode(node); } - function parseParenthesizedType(): ParenthesizedTypeNode { + function parseParenthesizedType(): TypeNode { const node = createNode(SyntaxKind.ParenthesizedType); parseExpected(SyntaxKind.OpenParenToken); node.type = parseType(); - if (!node.type) { - return undefined!; // TODO: GH#18217 - } parseExpected(SyntaxKind.CloseParenToken); return finishNode(node); } - function parseFunctionOrConstructorType(kind: SyntaxKind): FunctionOrConstructorTypeNode | undefined { - const node = createNodeWithJSDoc(kind); - if (kind === SyntaxKind.ConstructorType) { - parseExpected(SyntaxKind.NewKeyword); - } - if (!fillSignature(SyntaxKind.EqualsGreaterThanToken, SignatureFlags.Type | (sourceFile.languageVariant === LanguageVariant.JSX ? SignatureFlags.RequireCompleteParameterList : 0), node)) { - return undefined; - } - if (!node.parameters) { - return undefined; - } + function parseFunctionOrConstructorType(): TypeNode { + const pos = getNodePos(); + const kind = parseOptional(SyntaxKind.NewKeyword) ? SyntaxKind.ConstructorType : SyntaxKind.FunctionType; + const node = createNodeWithJSDoc(kind, pos); + fillSignature(SyntaxKind.EqualsGreaterThanToken, SignatureFlags.Type, node); return finishNode(node); } @@ -3134,11 +3136,8 @@ namespace ts { } function parseTypeWorker(noConditionalTypes?: boolean): TypeNode { - if (isStartOfFunctionType()) { - return parseFunctionOrConstructorType(SyntaxKind.FunctionType)!; // TODO: GH#18217 - } - if (token() === SyntaxKind.NewKeyword) { - return parseFunctionOrConstructorType(SyntaxKind.ConstructorType)!; + if (isStartOfFunctionType() || token() === SyntaxKind.NewKeyword) { + return parseFunctionOrConstructorType(); } const type = parseUnionTypeOrHigher(); if (!noConditionalTypes && !scanner.hasPrecedingLineBreak() && parseOptional(SyntaxKind.ExtendsKeyword)) { @@ -3626,12 +3625,7 @@ namespace ts { // a => (b => c) // And think that "(b =>" was actually a parenthesized arrow function with a missing // close paren. - if (!fillSignature(SyntaxKind.ColonToken, isAsync | (allowAmbiguity ? SignatureFlags.None : SignatureFlags.RequireCompleteParameterList), node)) { - return undefined; - } - - // If we couldn't get parameters, we definitely could not parse out an arrow function. - if (!node.parameters) { + if (!fillSignature(SyntaxKind.ColonToken, isAsync, node) && !allowAmbiguity) { return undefined; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 93162944fc..5f29444619 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1098,11 +1098,16 @@ namespace ts { export type FunctionOrConstructorTypeNode = FunctionTypeNode | ConstructorTypeNode; - export interface FunctionTypeNode extends TypeNode, SignatureDeclarationBase { + export interface FunctionOrConstructorTypeNodeBase extends TypeNode, SignatureDeclarationBase { + kind: SyntaxKind.FunctionType | SyntaxKind.ConstructorType; + type: TypeNode; + } + + export interface FunctionTypeNode extends FunctionOrConstructorTypeNodeBase { kind: SyntaxKind.FunctionType; } - export interface ConstructorTypeNode extends TypeNode, SignatureDeclarationBase { + export interface ConstructorTypeNode extends FunctionOrConstructorTypeNodeBase { kind: SyntaxKind.ConstructorType; } diff --git a/tests/baselines/reference/ArrowFunction3.errors.txt b/tests/baselines/reference/ArrowFunction3.errors.txt index aa5d1bfb72..8fd4bb742f 100644 --- a/tests/baselines/reference/ArrowFunction3.errors.txt +++ b/tests/baselines/reference/ArrowFunction3.errors.txt @@ -1,9 +1,15 @@ -tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,14): error TS1110: Type expected. +tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,10): error TS2304: Cannot find name 'a'. +tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,12): error TS1005: ',' expected. +tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts(1,14): error TS1005: ';' expected. -==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts (1 errors) ==== +==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts (3 errors) ==== var v = (a): => { + ~ +!!! error TS2304: Cannot find name 'a'. + ~ +!!! error TS1005: ',' expected. ~~ -!!! error TS1110: Type expected. +!!! error TS1005: ';' expected. }; \ No newline at end of file diff --git a/tests/baselines/reference/ArrowFunction3.js b/tests/baselines/reference/ArrowFunction3.js index 8ce76f7da3..41182da2ee 100644 --- a/tests/baselines/reference/ArrowFunction3.js +++ b/tests/baselines/reference/ArrowFunction3.js @@ -4,5 +4,7 @@ var v = (a): => { }; //// [ArrowFunction3.js] -var v = function (a) { -}; +var v = (a); +{ +} +; diff --git a/tests/baselines/reference/ArrowFunction3.symbols b/tests/baselines/reference/ArrowFunction3.symbols index 825b7d54e1..3eb9c6c895 100644 --- a/tests/baselines/reference/ArrowFunction3.symbols +++ b/tests/baselines/reference/ArrowFunction3.symbols @@ -1,6 +1,5 @@ === tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts === var v = (a): => { >v : Symbol(v, Decl(ArrowFunction3.ts, 0, 3)) ->a : Symbol(a, Decl(ArrowFunction3.ts, 0, 9)) }; diff --git a/tests/baselines/reference/ArrowFunction3.types b/tests/baselines/reference/ArrowFunction3.types index 64b46bd0d6..c168b83587 100644 --- a/tests/baselines/reference/ArrowFunction3.types +++ b/tests/baselines/reference/ArrowFunction3.types @@ -1,8 +1,7 @@ === tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction3.ts === var v = (a): => { ->v : (a: any) => any ->(a): => { } : (a: any) => any +>v : any +>(a) : any >a : any -> : No type information available! }; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index bf6db412bf..fdaec83c91 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -715,10 +715,14 @@ declare namespace ts { kind: SyntaxKind.ThisType; } type FunctionOrConstructorTypeNode = FunctionTypeNode | ConstructorTypeNode; - interface FunctionTypeNode extends TypeNode, SignatureDeclarationBase { + interface FunctionOrConstructorTypeNodeBase extends TypeNode, SignatureDeclarationBase { + kind: SyntaxKind.FunctionType | SyntaxKind.ConstructorType; + type: TypeNode; + } + interface FunctionTypeNode extends FunctionOrConstructorTypeNodeBase { kind: SyntaxKind.FunctionType; } - interface ConstructorTypeNode extends TypeNode, SignatureDeclarationBase { + interface ConstructorTypeNode extends FunctionOrConstructorTypeNodeBase { kind: SyntaxKind.ConstructorType; } interface NodeWithTypeArguments extends TypeNode { diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 32316e23f1..ac0322fa0d 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -715,10 +715,14 @@ declare namespace ts { kind: SyntaxKind.ThisType; } type FunctionOrConstructorTypeNode = FunctionTypeNode | ConstructorTypeNode; - interface FunctionTypeNode extends TypeNode, SignatureDeclarationBase { + interface FunctionOrConstructorTypeNodeBase extends TypeNode, SignatureDeclarationBase { + kind: SyntaxKind.FunctionType | SyntaxKind.ConstructorType; + type: TypeNode; + } + interface FunctionTypeNode extends FunctionOrConstructorTypeNodeBase { kind: SyntaxKind.FunctionType; } - interface ConstructorTypeNode extends TypeNode, SignatureDeclarationBase { + interface ConstructorTypeNode extends FunctionOrConstructorTypeNodeBase { kind: SyntaxKind.ConstructorType; } interface NodeWithTypeArguments extends TypeNode { diff --git a/tests/baselines/reference/jsdocFunctionTypeFalsePositive.errors.txt b/tests/baselines/reference/jsdocFunctionTypeFalsePositive.errors.txt new file mode 100644 index 0000000000..b50b49c2e4 --- /dev/null +++ b/tests/baselines/reference/jsdocFunctionTypeFalsePositive.errors.txt @@ -0,0 +1,24 @@ +/a.js(1,14): error TS1139: Type parameter declaration expected. +/a.js(1,17): error TS1003: Identifier expected. +/a.js(1,17): error TS1110: Type expected. +/a.js(1,17): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name. +/a.js(1,18): error TS1005: '>' expected. +/a.js(1,18): error TS1005: '}' expected. + + +==== /a.js (6 errors) ==== + /** @param {<} x */ + ~ +!!! error TS1139: Type parameter declaration expected. + +!!! error TS1003: Identifier expected. + +!!! error TS1110: Type expected. + +!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name. + +!!! error TS1005: '>' expected. + +!!! error TS1005: '}' expected. + function f(x) {} + \ No newline at end of file diff --git a/tests/baselines/reference/jsdocFunctionTypeFalsePositive.symbols b/tests/baselines/reference/jsdocFunctionTypeFalsePositive.symbols new file mode 100644 index 0000000000..ecbfb16a4a --- /dev/null +++ b/tests/baselines/reference/jsdocFunctionTypeFalsePositive.symbols @@ -0,0 +1,6 @@ +=== /a.js === +/** @param {<} x */ +function f(x) {} +>f : Symbol(f, Decl(a.js, 0, 0)) +>x : Symbol(x, Decl(a.js, 1, 11)) + diff --git a/tests/baselines/reference/jsdocFunctionTypeFalsePositive.types b/tests/baselines/reference/jsdocFunctionTypeFalsePositive.types new file mode 100644 index 0000000000..29b3b4d2e1 --- /dev/null +++ b/tests/baselines/reference/jsdocFunctionTypeFalsePositive.types @@ -0,0 +1,6 @@ +=== /a.js === +/** @param {<} x */ +function f(x) {} +>f : (x: any) => void +>x : any + diff --git a/tests/baselines/reference/parserX_ArrowFunction3.errors.txt b/tests/baselines/reference/parserX_ArrowFunction3.errors.txt index aa4a4c55a9..d0af7dbacc 100644 --- a/tests/baselines/reference/parserX_ArrowFunction3.errors.txt +++ b/tests/baselines/reference/parserX_ArrowFunction3.errors.txt @@ -1,9 +1,15 @@ -tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts(1,14): error TS1110: Type expected. +tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts(1,10): error TS2304: Cannot find name 'a'. +tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts(1,12): error TS1005: ',' expected. +tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts(1,14): error TS1005: ';' expected. -==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts (1 errors) ==== +==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts (3 errors) ==== var v = (a): => { + ~ +!!! error TS2304: Cannot find name 'a'. + ~ +!!! error TS1005: ',' expected. ~~ -!!! error TS1110: Type expected. +!!! error TS1005: ';' expected. }; \ No newline at end of file diff --git a/tests/baselines/reference/parserX_ArrowFunction3.js b/tests/baselines/reference/parserX_ArrowFunction3.js index 6d1ecad369..e326619585 100644 --- a/tests/baselines/reference/parserX_ArrowFunction3.js +++ b/tests/baselines/reference/parserX_ArrowFunction3.js @@ -4,5 +4,7 @@ var v = (a): => { }; //// [parserX_ArrowFunction3.js] -var v = function (a) { -}; +var v = (a); +{ +} +; diff --git a/tests/baselines/reference/parserX_ArrowFunction3.symbols b/tests/baselines/reference/parserX_ArrowFunction3.symbols index 92b4ff3927..37f59cb923 100644 --- a/tests/baselines/reference/parserX_ArrowFunction3.symbols +++ b/tests/baselines/reference/parserX_ArrowFunction3.symbols @@ -1,6 +1,5 @@ === tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts === var v = (a): => { >v : Symbol(v, Decl(parserX_ArrowFunction3.ts, 0, 3)) ->a : Symbol(a, Decl(parserX_ArrowFunction3.ts, 0, 9)) }; diff --git a/tests/baselines/reference/parserX_ArrowFunction3.types b/tests/baselines/reference/parserX_ArrowFunction3.types index 73e18eec5f..4c09ef0aa4 100644 --- a/tests/baselines/reference/parserX_ArrowFunction3.types +++ b/tests/baselines/reference/parserX_ArrowFunction3.types @@ -1,8 +1,7 @@ === tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction3.ts === var v = (a): => { ->v : (a: any) => any ->(a): => { } : (a: any) => any +>v : any +>(a) : any >a : any -> : No type information available! }; diff --git a/tests/cases/compiler/jsdocFunctionTypeFalsePositive.ts b/tests/cases/compiler/jsdocFunctionTypeFalsePositive.ts new file mode 100644 index 0000000000..194bdf876d --- /dev/null +++ b/tests/cases/compiler/jsdocFunctionTypeFalsePositive.ts @@ -0,0 +1,7 @@ +// @allowJs: true +// @checkJs: true +// @noEmit: true + +// @Filename: /a.js +/** @param {<} x */ +function f(x) {} diff --git a/tests/cases/fourslash/completionListNewIdentifierVariableDeclaration.ts b/tests/cases/fourslash/completionListNewIdentifierVariableDeclaration.ts index ff4fcba0a5..cdebbaa133 100644 --- a/tests/cases/fourslash/completionListNewIdentifierVariableDeclaration.ts +++ b/tests/cases/fourslash/completionListNewIdentifierVariableDeclaration.ts @@ -1,13 +1,7 @@ /// -////var x : (s/*1*/ - ////var y : (s:string, list/*2*/ -goTo.marker("1"); -verify.not.completionListIsEmpty();// As this can either be type or become arrow function parameter -verify.completionListAllowsNewIdentifier(); - goTo.marker("2"); verify.completionListIsEmpty(); // Parameter name verify.completionListAllowsNewIdentifier(); \ No newline at end of file