From 7f652509b65da89355fedcc0ef3b707cc6d79e58 Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Wed, 16 Mar 2022 11:04:07 -0700 Subject: [PATCH] Handle JS synthetic rest args in contextual parameter assignment (#48263) * Handle JS synthetic rest args in contextual parameter assignment * Limit fixing to only unannotated js rest parameters * Minimize test * Add annotated version * Remove explicit CheckFlags.RestParameter check since apparently not all rest parameters are CheckFlags.RestParameter --- src/compiler/checker.ts | 27 +++++++++++++---- ...ameterReassignmentIIFEAnnotated.errors.txt | 15 ++++++++++ ...ParameterReassignmentIIFEAnnotated.symbols | 21 ++++++++++++++ ...noParameterReassignmentIIFEAnnotated.types | 29 +++++++++++++++++++ .../noParameterReassignmentJSIIFE.symbols | 18 ++++++++++++ .../noParameterReassignmentJSIIFE.types | 26 +++++++++++++++++ .../noParameterReassignmentIIFEAnnotated.ts | 12 ++++++++ .../compiler/noParameterReassignmentJSIIFE.ts | 9 ++++++ 8 files changed, 152 insertions(+), 5 deletions(-) create mode 100644 tests/baselines/reference/noParameterReassignmentIIFEAnnotated.errors.txt create mode 100644 tests/baselines/reference/noParameterReassignmentIIFEAnnotated.symbols create mode 100644 tests/baselines/reference/noParameterReassignmentIIFEAnnotated.types create mode 100644 tests/baselines/reference/noParameterReassignmentJSIIFE.symbols create mode 100644 tests/baselines/reference/noParameterReassignmentJSIIFE.types create mode 100644 tests/cases/compiler/noParameterReassignmentIIFEAnnotated.ts create mode 100644 tests/cases/compiler/noParameterReassignmentJSIIFE.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6268e17945c..786343fdac4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -12862,7 +12862,19 @@ namespace ts { p.typeExpression && isJSDocVariadicType(p.typeExpression.type) ? p.typeExpression.type : undefined); const syntheticArgsSymbol = createSymbol(SymbolFlags.Variable, "args" as __String, CheckFlags.RestParameter); - syntheticArgsSymbol.type = lastParamVariadicType ? createArrayType(getTypeFromTypeNode(lastParamVariadicType.type)) : anyArrayType; + if (lastParamVariadicType) { + // Parameter has effective annotation, lock in type + syntheticArgsSymbol.type = createArrayType(getTypeFromTypeNode(lastParamVariadicType.type)); + } + else { + // Parameter has no annotation + // By using a `DeferredType` symbol, we allow the type of this rest arg to be overriden by contextual type assignment so long as its type hasn't been + // cached by `getTypeOfSymbol` yet. + syntheticArgsSymbol.checkFlags |= CheckFlags.DeferredType; + syntheticArgsSymbol.deferralParent = neverType; + syntheticArgsSymbol.deferralConstituents = [anyArrayType]; + syntheticArgsSymbol.deferralWriteConstituents = [anyArrayType]; + } if (lastParamVariadicType) { // Replace the last parameter with a rest parameter. parameters.pop(); @@ -32163,7 +32175,12 @@ namespace ts { if (signatureHasRestParameter(signature)) { // parameter might be a transient symbol generated by use of `arguments` in the function body. const parameter = last(signature.parameters); - if (isTransientSymbol(parameter) || !getEffectiveTypeAnnotationNode(parameter.valueDeclaration as ParameterDeclaration)) { + if (parameter.valueDeclaration + ? !getEffectiveTypeAnnotationNode(parameter.valueDeclaration as ParameterDeclaration) + // a declarationless parameter may still have a `.type` already set by its construction logic + // (which may pull a type from a jsdoc) - only allow fixing on `DeferredType` parameters with a fallback type + : !!(getCheckFlags(parameter) & CheckFlags.DeferredType) + ) { const contextualParameterType = getRestTypeAtPosition(context, len); assignParameterType(parameter, contextualParameterType); } @@ -32182,9 +32199,9 @@ namespace ts { function assignParameterType(parameter: Symbol, type?: Type) { const links = getSymbolLinks(parameter); if (!links.type) { - const declaration = parameter.valueDeclaration as ParameterDeclaration; - links.type = type || getWidenedTypeForVariableLikeDeclaration(declaration, /*reportErrors*/ true); - if (declaration.name.kind !== SyntaxKind.Identifier) { + const declaration = parameter.valueDeclaration as ParameterDeclaration | undefined; + links.type = type || (declaration ? getWidenedTypeForVariableLikeDeclaration(declaration, /*reportErrors*/ true) : getTypeOfSymbol(parameter)); + if (declaration && declaration.name.kind !== SyntaxKind.Identifier) { // if inference didn't come up with anything but unknown, fall back to the binding pattern if present. if (links.type === unknownType) { links.type = getTypeFromBindingPattern(declaration.name); diff --git a/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.errors.txt b/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.errors.txt new file mode 100644 index 00000000000..a3d7905a774 --- /dev/null +++ b/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.errors.txt @@ -0,0 +1,15 @@ +tests/cases/compiler/index.js(3,28): error TS8029: JSDoc '@param' tag has name 'rest', but there is no parameter with that name. It would match 'arguments' if it had an array type. + + +==== tests/cases/compiler/index.js (1 errors) ==== + self.importScripts = (function (importScripts) { + /** + * @param {...unknown} rest + ~~~~ +!!! error TS8029: JSDoc '@param' tag has name 'rest', but there is no parameter with that name. It would match 'arguments' if it had an array type. + */ + return function () { + return importScripts.apply(this, arguments); + }; + })(importScripts); + \ No newline at end of file diff --git a/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.symbols b/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.symbols new file mode 100644 index 00000000000..f12c4e640aa --- /dev/null +++ b/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.symbols @@ -0,0 +1,21 @@ +=== tests/cases/compiler/index.js === +self.importScripts = (function (importScripts) { +>self.importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --)) +>self : Symbol(self, Decl(lib.dom.d.ts, --, --), Decl(index.js, 0, 0)) +>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --)) +>importScripts : Symbol(importScripts, Decl(index.js, 0, 32)) + + /** + * @param {...unknown} rest + */ + return function () { + return importScripts.apply(this, arguments); +>importScripts.apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --)) +>importScripts : Symbol(importScripts, Decl(index.js, 0, 32)) +>apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --)) +>arguments : Symbol(arguments) + + }; +})(importScripts); +>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --)) + diff --git a/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.types b/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.types new file mode 100644 index 00000000000..ce2101e1c1c --- /dev/null +++ b/tests/baselines/reference/noParameterReassignmentIIFEAnnotated.types @@ -0,0 +1,29 @@ +=== tests/cases/compiler/index.js === +self.importScripts = (function (importScripts) { +>self.importScripts = (function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: unknown[]) => any +>self.importScripts : (...urls: string[]) => void +>self : Window & typeof globalThis +>importScripts : (...urls: string[]) => void +>(function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: unknown[]) => any +>(function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };}) : (importScripts: (...urls: string[]) => void) => (...args: unknown[]) => any +>function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };} : (importScripts: (...urls: string[]) => void) => (...args: unknown[]) => any +>importScripts : (...urls: string[]) => void + + /** + * @param {...unknown} rest + */ + return function () { +>function () { return importScripts.apply(this, arguments); } : (...args: unknown[]) => any + + return importScripts.apply(this, arguments); +>importScripts.apply(this, arguments) : any +>importScripts.apply : (this: Function, thisArg: any, argArray?: any) => any +>importScripts : (...urls: string[]) => void +>apply : (this: Function, thisArg: any, argArray?: any) => any +>this : any +>arguments : IArguments + + }; +})(importScripts); +>importScripts : (...urls: string[]) => void + diff --git a/tests/baselines/reference/noParameterReassignmentJSIIFE.symbols b/tests/baselines/reference/noParameterReassignmentJSIIFE.symbols new file mode 100644 index 00000000000..339cdfda0e5 --- /dev/null +++ b/tests/baselines/reference/noParameterReassignmentJSIIFE.symbols @@ -0,0 +1,18 @@ +=== tests/cases/compiler/index.js === +self.importScripts = (function (importScripts) { +>self.importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --)) +>self : Symbol(self, Decl(lib.dom.d.ts, --, --), Decl(index.js, 0, 0)) +>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --)) +>importScripts : Symbol(importScripts, Decl(index.js, 0, 32)) + + return function () { + return importScripts.apply(this, arguments); +>importScripts.apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --)) +>importScripts : Symbol(importScripts, Decl(index.js, 0, 32)) +>apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --)) +>arguments : Symbol(arguments) + + }; +})(importScripts); +>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --)) + diff --git a/tests/baselines/reference/noParameterReassignmentJSIIFE.types b/tests/baselines/reference/noParameterReassignmentJSIIFE.types new file mode 100644 index 00000000000..bb447e31f33 --- /dev/null +++ b/tests/baselines/reference/noParameterReassignmentJSIIFE.types @@ -0,0 +1,26 @@ +=== tests/cases/compiler/index.js === +self.importScripts = (function (importScripts) { +>self.importScripts = (function (importScripts) { return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: string[]) => any +>self.importScripts : (...urls: string[]) => void +>self : Window & typeof globalThis +>importScripts : (...urls: string[]) => void +>(function (importScripts) { return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: string[]) => any +>(function (importScripts) { return function () { return importScripts.apply(this, arguments); };}) : (importScripts: (...urls: string[]) => void) => (...args: string[]) => any +>function (importScripts) { return function () { return importScripts.apply(this, arguments); };} : (importScripts: (...urls: string[]) => void) => (...args: string[]) => any +>importScripts : (...urls: string[]) => void + + return function () { +>function () { return importScripts.apply(this, arguments); } : (...args: string[]) => any + + return importScripts.apply(this, arguments); +>importScripts.apply(this, arguments) : any +>importScripts.apply : (this: Function, thisArg: any, argArray?: any) => any +>importScripts : (...urls: string[]) => void +>apply : (this: Function, thisArg: any, argArray?: any) => any +>this : any +>arguments : IArguments + + }; +})(importScripts); +>importScripts : (...urls: string[]) => void + diff --git a/tests/cases/compiler/noParameterReassignmentIIFEAnnotated.ts b/tests/cases/compiler/noParameterReassignmentIIFEAnnotated.ts new file mode 100644 index 00000000000..d634092a15d --- /dev/null +++ b/tests/cases/compiler/noParameterReassignmentIIFEAnnotated.ts @@ -0,0 +1,12 @@ +// @allowJs: true +// @checkJs: true +// @noEmit: true +// @filename: index.js +self.importScripts = (function (importScripts) { + /** + * @param {...unknown} rest + */ + return function () { + return importScripts.apply(this, arguments); + }; +})(importScripts); diff --git a/tests/cases/compiler/noParameterReassignmentJSIIFE.ts b/tests/cases/compiler/noParameterReassignmentJSIIFE.ts new file mode 100644 index 00000000000..175e87887ec --- /dev/null +++ b/tests/cases/compiler/noParameterReassignmentJSIIFE.ts @@ -0,0 +1,9 @@ +// @allowJs: true +// @checkJs: true +// @noEmit: true +// @filename: index.js +self.importScripts = (function (importScripts) { + return function () { + return importScripts.apply(this, arguments); + }; +})(importScripts);