Do not reuse ambient module name resolution from other files while determining if resolution can be reused (#59243)

Author: sheetalkamat

src/compiler/checker.ts

@@ -1794,11 +1794,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         tryGetMemberInModuleExports: (name, symbol) => tryGetMemberInModuleExports(escapeLeadingUnderscores(name), symbol),
         tryGetMemberInModuleExportsAndProperties: (name, symbol) => tryGetMemberInModuleExportsAndProperties(escapeLeadingUnderscores(name), symbol),
         tryFindAmbientModule: moduleName => tryFindAmbientModule(moduleName, /*withAugmentations*/ true),
-        tryFindAmbientModuleWithoutAugmentations: moduleName => {
-            // we deliberately exclude augmentations
-            // since we are only interested in declarations of the module itself
-            return tryFindAmbientModule(moduleName, /*withAugmentations*/ false);
-        },
         getApparentType,
         getUnionType,
         isTypeAssignableTo,

src/compiler/diagnosticMessages.json

@@ -5113,10 +5113,6 @@
         "category": "Message",
         "code": 6144
     },
-    "Module '{0}' was resolved as ambient module declared in '{1}' since this file was not modified.": {
-        "category": "Message",
-        "code": 6145
-    },
     "Specify the JSX factory function to use when targeting 'react' JSX emit, e.g. 'React.createElement' or 'h'.": {
         "category": "Message",
         "code": 6146

src/compiler/program.ts

@@ -1545,7 +1545,6 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
     let commonSourceDirectory: string;
     let typeChecker: TypeChecker;
     let classifiableNames: Set<__String>;
-    const ambientModuleNameToUnmodifiedFileName = new Map<string, string>();
     let fileReasons = createMultiMap<Path, FileIncludeReason>();
     let filesWithReferencesProcessed: Set<Path> | undefined;
     let fileReasonsToChain: Map<Path, FileReasonToChainCache> | undefined;
@@ -2250,52 +2249,10 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
             canReuseResolutionsInFile: () =>
                 containingFile === oldProgram?.getSourceFile(containingFile.fileName) &&
                 !hasInvalidatedResolutions(containingFile.path),
-            isEntryResolvingToAmbientModule: moduleNameResolvesToAmbientModule,
+            resolveToOwnAmbientModule: true,
         });
     }
 
