From 3fef5177dafa9b7aa04cf9bdf9e30a52348c5447 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 7 Jul 2020 15:27:03 -0700 Subject: [PATCH 1/2] More consistent checking of @property/@param 1. Use getWidenedTypeForVariableLikeDeclaration, instead of directly calling tryGetTypeFromEffectiveTypeNode. This requires some changes in the former function since it can't assume that the declaration has an initializer. 2. isOptional now calls isOptionalJSDocPropertyLikeTag. 3. isOptionalJSDocPropertyLikeTag now handles JSDocPropertyTag (previously it was named isOptionalJSDocParameterTag). --- src/compiler/checker.ts | 35 ++++++++++--------- .../reference/jsdocParamTagTypeLiteral.types | 16 ++++----- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0a39022e276fd..a75f2c59ad6fb 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7736,7 +7736,7 @@ namespace ts { } // Return the inferred type for a variable, parameter, or property declaration - function getTypeForVariableLikeDeclaration(declaration: ParameterDeclaration | PropertyDeclaration | PropertySignature | VariableDeclaration | BindingElement, includeOptionality: boolean): Type | undefined { + function getTypeForVariableLikeDeclaration(declaration: ParameterDeclaration | PropertyDeclaration | PropertySignature | VariableDeclaration | BindingElement | JSDocPropertyLikeTag, includeOptionality: boolean): Type | undefined { // A variable declared in a for..in statement is of type string, or of type keyof T when the // right hand expression is of a type parameter type. if (isVariableDeclaration(declaration) && declaration.parent.parent.kind === SyntaxKind.ForInStatement) { @@ -7759,6 +7759,7 @@ namespace ts { const isOptional = includeOptionality && ( isParameter(declaration) && isJSDocOptionalParameter(declaration) + || isOptionalJSDocParameterTag(declaration) || !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken); // Use type from type annotation if one is present @@ -7768,7 +7769,7 @@ namespace ts { } if ((noImplicitAny || isInJSFile(declaration)) && - declaration.kind === SyntaxKind.VariableDeclaration && !isBindingPattern(declaration.name) && + isVariableDeclaration(declaration) && !isBindingPattern(declaration.name) && !(getCombinedModifierFlags(declaration) & ModifierFlags.Export) && !(declaration.flags & NodeFlags.Ambient)) { // If --noImplicitAny is on or the declaration is in a Javascript file, // use control flow tracked 'any' type for non-ambient, non-exported var or let variables with no @@ -7783,7 +7784,7 @@ namespace ts { } } - if (declaration.kind === SyntaxKind.Parameter) { + if (isParameter(declaration)) { const func = declaration.parent; // For a parameter of a set accessor, use the type of the get accessor if one is present if (func.kind === SyntaxKind.SetAccessor && !hasNonBindableDynamicName(func)) { @@ -7811,16 +7812,16 @@ namespace ts { return addOptionality(type, isOptional); } } - else if (isInJSFile(declaration)) { - const containerObjectType = getJSContainerObjectType(declaration, getSymbolOfNode(declaration), getDeclaredExpandoInitializer(declaration)); - if (containerObjectType) { - return containerObjectType; - } - } // Use the type of the initializer expression if one is present and the declaration is // not a parameter of a contextually typed function - if (declaration.initializer) { + if (hasOnlyExpressionInitializer(declaration) && !!declaration.initializer) { + if (isInJSFile(declaration) && !isParameter(declaration)) { + const containerObjectType = getJSContainerObjectType(declaration, getSymbolOfNode(declaration), getDeclaredExpandoInitializer(declaration)); + if (containerObjectType) { + return containerObjectType; + } + } const type = widenTypeInferredFromInitializer(declaration, checkDeclarationInitializer(declaration)); return addOptionality(type, isOptional); } @@ -8240,7 +8241,7 @@ namespace ts { // Here, the array literal [1, "one"] is contextually typed by the type [any, string], which is the implied type of the // binding pattern [x, s = ""]. Because the contextual type is a tuple type, the resulting type of [1, "one"] is the // tuple type [number, string]. Thus, the type inferred for 'x' is number and the type inferred for 's' is string. - function getWidenedTypeForVariableLikeDeclaration(declaration: ParameterDeclaration | PropertyDeclaration | PropertySignature | VariableDeclaration | BindingElement, reportErrors?: boolean): Type { + function getWidenedTypeForVariableLikeDeclaration(declaration: ParameterDeclaration | PropertyDeclaration | PropertySignature | VariableDeclaration | BindingElement | JSDocPropertyLikeTag, reportErrors?: boolean): Type { return widenTypeForVariableLikeDeclaration(getTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ true), declaration, reportErrors); } @@ -8347,8 +8348,7 @@ namespace ts { (isCallExpression(declaration) || (isPropertyAccessExpression(declaration) || isBindableStaticElementAccessExpression(declaration)) && isBinaryExpression(declaration.parent)))) { type = getWidenedTypeForAssignmentDeclaration(symbol); } - else if (isJSDocPropertyLikeTag(declaration) - || isPropertyAccessExpression(declaration) + else if (isPropertyAccessExpression(declaration) || isElementAccessExpression(declaration) || isIdentifier(declaration) || isStringLiteralLike(declaration) @@ -8382,7 +8382,8 @@ namespace ts { || isPropertyDeclaration(declaration) || isPropertySignature(declaration) || isVariableDeclaration(declaration) - || isBindingElement(declaration)) { + || isBindingElement(declaration) + || isJSDocPropertyLikeTag(declaration)) { type = getWidenedTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ true); } // getTypeOfSymbol dispatches some JS merges incorrectly because their symbol flags are not mutually exclusive. @@ -11168,7 +11169,7 @@ namespace ts { return symbol && withAugmentations ? getMergedSymbol(symbol) : symbol; } - function isOptionalParameter(node: ParameterDeclaration | JSDocParameterTag) { + function isOptionalParameter(node: ParameterDeclaration | JSDocParameterTag | JSDocPropertyTag) { if (hasQuestionToken(node) || isOptionalJSDocParameterTag(node) || isJSDocOptionalParameter(node)) { return true; } @@ -11189,8 +11190,8 @@ namespace ts { return false; } - function isOptionalJSDocParameterTag(node: Node): node is JSDocParameterTag { - if (!isJSDocParameterTag(node)) { + function isOptionalJSDocParameterTag(node: Node): node is JSDocPropertyLikeTag { + if (!isJSDocPropertyLikeTag(node)) { return false; } const { isBracketed, typeExpression } = node; diff --git a/tests/baselines/reference/jsdocParamTagTypeLiteral.types b/tests/baselines/reference/jsdocParamTagTypeLiteral.types index 6a51e49ee6c15..ef926eec22214 100644 --- a/tests/baselines/reference/jsdocParamTagTypeLiteral.types +++ b/tests/baselines/reference/jsdocParamTagTypeLiteral.types @@ -23,18 +23,18 @@ normal(12); * @param {string} [opts1.w="hi"] doc5 */ function foo1(opts1) { ->foo1 : (opts1: { x: string; y?: string | undefined; z: string; w: string;}) => void ->opts1 : { x: string; y?: string | undefined; z?: string; w?: string; } +>foo1 : (opts1: { x: string; y?: string | undefined; z: string | undefined; w: string | undefined;}) => void +>opts1 : { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; } opts1.x; >opts1.x : string ->opts1 : { x: string; y?: string | undefined; z?: string; w?: string; } +>opts1 : { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; } >x : string } foo1({x: 'abc'}); >foo1({x: 'abc'}) : void ->foo1 : (opts1: { x: string; y?: string | undefined; z?: string; w?: string; }) => void +>foo1 : (opts1: { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; }) => void >{x: 'abc'} : { x: string; } >x : string >'abc' : "abc" @@ -93,19 +93,19 @@ foo3({x: 'abc'}); */ function foo4(opts4) { >foo4 : (opts4: { x: string; y?: string | undefined; z: string; w: string;}) => void ->opts4 : { x: string; y?: string | undefined; z?: string; w?: string; }[] +>opts4 : { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; }[] opts4[0].x; >opts4[0].x : string ->opts4[0] : { x: string; y?: string | undefined; z?: string; w?: string; } ->opts4 : { x: string; y?: string | undefined; z?: string; w?: string; }[] +>opts4[0] : { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; } +>opts4 : { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; }[] >0 : 0 >x : string } foo4([{ x: 'hi' }]); >foo4([{ x: 'hi' }]) : void ->foo4 : (opts4: { x: string; y?: string | undefined; z?: string; w?: string; }[]) => void +>foo4 : (opts4: { x: string; y?: string | undefined; z?: string | undefined; w?: string | undefined; }[]) => void >[{ x: 'hi' }] : { x: string; }[] >{ x: 'hi' } : { x: string; } >x : string From c11528a4bf0c3933466fc061101d3c5eb7f83a3c Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 7 Jul 2020 15:32:20 -0700 Subject: [PATCH 2/2] rename to isOptionalJSDocPropertyLikeTag --- src/compiler/checker.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a75f2c59ad6fb..2d3c941263b47 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7759,7 +7759,7 @@ namespace ts { const isOptional = includeOptionality && ( isParameter(declaration) && isJSDocOptionalParameter(declaration) - || isOptionalJSDocParameterTag(declaration) + || isOptionalJSDocPropertyLikeTag(declaration) || !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken); // Use type from type annotation if one is present @@ -11170,7 +11170,7 @@ namespace ts { } function isOptionalParameter(node: ParameterDeclaration | JSDocParameterTag | JSDocPropertyTag) { - if (hasQuestionToken(node) || isOptionalJSDocParameterTag(node) || isJSDocOptionalParameter(node)) { + if (hasQuestionToken(node) || isOptionalJSDocPropertyLikeTag(node) || isJSDocOptionalParameter(node)) { return true; } @@ -11190,7 +11190,7 @@ namespace ts { return false; } - function isOptionalJSDocParameterTag(node: Node): node is JSDocPropertyLikeTag { + function isOptionalJSDocPropertyLikeTag(node: Node): node is JSDocPropertyLikeTag { if (!isJSDocPropertyLikeTag(node)) { return false; } @@ -11299,7 +11299,7 @@ namespace ts { } // Record a new minimum argument count if this is not an optional parameter - const isOptionalParameter = isOptionalJSDocParameterTag(param) || + const isOptionalParameter = isOptionalJSDocPropertyLikeTag(param) || param.initializer || param.questionToken || param.dotDotDotToken || iife && parameters.length > iife.arguments.length && !type || isJSDocOptionalParameter(param);