Add missing recursive vistor on copied import type nodes (microsoft#58165)

weswigham · web-flow · commit 6092c2d4c404 · 2024-04-12T10:53:57.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -112,7 +112,6 @@ import {
     createEvaluator,
     createFileDiagnostic,
     createFlowNode,
-    createGetCanonicalFileName,
     createGetSymbolWalker,
     createModeAwareCacheKey,
     createModuleNotFoundChain,
@@ -347,7 +346,6 @@ import {
     getPropertyNameFromType,
     getResolutionDiagnostic,
     getResolutionModeOverride,
-    getResolvedExternalModuleName,
     getResolveJsonModule,
     getRestParameterElementType,
     getRootDeclaration,
@@ -7668,14 +7666,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                     return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);
                 }
             }
-            if (!context.enclosingDeclaration || !context.tracker.moduleResolverHost) {
+            if (!context.enclosingFile || !context.tracker.moduleResolverHost) {
                 // If there's no context declaration, we can't lookup a non-ambient specifier, so we just use the symbol name
                 if (ambientModuleSymbolRegex.test(symbol.escapedName as string)) {
                     return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);
                 }
                 return getSourceFileOfNode(getNonAugmentationDeclaration(symbol)!).fileName; // A resolver may not be provided for baselines and errors - in those cases we use the fileName in full
             }
-            const contextFile = getSourceFileOfNode(getOriginalNode(context.enclosingDeclaration));
+            const contextFile = context.enclosingFile;
             const resolutionMode = overrideImportMode || contextFile?.impliedNodeFormat;
             const cacheKey = createModeAwareCacheKey(contextFile.path, resolutionMode);
             const links = getSymbolLinks(symbol);
