Skip to content

Use proper type parameter hosts in JS files #61013

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.

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
13 changes: 8 additions & 5 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ import {
getDirectoryPath,
getEffectiveBaseTypeNode,
getEffectiveConstraintOfTypeParameter,
getEffectiveContainerForJSDocTemplateTag,
getEffectiveImplementsTypeNodes,
getEffectiveInitializer,
getEffectiveJSDocHost,
Expand Down Expand Up @@ -16217,9 +16216,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return typeParameter.constraint === noConstraintType ? undefined : typeParameter.constraint;
}

function getParentSymbolOfTypeParameter(typeParameter: TypeParameter): Symbol | undefined {
function getEffectiveTypeParameterHost(typeParameter: TypeParameter) {
const tp = getDeclarationOfKind<TypeParameterDeclaration>(typeParameter.symbol, SyntaxKind.TypeParameter)!;
const host = isJSDocTemplateTag(tp.parent) ? getEffectiveContainerForJSDocTemplateTag(tp.parent) : tp.parent;
return isJSDocTemplateTag(tp.parent) ? getEffectiveJSDocHost(tp.parent) : tp.parent;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of makes me wonder when we really should be using getEffectiveJSDocHost where we are currently using getEffectiveContainerForJSDocTemplateTag...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but to be fair - I had to use getEffectiveJSDocHost to fix this issue at hand and it didn't look to me like this older function would have to stick to the other one. There is a chance that it should not use this - but there is also a chance that this change fixed some other issue ;p

In the ideal world, I'd go through all of the callers to both and assess their needs but I just don't have time to do that right now 😢

}

function getParentSymbolOfTypeParameter(typeParameter: TypeParameter): Symbol | undefined {
const host = getEffectiveTypeParameterHost(typeParameter);
return host && getSymbolOfNode(host);
}

Expand Down Expand Up @@ -36138,8 +36141,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (candidate.typeParameters) {
// If we are *inside the body of candidate*, we need to create a clone of `candidate` with differing type parameter identities,
// so our inference results for this call doesn't pollute expression types referencing the outer type parameter!
const paramLocation = candidate.typeParameters[0].symbol.declarations?.[0]?.parent;
const candidateParameterContext = paramLocation || (candidate.declaration && isConstructorDeclaration(candidate.declaration) ? candidate.declaration.parent : candidate.declaration);
const paramHost = getEffectiveTypeParameterHost(candidate.typeParameters[0]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually use the term "host" like this elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess "host" might only be appropriate in the JSDoc context but I don't have a better name for this that would cover both TS and JSDoc-based types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean "context" is what the PR title and the code below uses; could technically just inline it too. But, the existing JSDoc func used the term, so it's probably okay.

const candidateParameterContext = paramHost || (candidate.declaration && isConstructorDeclaration(candidate.declaration) ? candidate.declaration.parent : candidate.declaration);
if (candidateParameterContext && findAncestor(node, a => a === candidateParameterContext)) {
candidate = getImplementationSignature(candidate);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//// [tests/cases/compiler/inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts] ////

=== inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts ===
class S<T> {
>S : Symbol(S, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 0))
>T : Symbol(T, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 8))

set: Set<T>;
>set : Symbol(S.set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 12))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>T : Symbol(T, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 8))

constructor(set: Set<T>) {
>set : Symbol(set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 3, 14))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>T : Symbol(T, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 8))

this.set = set;
>this.set : Symbol(S.set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 12))
>this : Symbol(S, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 0))
>set : Symbol(S.set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 12))
>set : Symbol(set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 3, 14))
}

array() {
>array : Symbol(S.array, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 5, 3))

return new S(new Set([...this.set].map((item) => [item])));
>S : Symbol(S, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 0))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>[...this.set].map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>this.set : Symbol(S.set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 12))
>this : Symbol(S, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 0))
>set : Symbol(S.set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 12))
>map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>item : Symbol(item, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 8, 44))
>item : Symbol(item, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 8, 44))
}
}

function sArray<T>(set: Set<T>) {
>sArray : Symbol(sArray, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 10, 1))
>T : Symbol(T, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 12, 16))
>set : Symbol(set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 12, 19))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>T : Symbol(T, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 12, 16))

