Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better checking of @param/@property tags #39487

Merged
merged 2 commits into from
Jul 8, 2020
Merged

Better checking of @param/@property tags #39487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fef517
More consistent checking of @property/@param
sandersn Jul 7, 2020
c11528a
rename to isOptionalJSDocPropertyLikeTag
sandersn Jul 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 20 additions & 19 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -7759,6 +7759,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 @@ -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
Expand All @@ -7783,7 +7784,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 @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -11168,8 +11169,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 @@ -11189,8 +11190,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 @@ -11298,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);
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/jsdocParamTagTypeLiteral.types
View file
Open in desktop
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