Use fake scopes for type parameters too, and other needed changes

jakebailey · jakebailey · commit 83275d2c18cf · 2023-09-21T15:00:51.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -6598,8 +6598,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 }
                 if (
                     context.flags & NodeBuilderFlags.GenerateNamesForShadowedTypeParams &&
-                    type.flags & TypeFlags.TypeParameter &&
-                    !isTypeSymbolAccessible(type.symbol, context.enclosingDeclaration)
+                    type.flags & TypeFlags.TypeParameter
                 ) {
                     const name = typeParameterToName(type, context);
                     context.approximateLength += idText(name).length;
@@ -7464,7 +7463,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 && signature.declaration
                 && signature.declaration !== context.enclosingDeclaration
                 && !isInJSFile(signature.declaration)
-                && some(expandedParams)
+                && (some(expandedParams) || some(signature.typeParameters))
             ) {
                 // As a performance optimization, reuse the same fake scope within this chain.
                 // This is especially needed when we are working on an excessively deep type;
@@ -7482,32 +7481,64 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 // Note that we only check the most immediate enclosingDeclaration; the only place we
                 // could potentially add another fake scope into the chain is right here, so we don't
                 // traverse all ancestors.
-                const existingFakeScope = getNodeLinks(context.enclosingDeclaration).fakeScopeForSignatureDeclaration ? context.enclosingDeclaration : undefined;
-                Debug.assertOptionalNode(existingFakeScope, isBlock);
+                pushFakeScope(
+                    "params",
+                    add => {
+                        for (const param of expandedParams) {
+                            add(param.escapedName, param);
+                        }
+                    },
+                );
 
-                const locals = existingFakeScope?.locals ?? createSymbolTable();
+                if (context.flags & NodeBuilderFlags.GenerateNamesForShadowedTypeParams) {
+                    // TODO(jakebailey): should this instead be done before walking type parameters?
+                    pushFakeScope(
+                        "typeParams",
+                        add => {
+                            for (const typeParam of signature.typeParameters ?? emptyArray) {
+                                const typeParamName = typeParameterToName(typeParam, context).escapedText;
+                                add(typeParamName, typeParam.symbol);
+                            }
+                        },
+                    );
+                }
 
-                let newLocals: __String[] | undefined;
-                for (const param of expandedParams) {
-                    if (!locals.has(param.escapedName)) {
-                        newLocals = append(newLocals, param.escapedName);
-                        locals.set(param.escapedName, param);
+                function pushFakeScope(kind: "params" | "typeParams", addAll: (addSymbol: (name: __String, symbol: Symbol) => void) => void) {
+                    // We only ever need to look two declarations upward.
+                    Debug.assert(context.enclosingDeclaration);
+                    let existingFakeScope: Node | undefined;
+                    if (getNodeLinks(context.enclosingDeclaration).fakeScopeForSignatureDeclaration === kind) {
+                        existingFakeScope = context.enclosingDeclaration;
                     }
-                }
+                    else if (context.enclosingDeclaration.parent && getNodeLinks(context.enclosingDeclaration.parent).fakeScopeForSignatureDeclaration === kind) {
+                        existingFakeScope = context.enclosingDeclaration.parent;
+                    }
+                    Debug.assertOptionalNode(existingFakeScope, isBlock);
 
-                if (newLocals) {
-                    function removeNewLocals() {
+                    const locals = existingFakeScope?.locals ?? createSymbolTable();
+                    let newLocals: __String[] | undefined;
+                    addAll((name, symbol) => {
+                        if (!locals.has(name)) {
+                            newLocals = append(newLocals, name);
+                            locals.set(name, symbol);
+                        }
+                    });
+                    if (!newLocals) return;
+
+                    const oldCleanup = cleanup;
+                    function undo() {
                         forEach(newLocals, s => locals.delete(s));
+                        oldCleanup?.();
                     }
 
                     if (existingFakeScope) {
-                        cleanup = removeNewLocals;
+                        cleanup = undo;
                     }
                     else {
                         // Use a Block for this; the type of the node doesn't matter so long as it
                         // has locals, and this is cheaper/easier than using a function-ish Node.
                         const fakeScope = parseNodeFactory.createBlock(emptyArray);
-                        getNodeLinks(fakeScope).fakeScopeForSignatureDeclaration = true;
+                        getNodeLinks(fakeScope).fakeScopeForSignatureDeclaration = kind;
                         fakeScope.locals = locals;
 
                         const saveEnclosingDeclaration = context.enclosingDeclaration;
@@ -7516,7 +7547,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
 
                         cleanup = () => {
                             context.enclosingDeclaration = saveEnclosingDeclaration;
-                            removeNewLocals();
+                            undo();
                         };
                     }
                 }
@@ -8062,13 +8093,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
             }
         }
 
-        function typeParameterShadowsNameInScope(escapedName: __String, context: NodeBuilderContext, type: TypeParameter) {
+        function typeParameterShadowsOtherTypeParameterInScope(escapedName: __String, context: NodeBuilderContext, type: TypeParameter) {
             const result = resolveName(context.enclosingDeclaration, escapedName, SymbolFlags.Type, /*nameNotFoundMessage*/ undefined, escapedName, /*isUse*/ false);
-            if (result) {
-                if (result.flags & SymbolFlags.TypeParameter && result === type.symbol) {
-                    return false;
-                }
-                return true;
+            if (result && result.flags & SymbolFlags.TypeParameter) {
+                return result !== type.symbol;
             }
             return false;
         }
@@ -8088,7 +8116,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 const rawtext = result.escapedText as string;
                 let i = context.typeParameterNamesByTextNextNameCount?.get(rawtext) || 0;
                 let text = rawtext;
-                while (context.typeParameterNamesByText?.has(text) || typeParameterShadowsNameInScope(text as __String, context, type)) {
+                while (context.typeParameterNamesByText?.has(text) || typeParameterShadowsOtherTypeParameterInScope(text as __String, context, type)) {
                     i++;
                     text = `${rawtext}_${i}`;
                 }
@@ -8101,7 +8129,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 // `i` we've used thus far, to save work later
                 (context.typeParameterNamesByTextNextNameCount ||= new Map()).set(rawtext, i);
                 (context.typeParameterNames ||= new Map()).set(getTypeId(type), result);
-                (context.typeParameterNamesByText ||= new Set()).add(rawtext);
+                (context.typeParameterNamesByText ||= new Set()).add(text);
             }
             return result;
         }
@@ -8280,7 +8308,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         }
 
         function getEnclosingDeclarationIgnoringFakeScope(enclosingDeclaration: Node) {
-            return getNodeLinks(enclosingDeclaration).fakeScopeForSignatureDeclaration ? enclosingDeclaration.parent : enclosingDeclaration;
+            while (getNodeLinks(enclosingDeclaration).fakeScopeForSignatureDeclaration) {
+                enclosingDeclaration = enclosingDeclaration.parent;
+            }
+            return enclosingDeclaration;
         }
 
         /**
@@ -8360,7 +8391,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 }
                 if (isIdentifier(node)) {
                     const type = getDeclaredTypeOfSymbol(sym);
-                    const name = sym.flags & SymbolFlags.TypeParameter && !isTypeSymbolAccessible(type.symbol, context.enclosingDeclaration) ? typeParameterToName(type, context) : factory.cloneNode(node);
+                    const name = sym.flags & SymbolFlags.TypeParameter ? typeParameterToName(type, context) : factory.cloneNode(node);
                     name.symbol = sym; // for quickinfo, which uses identifier symbol information
                     return { introducesError, node: setEmitFlags(setOriginalNode(name, node), EmitFlags.NoAsciiEscaping) };
                 }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -5987,7 +5987,7 @@ export interface NodeLinks {
     decoratorSignature?: Signature;     // Signature for decorator as if invoked by the runtime.
     spreadIndices?: { first: number | undefined, last: number | undefined }; // Indices of first and last spread elements in array literal
     parameterInitializerContainsUndefined?: boolean; // True if this is a parameter declaration whose type annotation contains "undefined".
-    fakeScopeForSignatureDeclaration?: boolean; // True if this is a fake scope injected into an enclosing declaration chain.
+    fakeScopeForSignatureDeclaration?: "params" | "typeParams"; // If present, this is a fake scope injected into an enclosing declaration chain.
     assertionExpressionType?: Type;     // Cached type of the expression of a type assertion
 }
 
diff --git a/tests/baselines/reference/conditionalTypes1.js b/tests/baselines/reference/conditionalTypes1.js
@@ -649,7 +649,7 @@ type Foo<T> = T extends string ? boolean : number;
 type Bar<T> = T extends string ? boolean : number;
 declare const convert: <U>(value: Foo<U>) => Bar<U>;
 type Baz<T> = Foo<T>;
-declare const convert2: <T>(value: Foo<T>) => Foo<T>;
+declare const convert2: <T>(value: Foo<T>) => Baz<T>;
 declare function f31<T>(): void;
 declare function f32<T, U>(): void;
 declare function f33<T, U>(): void;
diff --git a/tests/baselines/reference/correlatedUnions.js b/tests/baselines/reference/correlatedUnions.js
@@ -602,7 +602,7 @@ type SameKeys<T> = {
     };
 };
 type MappedFromOriginal = SameKeys<Original>;
-declare const getStringAndNumberFromOriginalAndMapped: <K extends keyof Original, N extends keyof Original[K]>(original: Original, mappedFromOriginal: MappedFromOriginal, key: K, nestedKey: N) => [Original[K][N], SameKeys<Original>[K][N]];
+declare const getStringAndNumberFromOriginalAndMapped: <K extends keyof Original, N extends keyof Original[K]>(original: Original, mappedFromOriginal: MappedFromOriginal, key: K, nestedKey: N) => [Original[K][N], MappedFromOriginal[K][N]];
 interface Config {
     string: string;
     number: number;
diff --git a/tests/baselines/reference/declarationEmitDistributiveConditionalWithInfer.js b/tests/baselines/reference/declarationEmitDistributiveConditionalWithInfer.js
@@ -17,4 +17,4 @@ exports.fun = fun;
 
 
 //// [declarationEmitDistributiveConditionalWithInfer.d.ts]
-export declare const fun: (subFun: <Collection, Field extends keyof Collection>() => (Collection[Field] extends infer T ? T extends Collection[Field] ? T extends readonly (infer InnerArr)[] ? InnerArr : T : never : never)[]) => void;
+export declare const fun: (subFun: <Collection, Field extends keyof Collection>() => FlatArray<Collection[Field], 0>[]) => void;
diff --git a/tests/baselines/reference/declarationEmitPrivatePromiseLikeInterface.js b/tests/baselines/reference/declarationEmitPrivatePromiseLikeInterface.js
@@ -72,6 +72,6 @@ export interface HttpResponse<D extends unknown, E extends unknown = unknown> ex
     error: E;
 }
 export declare class HttpClient<SecurityDataType = unknown> {
-    request: <T = any, E = any>() => TPromise<HttpResponse<T, E>, any>;
+    request: <T = any, E = any>() => TPromise<HttpResponse<T, E>>;
 }
 export {};
diff --git a/tests/baselines/reference/declarationEmitTypeParameterNameInOuterScope.js b/tests/baselines/reference/declarationEmitTypeParameterNameInOuterScope.js
@@ -33,11 +33,11 @@ function b2(x) { return x; }
 //// [declarationEmitTypeParameterNameInOuterScope.d.ts]
 declare class A {
 }
-declare var a: <A_1>(x: A_1) => A_1;
+declare var a: <A>(x: A) => A;
 declare function a2<A>(x: A): A;
-declare var a3: <A_1>(x: A_1) => A;
+declare var a3: <A>(x: A) => globalThis.A;
 declare function a4<A>(x: A): globalThis.A;
 interface B {
 }
-declare var b: <B_1>(x: B_1) => B_1;
+declare var b: <B>(x: B) => B;
 declare function b2<B>(x: B): B;
diff --git a/tests/baselines/reference/jsDeclarationsClasses.js b/tests/baselines/reference/jsDeclarationsClasses.js
@@ -541,7 +541,7 @@ export class F<T, U> {
      * @param {A} a
      * @param {B} b
      */
-    static create<A_1, B_1>(a: A_1, b: B_1): F<A_1, B_1>;
+    static create<A, B>(a: A, b: B): F<A, B>;
     /**
      * @param {T} a
      * @param {U} b
diff --git a/tests/baselines/reference/jsDeclarationsInterfaces.js b/tests/baselines/reference/jsDeclarationsInterfaces.js
@@ -134,19 +134,19 @@ export interface A {
 export interface B {
     cat: string;
 }
-export interface C<T_1, U_1> {
+export interface C<T, U> {
     new (): string;
-    new (x: T_1): U_1;
-    new <Q_4>(x: Q_4): T_1 & Q_4;
+    new (x: T): U;
+    new <Q_3>(x: Q_3): T & Q_3;
     (): number;
-    (x: T_1): U_1;
-    <Q_3>(x: Q_3): T_1 & Q_3;
-    field: T_1 & U_1;
-    optionalField?: T_1;
-    readonly readonlyField: T_1 & U_1;
-    readonly readonlyOptionalField?: U_1;
-    method<Q_1 = number>(): number;
-    method<Q_2>(a: T_1 & Q_2): Q_2 & number;
+    (x: T): U;
+    <Q_2>(x: Q_2): T & Q_2;
+    field: T & U;
+    optionalField?: T;
+    readonly readonlyField: T & U;
+    readonly readonlyOptionalField?: U;
+    method<Q = number>(): number;
+    method<Q_1>(a: T & Q_1): Q_1 & number;
     method(a?: number): number;
     method(...args: any[]): number;
     optMethod?(): number;
@@ -161,11 +161,11 @@ export interface K extends I, J {
 export interface L extends K {
     y: string;
 }
-export interface M<T_1> {
-    field: T_1;
+export interface M<T> {
+    field: T;
 }
-export interface N<U_1> extends M<U_1> {
-    other: U_1;
+export interface N<U> extends M<U> {
+    other: U;
 }
 export interface O {
     [idx: string]: string;

Original file line number	Diff line number	Diff line change
`@@ -5987,7 +5987,7 @@ export interface NodeLinks {`
`5987`	`5987`	`decoratorSignature?: Signature; // Signature for decorator as if invoked by the runtime.`
`5988`	`5988`	`spreadIndices?: { first: number \| undefined, last: number \| undefined }; // Indices of first and last spread elements in array literal`
`5989`	`5989`	`parameterInitializerContainsUndefined?: boolean; // True if this is a parameter declaration whose type annotation contains "undefined".`
`5990`		`- fakeScopeForSignatureDeclaration?: boolean; // True if this is a fake scope injected into an enclosing declaration chain.`
	`5990`	`+ fakeScopeForSignatureDeclaration?: "params" \| "typeParams"; // If present, this is a fake scope injected into an enclosing declaration chain.`
`5991`	`5991`	`assertionExpressionType?: Type; // Cached type of the expression of a type assertion`
`5992`	`5992`	`}`
`5993`	`5993`
Original file line number	Diff line number	Diff line change
`@@ -17,4 +17,4 @@ exports.fun = fun;`
`17`	`17`
`18`	`18`
`19`	`19`	`//// [declarationEmitDistributiveConditionalWithInfer.d.ts]`
`20`		`-export declare const fun: (subFun: <Collection, Field extends keyof Collection>() => (Collection[Field] extends infer T ? T extends Collection[Field] ? T extends readonly (infer InnerArr)[] ? InnerArr : T : never : never)[]) => void;`
	`20`	`+export declare const fun: (subFun: <Collection, Field extends keyof Collection>() => FlatArray<Collection[Field], 0>[]) => void;`
Original file line number	Diff line number	Diff line change
`@@ -72,6 +72,6 @@ export interface HttpResponse<D extends unknown, E extends unknown = unknown> ex`
`72`	`72`	`error: E;`
`73`	`73`	`}`
`74`	`74`	`export declare class HttpClient<SecurityDataType = unknown> {`
`75`		`- request: <T = any, E = any>() => TPromise<HttpResponse<T, E>, any>;`
	`75`	`+ request: <T = any, E = any>() => TPromise<HttpResponse<T, E>>;`
`76`	`76`	`}`
`77`	`77`	`export {};`