return new S(new Set([...set].map((item) => [item])));
>S : Symbol(S, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 0, 0))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>[...set].map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>set : Symbol(set, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 12, 19))
>map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>item : Symbol(item, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 13, 37))
>item : Symbol(item, Decl(inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts, 13, 37))
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
//// [tests/cases/compiler/inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts] ////

=== Performance Stats ===
Type Count: 1,000
Instantiation count: 2,500

=== inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResult2.ts ===
class S<T> {
>S : S<T>
> : ^^^^

set: Set<T>;
>set : Set<T>
> : ^^^^^^

constructor(set: Set<T>) {
>set : Set<T>
> : ^^^^^^

this.set = set;
>this.set = set : Set<T>
> : ^^^^^^
>this.set : Set<T>
> : ^^^^^^
>this : this
> : ^^^^
>set : Set<T>
> : ^^^^^^
>set : Set<T>
> : ^^^^^^
}

array() {
>array : () => S<T[]>
> : ^^^^^^^^^^^^

return new S(new Set([...this.set].map((item) => [item])));
>new S(new Set([...this.set].map((item) => [item]))) : S<T[]>
> : ^^^^^^
>S : typeof S
> : ^^^^^^^^
>new Set([...this.set].map((item) => [item])) : Set<T[]>
> : ^^^^^^^^
>Set : SetConstructor
> : ^^^^^^^^^^^^^^
>[...this.set].map((item) => [item]) : T[][]
> : ^^^^^
>[...this.set].map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>[...this.set] : T[]
> : ^^^
>...this.set : T
> : ^
>this.set : Set<T>
> : ^^^^^^
>this : this
> : ^^^^
>set : Set<T>
> : ^^^^^^
>map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>(item) => [item] : (item: T) => T[]
> : ^ ^^^^^^^^^^^
>item : T
> : ^
>[item] : T[]
> : ^^^
>item : T
> : ^
}
}

function sArray<T>(set: Set<T>) {
>sArray : <T>(set: Set<T>) => S<T[]>
> : ^ ^^ ^^ ^^^^^^^^^^^
>set : Set<T>
> : ^^^^^^

return new S(new Set([...set].map((item) => [item])));
>new S(new Set([...set].map((item) => [item]))) : S<T[]>
> : ^^^^^^
>S : typeof S
> : ^^^^^^^^
>new Set([...set].map((item) => [item])) : Set<T[]>
> : ^^^^^^^^
>Set : SetConstructor
> : ^^^^^^^^^^^^^^
>[...set].map((item) => [item]) : T[][]
> : ^^^^^
>[...set].map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>[...set] : T[]
> : ^^^
>...set : T
> : ^
>set : Set<T>
> : ^^^^^^
>map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>(item) => [item] : (item: T) => T[]
> : ^ ^^^^^^^^^^^
>item : T
> : ^
>[item] : T[]
> : ^^^
>item : T
> : ^
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
//// [tests/cases/compiler/inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResultJsDoc1.ts] ////

=== index.js ===
// https://github.com/microsoft/TypeScript/issues/60988

/**
* @template [T = any]
*/
class S {
>S : Symbol(S, Decl(index.js, 0, 0))

/**
* @type {Set<T>}
*/
set;
>set : Symbol(S.set, Decl(index.js, 5, 9))

/**
* @param {Set<T>} set
*/
constructor(set) {
>set : Symbol(set, Decl(index.js, 15, 13))

this.set = set;
>this.set : Symbol(S.set, Decl(index.js, 5, 9))
>this : Symbol(S, Decl(index.js, 0, 0))
>set : Symbol(S.set, Decl(index.js, 5, 9))
>set : Symbol(set, Decl(index.js, 15, 13))
}

array() {
>array : Symbol(S.array, Decl(index.js, 17, 2))

return new S(new Set([...this.set].map(item => [item])));
>S : Symbol(S, Decl(index.js, 0, 0))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>[...this.set].map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>this.set : Symbol(S.set, Decl(index.js, 5, 9))
>this : Symbol(S, Decl(index.js, 0, 0))
>set : Symbol(S.set, Decl(index.js, 5, 9))
>map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>item : Symbol(item, Decl(index.js, 20, 41))
>item : Symbol(item, Decl(index.js, 20, 41))
}
}

/**
* @template [T = any]
* @param {Set<T>} set
*/
const sArray = (set) => {
>sArray : Symbol(sArray, Decl(index.js, 28, 5))
>set : Symbol(set, Decl(index.js, 28, 16))

return new S(new Set([...set].map(item => [item])));
>S : Symbol(S, Decl(index.js, 0, 0))
>Set : Symbol(Set, Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.collection.d.ts, --, --), Decl(lib.es2015.iterable.d.ts, --, --), Decl(lib.es2015.symbol.wellknown.d.ts, --, --), Decl(lib.esnext.collection.d.ts, --, --))
>[...set].map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>set : Symbol(set, Decl(index.js, 28, 16))
>map : Symbol(Array.map, Decl(lib.es5.d.ts, --, --))
>item : Symbol(item, Decl(index.js, 29, 35))
>item : Symbol(item, Decl(index.js, 29, 35))

};

Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
//// [tests/cases/compiler/inferenceOuterResultNotIncorrectlyInstantiatedWithInnerResultJsDoc1.ts] ////

