Skip to content

Commit 7f65250

Browse files
weswighamweswigham
authored
Handle JS synthetic rest args in contextual parameter assignment (microsoft#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
1 parent 92bc2dd commit 7f65250

8 files changed

+152
-5
lines changed

‎src/compiler/checker.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12862,7 +12862,19 @@ namespace ts {
1286212862
p.typeExpression && isJSDocVariadicType(p.typeExpression.type) ? p.typeExpression.type : undefined);
1286312863

1286412864
const syntheticArgsSymbol = createSymbol(SymbolFlags.Variable, "args" as __String, CheckFlags.RestParameter);
12865-
syntheticArgsSymbol.type = lastParamVariadicType ? createArrayType(getTypeFromTypeNode(lastParamVariadicType.type)) : anyArrayType;
12865+
if (lastParamVariadicType) {
12866+
// Parameter has effective annotation, lock in type
12867+
syntheticArgsSymbol.type = createArrayType(getTypeFromTypeNode(lastParamVariadicType.type));
12868+
}
12869+
else {
12870+
// Parameter has no annotation
12871+
// 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
12872+
// cached by `getTypeOfSymbol` yet.
12873+
syntheticArgsSymbol.checkFlags |= CheckFlags.DeferredType;
12874+
syntheticArgsSymbol.deferralParent = neverType;
12875+
syntheticArgsSymbol.deferralConstituents = [anyArrayType];
12876+
syntheticArgsSymbol.deferralWriteConstituents = [anyArrayType];
12877+
}
1286612878
if (lastParamVariadicType) {
1286712879
// Replace the last parameter with a rest parameter.
1286812880
parameters.pop();
@@ -32163,7 +32175,12 @@ namespace ts {
3216332175
if (signatureHasRestParameter(signature)) {
3216432176
// parameter might be a transient symbol generated by use of `arguments` in the function body.
3216532177
const parameter = last(signature.parameters);
32166-
if (isTransientSymbol(parameter) || !getEffectiveTypeAnnotationNode(parameter.valueDeclaration as ParameterDeclaration)) {
32178+
if (parameter.valueDeclaration
32179+
? !getEffectiveTypeAnnotationNode(parameter.valueDeclaration as ParameterDeclaration)
32180+
// a declarationless parameter may still have a `.type` already set by its construction logic
32181+
// (which may pull a type from a jsdoc) - only allow fixing on `DeferredType` parameters with a fallback type
32182+
: !!(getCheckFlags(parameter) & CheckFlags.DeferredType)
32183+
) {
3216732184
const contextualParameterType = getRestTypeAtPosition(context, len);
3216832185
assignParameterType(parameter, contextualParameterType);
3216932186
}
@@ -32182,9 +32199,9 @@ namespace ts {
3218232199
function assignParameterType(parameter: Symbol, type?: Type) {
3218332200
const links = getSymbolLinks(parameter);
3218432201
if (!links.type) {
32185-
const declaration = parameter.valueDeclaration as ParameterDeclaration;
32186-
links.type = type || getWidenedTypeForVariableLikeDeclaration(declaration, /*reportErrors*/ true);
32187-
if (declaration.name.kind !== SyntaxKind.Identifier) {
32202+
const declaration = parameter.valueDeclaration as ParameterDeclaration | undefined;
32203+
links.type = type || (declaration ? getWidenedTypeForVariableLikeDeclaration(declaration, /*reportErrors*/ true) : getTypeOfSymbol(parameter));
32204+
if (declaration && declaration.name.kind !== SyntaxKind.Identifier) {
3218832205
// if inference didn't come up with anything but unknown, fall back to the binding pattern if present.
3218932206
if (links.type === unknownType) {
3219032207
links.type = getTypeFromBindingPattern(declaration.name);

‎tests/baselines/reference/noParameterReassignmentIIFEAnnotated.errors.txt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
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.
2+
3+
4+
==== tests/cases/compiler/index.js (1 errors) ====
5+
self.importScripts = (function (importScripts) {
6+
/**
7+
* @param {...unknown} rest
8+
~~~~
9+
!!! 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.
10+
*/
11+
return function () {
12+
return importScripts.apply(this, arguments);
13+
};
14+
})(importScripts);
15+

‎tests/baselines/reference/noParameterReassignmentIIFEAnnotated.symbols

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
=== tests/cases/compiler/index.js ===
2+
self.importScripts = (function (importScripts) {
3+
>self.importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --))
4+
>self : Symbol(self, Decl(lib.dom.d.ts, --, --), Decl(index.js, 0, 0))
5+
>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --))
6+
>importScripts : Symbol(importScripts, Decl(index.js, 0, 32))
7+
8+
/**
9+
* @param {...unknown} rest
10+
*/
11+
return function () {
12+
return importScripts.apply(this, arguments);
13+
>importScripts.apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --))
14+
>importScripts : Symbol(importScripts, Decl(index.js, 0, 32))
15+
>apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --))
16+
>arguments : Symbol(arguments)
17+
18+
};
19+
})(importScripts);
20+
>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --))
21+

‎tests/baselines/reference/noParameterReassignmentIIFEAnnotated.types

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/compiler/index.js ===
2+
self.importScripts = (function (importScripts) {
3+
>self.importScripts = (function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: unknown[]) => any
4+
>self.importScripts : (...urls: string[]) => void
5+
>self : Window & typeof globalThis
6+
>importScripts : (...urls: string[]) => void
7+
>(function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: unknown[]) => any
8+
>(function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };}) : (importScripts: (...urls: string[]) => void) => (...args: unknown[]) => any
9+
>function (importScripts) { /** * @param {...unknown} rest */ return function () { return importScripts.apply(this, arguments); };} : (importScripts: (...urls: string[]) => void) => (...args: unknown[]) => any
10+
>importScripts : (...urls: string[]) => void
11+
12+
/**
13+
* @param {...unknown} rest
14+
*/
15+
return function () {
16+
>function () { return importScripts.apply(this, arguments); } : (...args: unknown[]) => any
17+
18+
return importScripts.apply(this, arguments);
19+
>importScripts.apply(this, arguments) : any
20+
>importScripts.apply : (this: Function, thisArg: any, argArray?: any) => any
21+
>importScripts : (...urls: string[]) => void
22+
>apply : (this: Function, thisArg: any, argArray?: any) => any
23+
>this : any
24+
>arguments : IArguments
25+
26+
};
27+
})(importScripts);
28+
>importScripts : (...urls: string[]) => void
29+