-    function moduleNameResolvesToAmbientModule(moduleName: StringLiteralLike, file: SourceFile) {
-        // We know moduleName resolves to an ambient module provided that moduleName:
-        // - is in the list of ambient modules locally declared in the current source file.
-        // - resolved to an ambient module in the old program whose declaration is in an unmodified file
-        //   (so the same module declaration will land in the new program)
-        if (contains(file.ambientModuleNames, moduleName.text)) {
-            if (isTraceEnabled(options, host)) {
-                trace(host, Diagnostics.Module_0_was_resolved_as_locally_declared_ambient_module_in_file_1, moduleName.text, getNormalizedAbsolutePath(file.originalFileName, currentDirectory));
-            }
-            return true;
-        }
-        else {
-            return moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName, file);
-        }
-    }
-
-    // If we change our policy of rechecking failed lookups on each program create,
-    // we should adjust the value returned here.
-    function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: StringLiteralLike, file: SourceFile): boolean {
-        const resolutionToFile = oldProgram?.getResolvedModule(file, moduleName.text, getModeForUsageLocation(file, moduleName))?.resolvedModule;
-        const resolvedFile = resolutionToFile && oldProgram!.getSourceFile(resolutionToFile.resolvedFileName);
-        if (resolutionToFile && resolvedFile) {
-            // In the old program, we resolved to an ambient module that was in the same
-            //   place as we expected to find an actual module file.
-            // We actually need to return 'false' here even though this seems like a 'true' case
-            //   because the normal module resolution algorithm will find this anyway.
-            return false;
-        }
-
-        // at least one of declarations should come from non-modified source file
-        const unmodifiedFile = ambientModuleNameToUnmodifiedFileName.get(moduleName.text);
-
-        if (!unmodifiedFile) {
-            return false;
-        }
-
-        if (isTraceEnabled(options, host)) {
-            trace(host, Diagnostics.Module_0_was_resolved_as_ambient_module_declared_in_1_since_this_file_was_not_modified, moduleName.text, unmodifiedFile);
-        }
-        return true;
-    }
-
     function resolveTypeReferenceDirectiveNamesReusingOldState(typeDirectiveNames: readonly FileReference[], containingFile: SourceFile): readonly ResolvedTypeReferenceDirectiveWithFailedLookupLocations[];
     function resolveTypeReferenceDirectiveNamesReusingOldState(typeDirectiveNames: readonly string[], containingFile: string): readonly ResolvedTypeReferenceDirectiveWithFailedLookupLocations[];
     function resolveTypeReferenceDirectiveNamesReusingOldState<T extends string | FileReference>(typeDirectiveNames: readonly T[], containingFile: string | SourceFile): readonly ResolvedTypeReferenceDirectiveWithFailedLookupLocations[] {
@@ -2333,7 +2290,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
         getResolutionFromOldProgram: (name: string, mode: ResolutionMode) => Resolution | undefined;
         getResolved: (oldResolution: Resolution) => ResolutionWithResolvedFileName | undefined;
         canReuseResolutionsInFile: () => boolean;
-        isEntryResolvingToAmbientModule?: (entry: Entry, containingFile: SourceFileOrString) => boolean;
+        resolveToOwnAmbientModule?: true;
     }
 
     function resolveNamesReusingOldState<Entry, SourceFileOrString, SourceFileOrUndefined extends SourceFile | undefined, Resolution>({
@@ -2346,10 +2303,10 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
         getResolutionFromOldProgram,
         getResolved,
         canReuseResolutionsInFile,
-        isEntryResolvingToAmbientModule,
+        resolveToOwnAmbientModule,
     }: ResolveNamesReusingOldStateInput<Entry, SourceFileOrString, SourceFileOrUndefined, Resolution>): readonly Resolution[] {
         if (!entries.length) return emptyArray;
-        if (structureIsReused === StructureIsReused.Not && (!isEntryResolvingToAmbientModule || !containingSourceFile!.ambientModuleNames.length)) {
+        if (structureIsReused === StructureIsReused.Not && (!resolveToOwnAmbientModule || !containingSourceFile!.ambientModuleNames.length)) {
             // If the old program state does not permit reusing resolutions and `file` does not contain locally defined ambient modules,
             // the best we can do is fallback to the default logic.
             return resolutionWorker(
@@ -2394,14 +2351,27 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
                     continue;
                 }
             }
-            if (isEntryResolvingToAmbientModule?.(entry, containingFile)) {
-                (result ??= new Array(entries.length))[i] = emptyResolution;
-            }
-            else {
-                // Resolution failed in the old program, or resolved to an ambient module for which we can't reuse the result.
-                (unknownEntries ??= []).push(entry);
-                (unknownEntryIndices ??= []).push(i);
+            if (resolveToOwnAmbientModule) {
+                const name = nameAndModeGetter.getName(entry);
+                // We know moduleName resolves to an ambient module provided that moduleName:
+                // - is in the list of ambient modules locally declared in the current source file.
+                if (contains(containingSourceFile!.ambientModuleNames, name)) {
+                    if (isTraceEnabled(options, host)) {
+                        trace(
+                            host,
+                            Diagnostics.Module_0_was_resolved_as_locally_declared_ambient_module_in_file_1,
+                            name,
+                            getNormalizedAbsolutePath(containingSourceFile!.originalFileName, currentDirectory),
+                        );
+                    }
+                    (result ??= new Array(entries.length))[i] = emptyResolution;
+                    continue;
+                }
             }
+
+            // Resolution failed in the old program, or resolved to an ambient module for which we can't reuse the result.
+            (unknownEntries ??= []).push(entry);
+            (unknownEntryIndices ??= []).push(i);
         }
 
         if (!unknownEntries) return result!;
@@ -2586,11 +2556,6 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
                 // add file to the modified list so that we will resolve it later
                 modifiedSourceFiles.push(newSourceFile);
             }
-            else {
-                for (const moduleName of oldSourceFile.ambientModuleNames) {
-                    ambientModuleNameToUnmodifiedFileName.set(moduleName, oldSourceFile.fileName);
-                }
-            }
 
             // if file has passed all checks it should be safe to reuse it
             newSourceFiles.push(newSourceFile);

src/compiler/resolutionCache.ts

@@ -6,7 +6,6 @@ import {
     CompilerOptions,
     createModeAwareCache,
     createModuleResolutionCache,
-    createMultiMap,
     createTypeReferenceDirectiveResolutionCache,
     createTypeReferenceResolutionLoader,
     Debug,
@@ -572,7 +571,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
     let filesWithChangedSetOfUnresolvedImports: Path[] | undefined;
     let filesWithInvalidatedResolutions: Set<Path> | undefined;
     let filesWithInvalidatedNonRelativeUnresolvedImports: ReadonlyMap<Path, readonly string[]> | undefined;
-    const nonRelativeExternalModuleResolutions = createMultiMap<string, ResolutionWithFailedLookupLocations>();
+    const nonRelativeExternalModuleResolutions = new Set<ResolutionWithFailedLookupLocations>();
 
     const resolutionsWithFailedLookups = new Set<ResolutionWithFailedLookupLocations>();
     const resolutionsWithOnlyAffectingLocations = new Set<ResolutionWithFailedLookupLocations>();
@@ -755,8 +754,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
         libraryResolutionCache.clearAllExceptPackageJsonInfoCache();
         // perDirectoryResolvedModuleNames and perDirectoryResolvedTypeReferenceDirectives could be non empty if there was exception during program update
         // (between startCachingPerDirectoryResolution and finishCachingPerDirectoryResolution)
-        nonRelativeExternalModuleResolutions.forEach(watchFailedLookupLocationOfNonRelativeModuleResolutions);
-        nonRelativeExternalModuleResolutions.clear();
+        watchFailedLookupLocationOfNonRelativeModuleResolutions();
         isSymlinkCache.clear();
     }
 
@@ -776,8 +774,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
     function finishCachingPerDirectoryResolution(newProgram: Program | undefined, oldProgram: Program | undefined) {
         filesWithInvalidatedNonRelativeUnresolvedImports = undefined;
         allModuleAndTypeResolutionsAreInvalidated = false;
-        nonRelativeExternalModuleResolutions.forEach(watchFailedLookupLocationOfNonRelativeModuleResolutions);
-        nonRelativeExternalModuleResolutions.clear();
+        watchFailedLookupLocationOfNonRelativeModuleResolutions();
         // Update file watches
         if (newProgram !== oldProgram) {
             cleanupLibResolutionWatching(newProgram);
@@ -1103,7 +1100,7 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
             watchFailedLookupLocationOfResolution(resolution);
         }
         else {
-            nonRelativeExternalModuleResolutions.add(name, resolution);
+            nonRelativeExternalModuleResolutions.add(resolution);
         }
         const resolved = getResolutionWithResolvedFileName(resolution);
         if (resolved && resolved.resolvedFileName) {
@@ -1236,14 +1233,9 @@ export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootD
         packageJsonMap?.delete(resolutionHost.toPath(path));
     }
 
-    function watchFailedLookupLocationOfNonRelativeModuleResolutions(resolutions: ResolutionWithFailedLookupLocations[], name: string) {
-        const program = resolutionHost.getCurrentProgram();
-        if (!program || !program.getTypeChecker().tryFindAmbientModuleWithoutAugmentations(name)) {
-            resolutions.forEach(watchFailedLookupLocationOfResolution);
-        }
-        else {
-            resolutions.forEach(resolution => watchAffectingLocationsOfResolution(resolution, /*addToResolutionsWithOnlyAffectingLocations*/ true));
-        }
+    function watchFailedLookupLocationOfNonRelativeModuleResolutions() {
+        nonRelativeExternalModuleResolutions.forEach(watchFailedLookupLocationOfResolution);
+        nonRelativeExternalModuleResolutions.clear();
     }
 
     function createDirectoryWatcherForPackageDir(

src/compiler/types.ts

@@ -5202,7 +5202,6 @@ export interface TypeChecker {
     /** @internal */ createIndexInfo(keyType: Type, type: Type, isReadonly: boolean, declaration?: SignatureDeclaration): IndexInfo;
     /** @internal */ isSymbolAccessible(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, shouldComputeAliasToMarkVisible: boolean): SymbolAccessibilityResult;
     /** @internal */ tryFindAmbientModule(moduleName: string): Symbol | undefined;
-    /** @internal */ tryFindAmbientModuleWithoutAugmentations(moduleName: string): Symbol | undefined;
 
     /** @internal */ getSymbolWalker(accept?: (symbol: Symbol) => boolean): SymbolWalker;
 

src/testRunner/unittests/helpers/tscWatch.ts

@@ -40,8 +40,6 @@ export interface TscWatchCompileChange<T extends ts.BuilderProgram = ts.EmitAndS
         programs: readonly CommandLineProgram[],
         watchOrSolution: WatchOrSolution<T>,
     ) => void;
-    // TODO:: sheetal: Needing these fields are technically issues that need to be fixed later
-    skipStructureCheck?: true;
 }
 export interface TscWatchCheckOptions {
     baselineSourceMap?: boolean;
@@ -214,7 +212,7 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
     });
 
     if (edits) {
-        for (const { caption, edit, timeouts, skipStructureCheck } of edits) {
+        for (const { caption, edit, timeouts } of edits) {
             applyEdit(sys, baseline, edit, caption);
             timeouts(sys, programs, watchOrSolution);
             programs = watchBaseline({
@@ -225,7 +223,7 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
                 baselineSourceMap,
                 baselineDependencies,
                 caption,
-                resolutionCache: !skipStructureCheck ? (watchOrSolution as ts.WatchOfConfigFile<T> | undefined)?.getResolutionCache?.() : undefined,
+                resolutionCache: (watchOrSolution as ts.WatchOfConfigFile<T> | undefined)?.getResolutionCache?.(),
                 useSourceOfProjectReferenceRedirect,
             });
         }

src/testRunner/unittests/reuseProgramStructure.ts

@@ -367,7 +367,7 @@ describe("unittests:: reuseProgramStructure:: General", () => {
         runBaseline("fetches imports after npm install", baselines);
     });
 
-    it("can reuse ambient module declarations from non-modified files", () => {
+    it("should not reuse ambient module declarations from non-modified files", () => {
         const files = [
             { name: "/a/b/app.ts", text: SourceText.New("", "import * as fs from 'fs'", "") },
             { name: "/a/b/node.d.ts", text: SourceText.New("", "", "declare module 'fs' {}") },
@@ -387,7 +387,7 @@ describe("unittests:: reuseProgramStructure:: General", () => {
             f[1].text = f[1].text.updateProgram("declare var process: any");
         });
         baselineProgram(baselines, program3);
-        runBaseline("can reuse ambient module declarations from non-modified files", baselines);
+        runBaseline("should not reuse ambient module declarations from non-modified files", baselines);
     });
 
     it("can reuse module resolutions from non-modified files", () => {

src/testRunner/unittests/tscWatch/moduleResolution.ts

@@ -13,7 +13,7 @@ import {
     libFile,
 } from "../helpers/virtualFileSystemWithWatch.js";
 
-describe("unittests:: tsc-watch:: moduleResolution", () => {
+describe("unittests:: tsc-watch:: moduleResolution::", () => {
     verifyTscWatch({
         scenario: "moduleResolution",
         subScenario: `watches for changes to package-json main fields`,
@@ -640,4 +640,76 @@ describe("unittests:: tsc-watch:: moduleResolution", () => {
             },
         ],
     });
+
+    verifyTscWatch({
+        scenario: "moduleResolution",
+        subScenario: "ambient module names are resolved correctly",
+        commandLineArgs: ["-w", "--extendedDiagnostics", "--explainFiles"],
+        sys: () =>
+            createWatchedSystem({
+                "/home/src/project/tsconfig.json": jsonToReadableText({
+                    compilerOptions: {
+                        noEmit: true,
+                        traceResolution: true,
+                    },
+                    include: ["**/*.ts"],
+                }),
+                "/home/src/project/witha/node_modules/mymodule/index.d.ts": Utils.dedent`
+                    declare module 'mymodule' {
+                        export function readFile(): void;
+                    }
+                    declare module 'mymoduleutils' {
+                        export function promisify(): void;
+                    }
+                `,
+                "/home/src/project/witha/a.ts": Utils.dedent`
+                    import { readFile } from 'mymodule';
+                    import { promisify, promisify2 } from 'mymoduleutils';
+                    readFile();
+                    promisify();
+                    promisify2();
+                `,
+                "/home/src/project/withb/node_modules/mymodule/index.d.ts": Utils.dedent`
+                    declare module 'mymodule' {
+                        export function readFile(): void;
+                    }
+                    declare module 'mymoduleutils' {
+                        export function promisify2(): void;
+                    }
+                `,
+                "/home/src/project/withb/b.ts": Utils.dedent`
+                    import { readFile } from 'mymodule';
+                    import { promisify, promisify2 } from 'mymoduleutils';
+                    readFile();
+                    promisify();
+                    promisify2();
+                `,
+                [libFile.path]: libFile.content,
+            }, { currentDirectory: "/home/src/project" }),
+        edits: [
+            {
+                caption: "remove a file that will remove module augmentation",
+                edit: sys => {
+                    sys.replaceFileText("/home/src/project/withb/b.ts", `import { readFile } from 'mymodule';`, "");
+                    sys.deleteFile("/home/src/project/withb/node_modules/mymodule/index.d.ts");
+                },
+                timeouts: sys => sys.runQueuedTimeoutCallbacks(),
+            },
+            {
+                caption: "write a file that will add augmentation",
+                edit: sys => {
+                    sys.ensureFileOrFolder({
+                        path: "/home/src/project/withb/node_modules/mymoduleutils/index.d.ts",
+                        content: Utils.dedent`
+                            declare module 'mymoduleutils' {
+                                export function promisify2(): void;
+                            }
+                        `,
+                    });
+                    sys.replaceFileText("/home/src/project/withb/b.ts", `readFile();`, "");
+                },
+                timeouts: sys => sys.runQueuedTimeoutCallbacks(),
+            },
+        ],
+    });
 });