From 9c84ccf8a0981360c9143ff564c162d2190c798f Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 13 Mar 2018 13:12:51 -0700 Subject: [PATCH 1/2] Brackets and postfix= in `@param` add undefined Previously they only added optionality. Note that, unlike Typescript, when a parameter initializer is specified in jsdoc, it does not remove undefined in the *body* of the function. That's because TS will generate initialisation code, but JS won't, so the author will have to manually write code to remove undefined from the type. ```js /** @param {number} [a=101] */ function f(a) { // a: number | undefined here if (!a) { a = 101 } // a: number here } ``` Note that we don't check that 1. the initializer value is actually assigned to the parameter. 2. the initializer's type matches the declared type of the parameter. Pretty much we just parse it and leave it alone. --- src/compiler/checker.ts | 17 ++++++- .../reference/jsdocParamTagTypeLiteral.types | 28 +++++------ ...ramTagBracketsAddOptionalUndefined.symbols | 38 +++++++++++++++ ...paramTagBracketsAddOptionalUndefined.types | 47 +++++++++++++++++++ .../paramTagBracketsAddOptionalUndefined.ts | 21 +++++++++ 5 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 tests/baselines/reference/paramTagBracketsAddOptionalUndefined.symbols create mode 100644 tests/baselines/reference/paramTagBracketsAddOptionalUndefined.types create mode 100644 tests/cases/conformance/jsdoc/paramTagBracketsAddOptionalUndefined.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a86f18e4cceac..cb0a2f59addd5 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4165,7 +4165,19 @@ namespace ts { return getTypeForBindingElement(declaration); } - const isOptional = !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken && includeOptionality; + let isOptional = false; + if (includeOptionality) { + if (isInJavaScriptFile(declaration) && isParameterDeclaration(declaration)) { + const parameterTags = getJSDocParameterTags(declaration as ParameterDeclaration); + if (parameterTags && parameterTags.length > 0 && find(parameterTags, tag => tag.isBracketed)) { + isOptional = true; + } + } + if (!isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken) { + isOptional = true; + } + } + // Use type from type annotation if one is present const declaredType = tryGetTypeFromEffectiveTypeNode(declaration); if (declaredType) { @@ -8668,9 +8680,10 @@ namespace ts { return getTypeFromIntersectionTypeNode(node); case SyntaxKind.JSDocNullableType: return getTypeFromJSDocNullableTypeNode(node); + case SyntaxKind.JSDocOptionalType: + return addOptionality(getTypeFromTypeNode((node as JSDocOptionalType).type)); case SyntaxKind.ParenthesizedType: case SyntaxKind.JSDocNonNullableType: - case SyntaxKind.JSDocOptionalType: case SyntaxKind.JSDocTypeExpression: return getTypeFromTypeNode((node).type); case SyntaxKind.JSDocVariadicType: diff --git a/tests/baselines/reference/jsdocParamTagTypeLiteral.types b/tests/baselines/reference/jsdocParamTagTypeLiteral.types index c705647c9d2f5..35b472e81bec5 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; z?: string; w?: string; }) => void ->opts1 : { x: string; y?: string; z?: string; w?: string; } +>foo1 : (opts1: { x: string; y?: string | undefined; z?: string; w?: string; }) => void +>opts1 : { x: string; y?: string | undefined; z?: string; w?: string; } opts1.x; >opts1.x : string ->opts1 : { x: string; y?: string; z?: string; w?: string; } +>opts1 : { x: string; y?: string | undefined; z?: string; w?: string; } >x : string } foo1({x: 'abc'}); >foo1({x: 'abc'}) : void ->foo1 : (opts1: { x: string; y?: string; z?: string; w?: string; }) => void +>foo1 : (opts1: { x: string; y?: string | undefined; z?: string; w?: string; }) => void >{x: 'abc'} : { x: string; } >x : string >'abc' : "abc" @@ -45,20 +45,20 @@ foo1({x: 'abc'}); * @param {string=} opts2[].anotherY */ function foo2(/** @param opts2 bad idea theatre! */opts2) { ->foo2 : (opts2: { anotherX: string; anotherY?: string; }[]) => void ->opts2 : { anotherX: string; anotherY?: string; }[] +>foo2 : (opts2: { anotherX: string; anotherY?: string | undefined; }[]) => void +>opts2 : { anotherX: string; anotherY?: string | undefined; }[] opts2[0].anotherX; >opts2[0].anotherX : string ->opts2[0] : { anotherX: string; anotherY?: string; } ->opts2 : { anotherX: string; anotherY?: string; }[] +>opts2[0] : { anotherX: string; anotherY?: string | undefined; } +>opts2 : { anotherX: string; anotherY?: string | undefined; }[] >0 : 0 >anotherX : string } foo2([{anotherX: "world"}]); >foo2([{anotherX: "world"}]) : void ->foo2 : (opts2: { anotherX: string; anotherY?: string; }[]) => void +>foo2 : (opts2: { anotherX: string; anotherY?: string | undefined; }[]) => void >[{anotherX: "world"}] : { anotherX: string; }[] >{anotherX: "world"} : { anotherX: string; } >anotherX : string @@ -92,20 +92,20 @@ foo3({x: 'abc'}); * @param {string} [opts4[].w="hi"] */ function foo4(opts4) { ->foo4 : (opts4: { x: string; y?: string; z?: string; w?: string; }[]) => void ->opts4 : { x: string; y?: string; z?: string; w?: string; }[] +>foo4 : (opts4: { x: string; y?: string | undefined; z?: string; w?: string; }[]) => void +>opts4 : { x: string; y?: string | undefined; z?: string; w?: string; }[] opts4[0].x; >opts4[0].x : string ->opts4[0] : { x: string; y?: string; z?: string; w?: string; } ->opts4 : { x: string; y?: string; z?: string; w?: string; }[] +>opts4[0] : { x: string; y?: string | undefined; z?: string; w?: string; } +>opts4 : { x: string; y?: string | undefined; z?: string; w?: string; }[] >0 : 0 >x : string } foo4([{ x: 'hi' }]); >foo4([{ x: 'hi' }]) : void ->foo4 : (opts4: { x: string; y?: string; z?: string; w?: string; }[]) => void +>foo4 : (opts4: { x: string; y?: string | undefined; z?: string; w?: string; }[]) => void >[{ x: 'hi' }] : { x: string; }[] >{ x: 'hi' } : { x: string; } >x : string diff --git a/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.symbols b/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.symbols new file mode 100644 index 0000000000000..fd2e2ceeedad2 --- /dev/null +++ b/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.symbols @@ -0,0 +1,38 @@ +=== tests/cases/conformance/jsdoc/a.js === +/** + * @param {number} [p] + * @param {number=} q + * @param {number} [r=101] + */ +function f(p, q, r) { +>f : Symbol(f, Decl(a.js, 0, 0)) +>p : Symbol(p, Decl(a.js, 5, 11)) +>q : Symbol(q, Decl(a.js, 5, 13)) +>r : Symbol(r, Decl(a.js, 5, 16)) + + p = undefined +>p : Symbol(p, Decl(a.js, 5, 11)) +>undefined : Symbol(undefined) + + q = undefined +>q : Symbol(q, Decl(a.js, 5, 13)) +>undefined : Symbol(undefined) + + // note that, unlike TS, JSDOC [r=101] retains | undefined because + // there's no code emitted to get rid of it. + r = undefined +>r : Symbol(r, Decl(a.js, 5, 16)) +>undefined : Symbol(undefined) +} +f() +>f : Symbol(f, Decl(a.js, 0, 0)) + +f(undefined, undefined, undefined) +>f : Symbol(f, Decl(a.js, 0, 0)) +>undefined : Symbol(undefined) +>undefined : Symbol(undefined) +>undefined : Symbol(undefined) + +f(1, 2, 3) +>f : Symbol(f, Decl(a.js, 0, 0)) + diff --git a/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.types b/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.types new file mode 100644 index 0000000000000..f8f531b3e2756 --- /dev/null +++ b/tests/baselines/reference/paramTagBracketsAddOptionalUndefined.types @@ -0,0 +1,47 @@ +=== tests/cases/conformance/jsdoc/a.js === +/** + * @param {number} [p] + * @param {number=} q + * @param {number} [r=101] + */ +function f(p, q, r) { +>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void +>p : number | undefined +>q : number | undefined +>r : number | undefined + + p = undefined +>p = undefined : undefined +>p : number | undefined +>undefined : undefined + + q = undefined +>q = undefined : undefined +>q : number | undefined +>undefined : undefined + + // note that, unlike TS, JSDOC [r=101] retains | undefined because + // there's no code emitted to get rid of it. + r = undefined +>r = undefined : undefined +>r : number | undefined +>undefined : undefined +} +f() +>f() : void +>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void + +f(undefined, undefined, undefined) +>f(undefined, undefined, undefined) : void +>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void +>undefined : undefined +>undefined : undefined +>undefined : undefined + +f(1, 2, 3) +>f(1, 2, 3) : void +>f : (p?: number | undefined, q?: number | undefined, r?: number | undefined) => void +>1 : 1 +>2 : 2 +>3 : 3 + diff --git a/tests/cases/conformance/jsdoc/paramTagBracketsAddOptionalUndefined.ts b/tests/cases/conformance/jsdoc/paramTagBracketsAddOptionalUndefined.ts new file mode 100644 index 0000000000000..e7ba7feae4629 --- /dev/null +++ b/tests/cases/conformance/jsdoc/paramTagBracketsAddOptionalUndefined.ts @@ -0,0 +1,21 @@ +// @noEmit: true +// @allowJs: true +// @checkJs: true +// @strict: true +// @Filename: a.js + +/** + * @param {number} [p] + * @param {number=} q + * @param {number} [r=101] + */ +function f(p, q, r) { + p = undefined + q = undefined + // note that, unlike TS, JSDOC [r=101] retains | undefined because + // there's no code emitted to get rid of it. + r = undefined +} +f() +f(undefined, undefined, undefined) +f(1, 2, 3) From 7cb96ee6dd9ec9d041418d29fbf9f78a424c61e9 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com> Date: Tue, 13 Mar 2018 15:20:59 -0700 Subject: [PATCH 2/2] Address PR comments --- src/compiler/checker.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index cb0a2f59addd5..3eb8267de7999 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4167,11 +4167,9 @@ namespace ts { let isOptional = false; if (includeOptionality) { - if (isInJavaScriptFile(declaration) && isParameterDeclaration(declaration)) { - const parameterTags = getJSDocParameterTags(declaration as ParameterDeclaration); - if (parameterTags && parameterTags.length > 0 && find(parameterTags, tag => tag.isBracketed)) { - isOptional = true; - } + if (isInJavaScriptFile(declaration) && isParameter(declaration)) { + const parameterTags = getJSDocParameterTags(declaration); + isOptional = !!(parameterTags && parameterTags.length > 0 && find(parameterTags, tag => tag.isBracketed)); } if (!isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken) { isOptional = true;