From 35c6fbfee06c09493d9a7e0016a45bd3204cc5f8 Mon Sep 17 00:00:00 2001 From: Jack Bates Date: Tue, 9 Aug 2022 17:03:30 -0700 Subject: [PATCH] JSDoc @type tag optional parameters (#48132) * JSDoc @type tag optional parameters * Don't repeat isInJSFile() condition * Exclude variable initializers * Add tests for class methods * Don't contextually type JS function declarations * Update Baselines and/or Applied Lint Fixes * Reword comment Co-authored-by: TypeScript Bot --- src/compiler/checker.ts | 22 ++--- .../reference/checkJsdocTypeTag5.types | 4 +- .../reference/checkJsdocTypeTag6.errors.txt | 31 ++++++- .../reference/checkJsdocTypeTag6.symbols | 26 ++++++ .../reference/checkJsdocTypeTag6.types | 35 +++++++- .../reference/checkJsdocTypeTag7.symbols | 4 + .../reference/checkJsdocTypeTag7.types | 4 + .../jsDocDontBreakWithNamespaces.baseline | 81 ++++++++++++++++--- .../jsdocTypeTagRequiredParameters.errors.txt | 4 +- .../typeTagWithGenericSignature.types | 4 +- .../conformance/jsdoc/checkJsdocTypeTag6.ts | 17 ++++ .../conformance/jsdoc/checkJsdocTypeTag7.ts | 3 + 12 files changed, 208 insertions(+), 27 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 701747ce8de..f95addbadda 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9065,10 +9065,8 @@ namespace ts { return getReturnTypeOfSignature(getterSignature); } } - if (isInJSFile(declaration)) { - const type = getParameterTypeOfTypeTag(func, declaration); - if (type) return type; - } + const parameterTypeOfTypeTag = getParameterTypeOfTypeTag(func, declaration); + if (parameterTypeOfTypeTag) return parameterTypeOfTypeTag; // Use contextual parameter type if one is available const type = declaration.symbol.escapedName === InternalSymbolName.This ? getContextualThisParameterType(func) : getContextuallyTypedParameterType(declaration); if (type) { @@ -13117,7 +13115,14 @@ namespace ts { continue; } } - result.push(getSignatureFromDeclaration(decl)); + // If this is a function or method declaration, get the signature from the @type tag for the sake of optional parameters. + // Exclude contextually-typed kinds because we already apply the @type tag to the context, plus applying it here to the initializer would supress checks that the two are compatible. + result.push( + (!isFunctionExpressionOrArrowFunction(decl) && + !isObjectLiteralMethod(decl) && + getSignatureOfTypeTag(decl)) || + getSignatureFromDeclaration(decl) + ); } return result; } @@ -13152,7 +13157,7 @@ namespace ts { else { const type = signature.declaration && getEffectiveReturnTypeNode(signature.declaration); let jsdocPredicate: TypePredicate | undefined; - if (!type && isInJSFile(signature.declaration)) { + if (!type) { const jsdocSignature = getSignatureOfTypeTag(signature.declaration!); if (jsdocSignature && signature !== jsdocSignature) { jsdocPredicate = getTypePredicateOfSignature(jsdocSignature); @@ -17470,8 +17475,7 @@ namespace ts { } function isContextSensitiveFunctionLikeDeclaration(node: FunctionLikeDeclaration): boolean { - return (!isFunctionDeclaration(node) || isInJSFile(node) && !!getTypeForDeclarationFromJSDocComment(node)) && - (hasContextSensitiveParameters(node) || hasContextSensitiveReturnExpression(node)); + return hasContextSensitiveParameters(node) || hasContextSensitiveReturnExpression(node); } function hasContextSensitiveReturnExpression(node: FunctionLikeDeclaration) { @@ -17480,7 +17484,7 @@ namespace ts { } function isContextSensitiveFunctionOrObjectLiteralMethod(func: Node): func is FunctionExpression | ArrowFunction | MethodDeclaration { - return (isInJSFile(func) && isFunctionDeclaration(func) || isFunctionExpressionOrArrowFunction(func) || isObjectLiteralMethod(func)) && + return (isFunctionExpressionOrArrowFunction(func) || isObjectLiteralMethod(func)) && isContextSensitiveFunctionLikeDeclaration(func); } diff --git a/tests/baselines/reference/checkJsdocTypeTag5.types b/tests/baselines/reference/checkJsdocTypeTag5.types index 816f86a80a6..c2dca1475da 100644 --- a/tests/baselines/reference/checkJsdocTypeTag5.types +++ b/tests/baselines/reference/checkJsdocTypeTag5.types @@ -44,7 +44,7 @@ var k = function (x) { return x } /** @typedef {(x: 'hi' | 'bye') => 0 | 1 | 2} Argle */ /** @type {Argle} */ function blargle(s) { ->blargle : (s: "hi" | "bye") => 0 | 1 | 2 +>blargle : (x: 'hi' | 'bye') => 0 | 1 | 2 >s : "hi" | "bye" return 0; @@ -55,7 +55,7 @@ function blargle(s) { var zeroonetwo = blargle('hi') >zeroonetwo : 0 | 1 | 2 >blargle('hi') : 0 | 1 | 2 ->blargle : (s: "hi" | "bye") => 0 | 1 | 2 +>blargle : (x: "hi" | "bye") => 0 | 1 | 2 >'hi' : "hi" /** @typedef {{(s: string): 0 | 1; (b: boolean): 2 | 3 }} Gioconda */ diff --git a/tests/baselines/reference/checkJsdocTypeTag6.errors.txt b/tests/baselines/reference/checkJsdocTypeTag6.errors.txt index 1106e60da75..605ddad823a 100644 --- a/tests/baselines/reference/checkJsdocTypeTag6.errors.txt +++ b/tests/baselines/reference/checkJsdocTypeTag6.errors.txt @@ -1,9 +1,13 @@ tests/cases/conformance/jsdoc/test.js(1,12): error TS8030: The type of a function declaration must match the function's signature. tests/cases/conformance/jsdoc/test.js(7,5): error TS2322: Type '(prop: any) => void' is not assignable to type '{ prop: string; }'. tests/cases/conformance/jsdoc/test.js(10,12): error TS8030: The type of a function declaration must match the function's signature. +tests/cases/conformance/jsdoc/test.js(23,12): error TS8030: The type of a function declaration must match the function's signature. +tests/cases/conformance/jsdoc/test.js(27,7): error TS2322: Type '(more: any) => void' is not assignable to type '() => void'. +tests/cases/conformance/jsdoc/test.js(30,7): error TS2322: Type '(more: any) => void' is not assignable to type '() => void'. +tests/cases/conformance/jsdoc/test.js(34,3): error TS2322: Type '(more: any) => void' is not assignable to type '() => void'. -==== tests/cases/conformance/jsdoc/test.js (3 errors) ==== +==== tests/cases/conformance/jsdoc/test.js (7 errors) ==== /** @type {number} */ ~~~~~~ !!! error TS8030: The type of a function declaration must match the function's signature. @@ -28,4 +32,29 @@ tests/cases/conformance/jsdoc/test.js(10,12): error TS8030: The type of a functi // TODO: Should be an error since signature doesn't match. /** @type {(a: number, b: number, c: number) => number} */ function add3(a, b) { return a + b; } + + // Confirm initializers are compatible. + // They can't have more parameters than the type/context. + + /** @type {() => void} */ + ~~~~~~~~~~ +!!! error TS8030: The type of a function declaration must match the function's signature. + function funcWithMoreParameters(more) {} // error + + /** @type {() => void} */ + const variableWithMoreParameters = function (more) {}; // error + ~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2322: Type '(more: any) => void' is not assignable to type '() => void'. + + /** @type {() => void} */ + const arrowWithMoreParameters = (more) => {}; // error + ~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2322: Type '(more: any) => void' is not assignable to type '() => void'. + + ({ + /** @type {() => void} */ + methodWithMoreParameters(more) {}, // error + ~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2322: Type '(more: any) => void' is not assignable to type '() => void'. + }); \ No newline at end of file diff --git a/tests/baselines/reference/checkJsdocTypeTag6.symbols b/tests/baselines/reference/checkJsdocTypeTag6.symbols index b7ddd9f289b..e87b49f2a1d 100644 --- a/tests/baselines/reference/checkJsdocTypeTag6.symbols +++ b/tests/baselines/reference/checkJsdocTypeTag6.symbols @@ -37,3 +37,29 @@ function add3(a, b) { return a + b; } >a : Symbol(a, Decl(test.js, 17, 14)) >b : Symbol(b, Decl(test.js, 17, 16)) +// Confirm initializers are compatible. +// They can't have more parameters than the type/context. + +/** @type {() => void} */ +function funcWithMoreParameters(more) {} // error +>funcWithMoreParameters : Symbol(funcWithMoreParameters, Decl(test.js, 17, 37)) +>more : Symbol(more, Decl(test.js, 23, 32)) + +/** @type {() => void} */ +const variableWithMoreParameters = function (more) {}; // error +>variableWithMoreParameters : Symbol(variableWithMoreParameters, Decl(test.js, 26, 5)) +>more : Symbol(more, Decl(test.js, 26, 45)) + +/** @type {() => void} */ +const arrowWithMoreParameters = (more) => {}; // error +>arrowWithMoreParameters : Symbol(arrowWithMoreParameters, Decl(test.js, 29, 5)) +>more : Symbol(more, Decl(test.js, 29, 33)) + +({ + /** @type {() => void} */ + methodWithMoreParameters(more) {}, // error +>methodWithMoreParameters : Symbol(methodWithMoreParameters, Decl(test.js, 31, 2)) +>more : Symbol(more, Decl(test.js, 33, 27)) + +}); + diff --git a/tests/baselines/reference/checkJsdocTypeTag6.types b/tests/baselines/reference/checkJsdocTypeTag6.types index 16f22e1332b..3feb2222e29 100644 --- a/tests/baselines/reference/checkJsdocTypeTag6.types +++ b/tests/baselines/reference/checkJsdocTypeTag6.types @@ -16,7 +16,7 @@ var g = function (prop) { /** @type {(a: number) => number} */ function add1(a, b) { return a + b; } ->add1 : (a: number, b: any) => number +>add1 : (a: number) => number >a : number >b : any >a + b : any @@ -35,10 +35,41 @@ function add2(a, b) { return a + b; } // TODO: Should be an error since signature doesn't match. /** @type {(a: number, b: number, c: number) => number} */ function add3(a, b) { return a + b; } ->add3 : (a: number, b: number) => number +>add3 : (a: number, b: number, c: number) => number >a : number >b : number >a + b : number >a : number >b : number +// Confirm initializers are compatible. +// They can't have more parameters than the type/context. + +/** @type {() => void} */ +function funcWithMoreParameters(more) {} // error +>funcWithMoreParameters : () => void +>more : any + +/** @type {() => void} */ +const variableWithMoreParameters = function (more) {}; // error +>variableWithMoreParameters : () => void +>function (more) {} : (more: any) => void +>more : any + +/** @type {() => void} */ +const arrowWithMoreParameters = (more) => {}; // error +>arrowWithMoreParameters : () => void +>(more) => {} : (more: any) => void +>more : any + +({ +>({ /** @type {() => void} */ methodWithMoreParameters(more) {}, // error}) : { methodWithMoreParameters(): void; } +>{ /** @type {() => void} */ methodWithMoreParameters(more) {}, // error} : { methodWithMoreParameters(): void; } + + /** @type {() => void} */ + methodWithMoreParameters(more) {}, // error +>methodWithMoreParameters : (more: any) => void +>more : any + +}); + diff --git a/tests/baselines/reference/checkJsdocTypeTag7.symbols b/tests/baselines/reference/checkJsdocTypeTag7.symbols index 7013bb921a5..50b91e09845 100644 --- a/tests/baselines/reference/checkJsdocTypeTag7.symbols +++ b/tests/baselines/reference/checkJsdocTypeTag7.symbols @@ -11,5 +11,9 @@ class C { >foo : Symbol(C.foo, Decl(test.js, 4, 9)) >a : Symbol(a, Decl(test.js, 6, 8)) >b : Symbol(b, Decl(test.js, 6, 10)) + + /** @type {(optional?) => void} */ + methodWithOptionalParameters() {} +>methodWithOptionalParameters : Symbol(C.methodWithOptionalParameters, Decl(test.js, 6, 16)) } diff --git a/tests/baselines/reference/checkJsdocTypeTag7.types b/tests/baselines/reference/checkJsdocTypeTag7.types index f4aac580fee..990535c17e4 100644 --- a/tests/baselines/reference/checkJsdocTypeTag7.types +++ b/tests/baselines/reference/checkJsdocTypeTag7.types @@ -11,5 +11,9 @@ class C { >foo : (a: string, b: number) => void >a : string >b : number + + /** @type {(optional?) => void} */ + methodWithOptionalParameters() {} +>methodWithOptionalParameters : (optional?: any) => void } diff --git a/tests/baselines/reference/jsDocDontBreakWithNamespaces.baseline b/tests/baselines/reference/jsDocDontBreakWithNamespaces.baseline index 3618b914c58..df5d7365580 100644 --- a/tests/baselines/reference/jsDocDontBreakWithNamespaces.baseline +++ b/tests/baselines/reference/jsDocDontBreakWithNamespaces.baseline @@ -181,19 +181,82 @@ "kind": "space" } ], - "parameters": [], - "documentation": [], - "tags": [ + "parameters": [ { - "name": "type", - "text": [ + "name": "arg0", + "documentation": [], + "displayParts": [ { - "text": "{function(module:xxxx, module:xxxx): module:xxxxx}", - "kind": "text" + "text": "arg0", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" } - ] + ], + "isOptional": false, + "isRest": false + }, + { + "name": "arg1", + "documentation": [], + "displayParts": [ + { + "text": "arg1", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false + }, + { + "name": "arg2", + "documentation": [], + "displayParts": [ + { + "text": "arg2", + "kind": "parameterName" + }, + { + "text": ":", + "kind": "punctuation" + }, + { + "text": " ", + "kind": "space" + }, + { + "text": "any", + "kind": "keyword" + } + ], + "isOptional": false, + "isRest": false } - ] + ], + "documentation": [], + "tags": [] } ], "applicableSpan": { diff --git a/tests/baselines/reference/jsdocTypeTagRequiredParameters.errors.txt b/tests/baselines/reference/jsdocTypeTagRequiredParameters.errors.txt index 3658797f87f..22379631294 100644 --- a/tests/baselines/reference/jsdocTypeTagRequiredParameters.errors.txt +++ b/tests/baselines/reference/jsdocTypeTagRequiredParameters.errors.txt @@ -21,9 +21,9 @@ tests/cases/conformance/jsdoc/a.js(13,1): error TS2554: Expected 1 arguments, bu g() // should error ~~~ !!! error TS2554: Expected 1 arguments, but got 0. -!!! related TS6210 tests/cases/conformance/jsdoc/a.js:5:12: An argument for 's' was not provided. +!!! related TS6210 tests/cases/conformance/jsdoc/a.js:4:13: An argument for 's' was not provided. h() ~~~ !!! error TS2554: Expected 1 arguments, but got 0. -!!! related TS6210 tests/cases/conformance/jsdoc/a.js:8:12: An argument for 's' was not provided. +!!! related TS6210 tests/cases/conformance/jsdoc/a.js:7:14: An argument for 's' was not provided. \ No newline at end of file diff --git a/tests/baselines/reference/typeTagWithGenericSignature.types b/tests/baselines/reference/typeTagWithGenericSignature.types index 585fd49fbf4..a5b92a7db86 100644 --- a/tests/baselines/reference/typeTagWithGenericSignature.types +++ b/tests/baselines/reference/typeTagWithGenericSignature.types @@ -1,7 +1,7 @@ === tests/cases/conformance/jsdoc/bug25618.js === /** @type {(param?: T) => T | undefined} */ function typed(param) { ->typed : (param: T | undefined) => T | undefined +>typed : (param?: T | undefined) => T | undefined >param : T | undefined return param; @@ -11,7 +11,7 @@ function typed(param) { var n = typed(1); >n : number | undefined >typed(1) : 1 | undefined ->typed : (param: T | undefined) => T | undefined +>typed : (param?: T | undefined) => T | undefined >1 : 1 diff --git a/tests/cases/conformance/jsdoc/checkJsdocTypeTag6.ts b/tests/cases/conformance/jsdoc/checkJsdocTypeTag6.ts index 5f629d42b3b..73b387c203e 100644 --- a/tests/cases/conformance/jsdoc/checkJsdocTypeTag6.ts +++ b/tests/cases/conformance/jsdoc/checkJsdocTypeTag6.ts @@ -21,3 +21,20 @@ function add2(a, b) { return a + b; } // TODO: Should be an error since signature doesn't match. /** @type {(a: number, b: number, c: number) => number} */ function add3(a, b) { return a + b; } + +// Confirm initializers are compatible. +// They can't have more parameters than the type/context. + +/** @type {() => void} */ +function funcWithMoreParameters(more) {} // error + +/** @type {() => void} */ +const variableWithMoreParameters = function (more) {}; // error + +/** @type {() => void} */ +const arrowWithMoreParameters = (more) => {}; // error + +({ + /** @type {() => void} */ + methodWithMoreParameters(more) {}, // error +}); diff --git a/tests/cases/conformance/jsdoc/checkJsdocTypeTag7.ts b/tests/cases/conformance/jsdoc/checkJsdocTypeTag7.ts index c65b206a859..2981292ba7e 100644 --- a/tests/cases/conformance/jsdoc/checkJsdocTypeTag7.ts +++ b/tests/cases/conformance/jsdoc/checkJsdocTypeTag7.ts @@ -10,4 +10,7 @@ class C { /** @type {Foo} */ foo(a, b) {} + + /** @type {(optional?) => void} */ + methodWithOptionalParameters() {} }