@@ -8479,22 +8477,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 }
 
                 function rewriteModuleSpecifier(parent: ImportTypeNode, lit: StringLiteral) {
-                    if (context.bundled) {
-                        if (context.tracker && context.tracker.moduleResolverHost) {
-                            const targetFile = getExternalModuleFileFromDeclaration(parent);
-                            if (targetFile) {
-                                const getCanonicalFileName = createGetCanonicalFileName(!!host.useCaseSensitiveFileNames);
-                                const resolverHost = {
-                                    getCanonicalFileName,
-                                    getCurrentDirectory: () => context.tracker.moduleResolverHost!.getCurrentDirectory(),
-                                    getCommonSourceDirectory: () => context.tracker.moduleResolverHost!.getCommonSourceDirectory(),
-                                };
-                                const newName = getResolvedExternalModuleName(resolverHost, targetFile);
-                                return factory.createStringLiteral(newName);
+                    if (context.bundled || context.enclosingFile !== getSourceFileOfNode(lit)) {
+                        const targetFile = getExternalModuleFileFromDeclaration(parent);
+                        if (targetFile) {
+                            const newName = getSpecifierForModuleSymbol(targetFile.symbol, context);
+                            if (newName !== lit.text) {
+                                return setOriginalNode(factory.createStringLiteral(newName), lit);
                             }
                         }
                     }
-                    return lit;
+                    return visitNode(lit, visitExistingNodeTreeSymbols, isStringLiteral)!;
                 }
             }
         }
diff --git a/tests/baselines/reference/callbackCrossModule.types b/tests/baselines/reference/callbackCrossModule.types
@@ -36,7 +36,7 @@ function C() {
 === use.js ===
 /** @param {import('./mod1').Con} k */
 function f(k) {
->f : (k: import('./mod1').Con) => any
+>f : (k: import("./mod1").Con) => any
 >  : ^ ^^                    ^^^^^^^^
 >k : import("mod1").Con
 >  : ^^^^^^^^^^^^^^^^^^
diff --git a/tests/baselines/reference/declarationEmitCrossFileCopiedGeneratedImportType.js b/tests/baselines/reference/declarationEmitCrossFileCopiedGeneratedImportType.js
@@ -0,0 +1,32 @@
+//// [tests/cases/compiler/declarationEmitCrossFileCopiedGeneratedImportType.ts] ////
+
+//// [index.d.ts]
+export declare class Foo extends Number {
+    private _;
+}
+//// [index.d.ts]
+import { Foo } from "../projA";
+export declare const f: (foo: Foo) => boolean;
+//// [index.d.ts]
+export declare const e: {
+    f: (foo: import("../projA").Foo) => boolean;
+};
+//// [index.ts]
+import {e} from "../projC";
+
+export const d = {e};
+
+//// [index.js]
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.d = void 0;
+var projC_1 = require("../projC");
+exports.d = { e: projC_1.e };
+
+
+//// [index.d.ts]
+export declare const d: {
+    e: {
+        f: (foo: import("../projA").Foo) => boolean;
+    };
+};
diff --git a/tests/baselines/reference/declarationEmitCrossFileCopiedGeneratedImportType.symbols b/tests/baselines/reference/declarationEmitCrossFileCopiedGeneratedImportType.symbols
@@ -0,0 +1,37 @@
+//// [tests/cases/compiler/declarationEmitCrossFileCopiedGeneratedImportType.ts] ////
+
+=== projA/index.d.ts ===
+export declare class Foo extends Number {
+>Foo : Symbol(Foo, Decl(index.d.ts, 0, 0))
+>Number : Symbol(Number, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
+
+    private _;
+>_ : Symbol(Foo._, Decl(index.d.ts, 0, 41))
+}
+=== projB/index.d.ts ===
+import { Foo } from "../projA";
+>Foo : Symbol(Foo, Decl(index.d.ts, 0, 8))
+
+export declare const f: (foo: Foo) => boolean;
+>f : Symbol(f, Decl(index.d.ts, 1, 20))
+>foo : Symbol(foo, Decl(index.d.ts, 1, 25))
+>Foo : Symbol(Foo, Decl(index.d.ts, 0, 8))
+
+=== projC/index.d.ts ===
+export declare const e: {
+>e : Symbol(e, Decl(index.d.ts, 0, 20))
+
+    f: (foo: import("../projA").Foo) => boolean;
+>f : Symbol(f, Decl(index.d.ts, 0, 25))
+>foo : Symbol(foo, Decl(index.d.ts, 1, 8))
+>Foo : Symbol(Foo, Decl(index.d.ts, 0, 0))
+
+};
+=== projD/index.ts ===
+import {e} from "../projC";
+>e : Symbol(e, Decl(index.ts, 0, 8))
+
+export const d = {e};
+>d : Symbol(d, Decl(index.ts, 2, 12))
+>e : Symbol(e, Decl(index.ts, 2, 18))
+
diff --git a/tests/baselines/reference/declarationEmitCrossFileCopiedGeneratedImportType.types b/tests/baselines/reference/declarationEmitCrossFileCopiedGeneratedImportType.types
@@ -0,0 +1,48 @@
+//// [tests/cases/compiler/declarationEmitCrossFileCopiedGeneratedImportType.ts] ////
+
+=== projA/index.d.ts ===
+export declare class Foo extends Number {
+>Foo : Foo
+>    : ^^^
+>Number : Number
+>       : ^^^^^^
+
+    private _;
+>_ : any
+}
+=== projB/index.d.ts ===
+import { Foo } from "../projA";
+>Foo : typeof Foo
+>    : ^^^^^^^^^^
+
+export declare const f: (foo: Foo) => boolean;
+>f : (foo: Foo) => boolean
+>  : ^   ^^   ^^^^^       
+>foo : Foo
+>    : ^^^
+
+=== projC/index.d.ts ===
+export declare const e: {
+>e : { f: (foo: import("../projA").Foo) => boolean; }
+>  : ^^^^^                                        ^^^
+
+    f: (foo: import("../projA").Foo) => boolean;
+>f : (foo: import("../projA").Foo) => boolean
+>  : ^   ^^                      ^^^^^       
+>foo : import("projA/index").Foo
+>    : ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+};
+=== projD/index.ts ===
+import {e} from "../projC";
+>e : { f: (foo: import("projA/index").Foo) => boolean; }
+>  : ^^^^^^   ^^                         ^^^^^^^^^^^^^^^
+
+export const d = {e};
+>d : { e: { f: (foo: import("projA/index").Foo) => boolean; }; }
+>  : ^^^^^^^^^^^   ^^                         ^^^^^^^^^^^^^^^^^^
+>{e} : { e: { f: (foo: import("projA/index").Foo) => boolean; }; }
+>    : ^^^^^                                                   ^^^
+>e : { f: (foo: import("projA/index").Foo) => boolean; }
+>  : ^^^^^^   ^^                         ^^^^^^^^^^^^^^^
+
diff --git a/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.js b/tests/baselines/reference/jsDeclarationsClassImplementsGenericsSerialization.js
@@ -68,4 +68,4 @@ export class Encoder<T> implements IEncoder<T> {
      */
     encode(value: T): Uint8Array;
 }
-export type IEncoder<T> = import('./interface').Encoder<T>;
+export type IEncoder<T> = import("./interface").Encoder<T>;
diff --git a/tests/baselines/reference/jsDeclarationsParameterTagReusesInputNodeInEmit2.js b/tests/baselines/reference/jsDeclarationsParameterTagReusesInputNodeInEmit2.js
@@ -63,4 +63,4 @@ declare class Base {
  * @returns {InstanceType<BaseFactory["Base"]>}
  */
 declare function test(base: InstanceType<BaseFactory["Base"]>): InstanceType<BaseFactory["Base"]>;
-type BaseFactory = typeof import('./base');
+type BaseFactory = typeof import("./base");
diff --git a/tests/baselines/reference/jsDeclarationsTypedefPropertyAndExportAssignment.js b/tests/baselines/reference/jsDeclarationsTypedefPropertyAndExportAssignment.js
@@ -148,7 +148,7 @@ declare class MainThreadTasks {
 declare namespace MainThreadTasks {
     export { TaskGroup, TaskNode, PriorTaskData };
 }
-type TaskGroup = import('./module.js').TaskGroup;
+type TaskGroup = import("./module.js").TaskGroup;
 type TaskNode = {
     children: TaskNode[];
     parent: TaskNode | undefined;
diff --git a/tests/baselines/reference/jsDeclarationsUniqueSymbolUsage.js b/tests/baselines/reference/jsDeclarationsUniqueSymbolUsage.js
@@ -28,4 +28,4 @@ export type WithSymbol = {
  * @returns {import('./a').WithSymbol}
  * @param {import('./a').WithSymbol} value
  */
-export function b(value: import('./a').WithSymbol): import('./a').WithSymbol;
+export function b(value: import("./a").WithSymbol): import("./a").WithSymbol;
diff --git a/tests/baselines/reference/jsDeclarationsUniqueSymbolUsage.types b/tests/baselines/reference/jsDeclarationsUniqueSymbolUsage.types
@@ -20,7 +20,7 @@ export const kSymbol = Symbol("my-symbol");
  * @param {import('./a').WithSymbol} value 
  */
 export function b(value) {
->b : (value: import('./a').WithSymbol) => import('./a').WithSymbol
+>b : (value: import("./a").WithSymbol) => import("./a").WithSymbol
 >  : ^     ^^                        ^^^^^                        
 >value : import("a").WithSymbol
 >      : ^^^^^^^^^^^^^^^^^^^^^^
diff --git a/tests/baselines/reference/jsdocThisType.types b/tests/baselines/reference/jsdocThisType.types
@@ -50,9 +50,9 @@ export function f2() {
 
 /** @type {(this: import('./types').Foo) => void} */
 export const f3 = function() {
->f3 : (this: import('./types').Foo) => void
+>f3 : (this: import("./types").Foo) => void
 >   : ^    ^^                     ^^^^^^^^^
->function() {    this.test();} : (this: import('./types').Foo) => void
+>function() {    this.test();} : (this: import("./types").Foo) => void
 >                              : ^    ^^                     ^^^^^^^^^
 
     this.test();
@@ -68,7 +68,7 @@ export const f3 = function() {
 
 /** @type {(this: import('./types').Foo) => void} */
 export function f4() {
->f4 : (this: import('./types').Foo) => void
+>f4 : (this: import("./types").Foo) => void
 >   : ^    ^^                     ^^^^^    
 
     this.test();
@@ -84,9 +84,9 @@ export function f4() {
 
 /** @type {function(this: import('./types').Foo): void} */
 export const f5 = function() {
->f5 : (this: import('./types').Foo) => void
+>f5 : (this: import("./types").Foo) => void
 >   : ^    ^^                     ^^^^^^^^^
->function() {    this.test();} : (this: import('./types').Foo) => void
+>function() {    this.test();} : (this: import("./types").Foo) => void
 >                              : ^    ^^                     ^^^^^^^^^
 
     this.test();
@@ -102,7 +102,7 @@ export const f5 = function() {
 
 /** @type {function(this: import('./types').Foo): void} */
 export function f6() {
->f6 : (this: import('./types').Foo) => void
+>f6 : (this: import("./types").Foo) => void
 >   : ^    ^^                     ^^^^^    
 
     this.test();
diff --git a/tests/baselines/reference/moduleExportAssignment7.types b/tests/baselines/reference/moduleExportAssignment7.types
@@ -227,7 +227,7 @@ function jsvalues(a, b, c, d, e, f, g) {
 
 === index.ts ===
 function types(
->types : (a: any, b: any, c: any, d: any, e: any, f: import('./mod').buz, g: any) => any
+>types : (a: any, b: any, c: any, d: any, e: any, f: import("./mod").buz, g: any) => any
 >      : ^ ^^^^^^^ ^^^^^^^ ^^^^^^^ ^^^^^^^ ^^^^^^^ ^^                   ^^ ^^^^^^^^^^^^^
 
     a: import('./mod').Thing,
@@ -309,7 +309,7 @@ function types(
 }
 
 function values(
->values : (a: typeof import('./mod').Thing, b: typeof import('./mod').AnotherThing, c: typeof import('./mod').foo, d: typeof import('./mod').qux, e: typeof import('./mod').baz, f: any, g: typeof import('./mod').literal) => any
+>values : (a: typeof import("./mod").Thing, b: typeof import("./mod").AnotherThing, c: typeof import("./mod").foo, d: typeof import("./mod").qux, e: typeof import("./mod").baz, f: any, g: typeof import("./mod").literal) => any
 >       : ^ ^^                            ^^ ^^                                   ^^ ^^                          ^^ ^^                          ^^ ^^                          ^^ ^^^^^^^ ^^                              ^^^^^^^^
 
     a: typeof import('./mod').Thing,
diff --git a/tests/baselines/reference/privateNamesUnique-2.types b/tests/baselines/reference/privateNamesUnique-2.types
@@ -65,12 +65,12 @@ const b = new B();
 a.copy(b); // error
 >a.copy(b) : void
 >          : ^^^^
->a.copy : (other: import(} fro).Foo) => void
->       : ^     ^^                 ^^^^^^^^^
+>a.copy : (other: import("b").Foo) => void
+>       : ^     ^^                     ^^^^^^^^^
 >a : A
 >  : ^
->copy : (other: import(} fro).Foo) => void
->     : ^     ^^                 ^^^^^^^^^
+>copy : (other: import("b").Foo) => void
+>     : ^     ^^                     ^^^^^^^^^
 >b : B
 >  : ^
 
diff --git a/tests/baselines/reference/spuriousCircularityOnTypeImport.types b/tests/baselines/reference/spuriousCircularityOnTypeImport.types
@@ -21,7 +21,7 @@ export type SelectorMap<T extends Record<string, (...params: unknown[]) => unkno
 };
 
 export declare const value2: {
->value2 : { sliceSelectors: <FuncMap extends import('./types').SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]>; }; }
+>value2 : { sliceSelectors: <FuncMap extends import("./types").SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]>; }; }
 >       : ^^^^^^^^^^^^^^^^^^                                                                                                                                        ^^^
 
     sliceSelectors: <FuncMap extends import('./types').SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]> };
diff --git a/tests/cases/compiler/declarationEmitCrossFileCopiedGeneratedImportType.ts b/tests/cases/compiler/declarationEmitCrossFileCopiedGeneratedImportType.ts
@@ -0,0 +1,16 @@
+// @declaration: true
+// @filename: projA/index.d.ts
+export declare class Foo extends Number {
+    private _;
+}
+// @filename: projB/index.d.ts
+import { Foo } from "../projA";
+export declare const f: (foo: Foo) => boolean;
+// @filename: projC/index.d.ts
+export declare const e: {
+    f: (foo: import("../projA").Foo) => boolean;
+};
+// @filename: projD/index.ts
+import {e} from "../projC";
+
+export const d = {e};

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,6 @@ import {`
`112`	`112`	`createEvaluator,`
`113`	`113`	`createFileDiagnostic,`
`114`	`114`	`createFlowNode,`
`115`		`- createGetCanonicalFileName,`
`116`	`115`	`createGetSymbolWalker,`
`117`	`116`	`createModeAwareCacheKey,`
`118`	`117`	`createModuleNotFoundChain,`
`@@ -347,7 +346,6 @@ import {`
`347`	`346`	`getPropertyNameFromType,`
`348`	`347`	`getResolutionDiagnostic,`
`349`	`348`	`getResolutionModeOverride,`
`350`		`- getResolvedExternalModuleName,`
`351`	`349`	`getResolveJsonModule,`
`352`	`350`	`getRestParameterElementType,`
`353`	`351`	`getRootDeclaration,`
`@@ -7668,14 +7666,14 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {`
`7668`	`7666`	`return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);`
`7669`	`7667`	`}`
`7670`	`7668`	`}`
`7671`		`- if (!context.enclosingDeclaration \|\| !context.tracker.moduleResolverHost) {`
	`7669`	`+ if (!context.enclosingFile \|\| !context.tracker.moduleResolverHost) {`
`7672`	`7670`	`// If there's no context declaration, we can't lookup a non-ambient specifier, so we just use the symbol name`
`7673`	`7671`	`if (ambientModuleSymbolRegex.test(symbol.escapedName as string)) {`
`7674`	`7672`	`return (symbol.escapedName as string).substring(1, (symbol.escapedName as string).length - 1);`
`7675`	`7673`	`}`
`7676`	`7674`	`return getSourceFileOfNode(getNonAugmentationDeclaration(symbol)!).fileName; // A resolver may not be provided for baselines and errors - in those cases we use the fileName in full`
`7677`	`7675`	`}`
`7678`		`- const contextFile = getSourceFileOfNode(getOriginalNode(context.enclosingDeclaration));`
	`7676`	`+ const contextFile = context.enclosingFile;`
`7679`	`7677`	`const resolutionMode = overrideImportMode \|\| contextFile?.impliedNodeFormat;`
`7680`	`7678`	`const cacheKey = createModeAwareCacheKey(contextFile.path, resolutionMode);`
`7681`	`7679`	`const links = getSymbolLinks(symbol);`
`@@ -8479,22 +8477,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {`
`8479`	`8477`	`}`
`8480`	`8478`
`8481`	`8479`	`function rewriteModuleSpecifier(parent: ImportTypeNode, lit: StringLiteral) {`
`8482`		`- if (context.bundled) {`
`8483`		`- if (context.tracker && context.tracker.moduleResolverHost) {`
`8484`		`- const targetFile = getExternalModuleFileFromDeclaration(parent);`
`8485`		`- if (targetFile) {`
`8486`		`- const getCanonicalFileName = createGetCanonicalFileName(!!host.useCaseSensitiveFileNames);`
`8487`		`- const resolverHost = {`
`8488`		`- getCanonicalFileName,`
`8489`		`- getCurrentDirectory: () => context.tracker.moduleResolverHost!.getCurrentDirectory(),`
`8490`		`- getCommonSourceDirectory: () => context.tracker.moduleResolverHost!.getCommonSourceDirectory(),`
`8491`		`- };`
`8492`		`- const newName = getResolvedExternalModuleName(resolverHost, targetFile);`
`8493`		`- return factory.createStringLiteral(newName);`
	`8480`	`+ if (context.bundled \|\| context.enclosingFile !== getSourceFileOfNode(lit)) {`
	`8481`	`+ const targetFile = getExternalModuleFileFromDeclaration(parent);`
	`8482`	`+ if (targetFile) {`
	`8483`	`+ const newName = getSpecifierForModuleSymbol(targetFile.symbol, context);`
	`8484`	`+ if (newName !== lit.text) {`
	`8485`	`+ return setOriginalNode(factory.createStringLiteral(newName), lit);`
`8494`	`8486`	`}`
`8495`	`8487`	`}`
`8496`	`8488`	`}`
`8497`		`- return lit;`
	`8489`	`+ return visitNode(lit, visitExistingNodeTreeSymbols, isStringLiteral)!;`
`8498`	`8490`	`}`
`8499`	`8491`	`}`
`8500`	`8492`	`}`
Original file line number	Diff line number	Diff line change
`@@ -68,4 +68,4 @@ export class Encoder<T> implements IEncoder<T> {`
`68`	`68`	`*/`
`69`	`69`	`encode(value: T): Uint8Array;`
`70`	`70`	`}`
`71`		`-export type IEncoder<T> = import('./interface').Encoder<T>;`
	`71`	`+export type IEncoder<T> = import("./interface").Encoder<T>;`
Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ declare class MainThreadTasks {`
`148`	`148`	`declare namespace MainThreadTasks {`
`149`	`149`	`export { TaskGroup, TaskNode, PriorTaskData };`
`150`	`150`	`}`
`151`		`-type TaskGroup = import('./module.js').TaskGroup;`
	`151`	`+type TaskGroup = import("./module.js").TaskGroup;`
`152`	`152`	`type TaskNode = {`
`153`	`153`	`children: TaskNode[];`
`154`	`154`	`parent: TaskNode \| undefined;`