‎tests/baselines/reference/noParameterReassignmentJSIIFE.symbols

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
=== tests/cases/compiler/index.js ===
2+
self.importScripts = (function (importScripts) {
3+
>self.importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --))
4+
>self : Symbol(self, Decl(lib.dom.d.ts, --, --), Decl(index.js, 0, 0))
5+
>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --))
6+
>importScripts : Symbol(importScripts, Decl(index.js, 0, 32))
7+
8+
return function () {
9+
return importScripts.apply(this, arguments);
10+
>importScripts.apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --))
11+
>importScripts : Symbol(importScripts, Decl(index.js, 0, 32))
12+
>apply : Symbol(Function.apply, Decl(lib.es5.d.ts, --, --))
13+
>arguments : Symbol(arguments)
14+
15+
};
16+
})(importScripts);
17+
>importScripts : Symbol(importScripts, Decl(lib.webworker.importscripts.d.ts, --, --))
18+

‎tests/baselines/reference/noParameterReassignmentJSIIFE.types

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
=== tests/cases/compiler/index.js ===
2+
self.importScripts = (function (importScripts) {
3+
>self.importScripts = (function (importScripts) { return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: string[]) => any
4+
>self.importScripts : (...urls: string[]) => void
5+
>self : Window & typeof globalThis
6+
>importScripts : (...urls: string[]) => void
7+
>(function (importScripts) { return function () { return importScripts.apply(this, arguments); };})(importScripts) : (...args: string[]) => any
8+
>(function (importScripts) { return function () { return importScripts.apply(this, arguments); };}) : (importScripts: (...urls: string[]) => void) => (...args: string[]) => any
9+
>function (importScripts) { return function () { return importScripts.apply(this, arguments); };} : (importScripts: (...urls: string[]) => void) => (...args: string[]) => any
10+
>importScripts : (...urls: string[]) => void
11+
12+
return function () {
13+
>function () { return importScripts.apply(this, arguments); } : (...args: string[]) => any
14+
15+
return importScripts.apply(this, arguments);
16+
>importScripts.apply(this, arguments) : any
17+
>importScripts.apply : (this: Function, thisArg: any, argArray?: any) => any
18+
>importScripts : (...urls: string[]) => void
19+
>apply : (this: Function, thisArg: any, argArray?: any) => any
20+
>this : any
21+
>arguments : IArguments
22+
23+
};
24+
})(importScripts);
25+
>importScripts : (...urls: string[]) => void
26+

‎tests/cases/compiler/noParameterReassignmentIIFEAnnotated.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// @allowJs: true
2+
// @checkJs: true
3+
// @noEmit: true
4+
// @filename: index.js
5+
self.importScripts = (function (importScripts) {
6+
/**
7+
* @param {...unknown} rest
8+
*/
9+
return function () {
10+
return importScripts.apply(this, arguments);
11+
};
12+
})(importScripts);

‎tests/cases/compiler/noParameterReassignmentJSIIFE.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// @allowJs: true
2+
// @checkJs: true
3+
// @noEmit: true
4+
// @filename: index.js
5+
self.importScripts = (function (importScripts) {
6+
return function () {
7+
return importScripts.apply(this, arguments);
8+
};
9+
})(importScripts);

0 commit comments

Comments
 (0)