=== Performance Stats ===
Type Count: 1,000
Instantiation count: 2,500

=== index.js ===
// https://github.com/microsoft/TypeScript/issues/60988

/**
* @template [T = any]
*/
class S {
>S : S<T>
> : ^^^^

/**
* @type {Set<T>}
*/
set;
>set : Set<T>
> : ^^^^^^

/**
* @param {Set<T>} set
*/
constructor(set) {
>set : Set<T>
> : ^^^^^^

this.set = set;
>this.set = set : Set<T>
> : ^^^^^^
>this.set : Set<T>
> : ^^^^^^
>this : this
> : ^^^^
>set : Set<T>
> : ^^^^^^
>set : Set<T>
> : ^^^^^^
}

array() {
>array : () => S<T[]>
> : ^^^^^^^^^^^^

return new S(new Set([...this.set].map(item => [item])));
>new S(new Set([...this.set].map(item => [item]))) : S<T[]>
> : ^^^^^^
>S : typeof S
> : ^^^^^^^^
>new Set([...this.set].map(item => [item])) : Set<T[]>
> : ^^^^^^^^
>Set : SetConstructor
> : ^^^^^^^^^^^^^^
>[...this.set].map(item => [item]) : T[][]
> : ^^^^^
>[...this.set].map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>[...this.set] : T[]
> : ^^^
>...this.set : T
> : ^
>this.set : Set<T>
> : ^^^^^^
>this : this
> : ^^^^
>set : Set<T>
> : ^^^^^^
>map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>item => [item] : (item: T) => T[]
> : ^ ^^^^^^^^^^^
>item : T
> : ^
>[item] : T[]
> : ^^^
>item : T
> : ^
}
}

/**
* @template [T = any]
* @param {Set<T>} set
*/
const sArray = (set) => {
>sArray : <T = any>(set: Set<T>) => S<T[]>
> : ^ ^^^^^^^^ ^^ ^^^^^^^^^^^
>(set) => { return new S(new Set([...set].map(item => [item])));} : <T = any>(set: Set<T>) => S<T[]>
> : ^ ^^^^^^^^ ^^ ^^^^^^^^^^^
>set : Set<T>
> : ^^^^^^

return new S(new Set([...set].map(item => [item])));
>new S(new Set([...set].map(item => [item]))) : S<T[]>
> : ^^^^^^
>S : typeof S
> : ^^^^^^^^
>new Set([...set].map(item => [item])) : Set<T[]>
> : ^^^^^^^^
>Set : SetConstructor
> : ^^^^^^^^^^^^^^
>[...set].map(item => [item]) : T[][]
> : ^^^^^
>[...set].map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>[...set] : T[]
> : ^^^
>...set : T
> : ^
>set : Set<T>
> : ^^^^^^
>map : <U>(callbackfn: (value: T, index: number, array: T[]) => U, thisArg?: any) => U[]
> : ^ ^^ ^^^ ^^^^^ ^^ ^^ ^^^^^^^^^^^^^ ^^^ ^^^^^^^^
>item => [item] : (item: T) => T[]
> : ^ ^^^^^^^^^^^
>item : T
> : ^
>[item] : T[]
> : ^^^
>item : T
> : ^

};

Loading
Loading