Skip to content

Commit

Permalink
Better checking of @param/@Property tags (#39487)
Browse files Browse the repository at this point in the history 
* 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).

* rename to isOptionalJSDocPropertyLikeTag
  • Loading branch information
@sandersn
sandersn authored Jul 8, 2020
1 parent 48c19ee commit 007b82f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 27 deletions.
39 changes: 20 additions & 19 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7737,7 +7737,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) {
Expand All @@ -7760,6 +7760,7 @@ namespace ts {

const isOptional = includeOptionality && (
isParameter(declaration) && isJSDocOptionalParameter(declaration)
|| isOptionalJSDocPropertyLikeTag(declaration)
|| !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken);

// Use type from type annotation if one is present
Expand All @@ -7769,7 +7770,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
Expand All @@ -7784,7 +7785,7 @@ namespace ts {
}
}

if (declaration.kind === SyntaxKind.Parameter) {
if (isParameter(declaration)) {
const func = <FunctionLikeDeclaration>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)) {
Expand Down Expand Up @@ -7814,16 +7815,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);
}
Expand Down Expand Up @@ -8243,7 +8244,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);
}

Expand Down Expand Up @@ -8350,8 +8351,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)
Expand Down Expand Up @@ -8385,7 +8385,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.
Expand Down Expand Up @@ -11171,8 +11172,8 @@ namespace ts {
return symbol && withAugmentations ? getMergedSymbol(symbol) : symbol;
}

function isOptionalParameter(node: ParameterDeclaration | JSDocParameterTag) {
if (hasQuestionToken(node) || isOptionalJSDocParameterTag(node) || isJSDocOptionalParameter(node)) {
function isOptionalParameter(node: ParameterDeclaration | JSDocParameterTag | JSDocPropertyTag) {
if (hasQuestionToken(node) || isOptionalJSDocPropertyLikeTag(node) || isJSDocOptionalParameter(node)) {
return true;
}

Expand All @@ -11192,8 +11193,8 @@ namespace ts {
return false;
}

function isOptionalJSDocParameterTag(node: Node): node is JSDocParameterTag {
if (!isJSDocParameterTag(node)) {
function isOptionalJSDocPropertyLikeTag(node: Node): node is JSDocPropertyLikeTag {
if (!isJSDocPropertyLikeTag(node)) {
return false;
}
const { isBracketed, typeExpression } = node;
Expand Down Expand Up @@ -11301,7 +11302,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);
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/jsdocParamTagTypeLiteral.types
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 007b82f

Please sign in to comment.