Add incremental test where cache should have same resolutions as whats in the program

sheetalkamat · sheetalkamat · commit feea116c71b3 · 2023-10-20T16:01:30.000-07:00
This shows cache is holding onto resolutions that are no longer needed by program because either those modules arent present in file or is determined to be ambient resolution
diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -947,7 +947,8 @@ export function getResolutionModeOverride(node: ImportAttributes | undefined, gr
     return elem.value.text === "import" ? ModuleKind.ESNext : ModuleKind.CommonJS;
 }
 
-const emptyResolution: ResolvedModuleWithFailedLookupLocations & ResolvedTypeReferenceDirectiveWithFailedLookupLocations = {
+/** @internal */
+export const emptyResolution: ResolvedModuleWithFailedLookupLocations & ResolvedTypeReferenceDirectiveWithFailedLookupLocations = {
     resolvedModule: undefined,
     resolvedTypeReferenceDirective: undefined,
 };
@@ -1112,6 +1113,13 @@ function forEachProjectReference<T>(
 /** @internal */
 export const inferredTypesContainingFile = "__inferred type names__.ts";
 
+/** @internal */
+export function getAutomaticTypeDirectiveContainingFile(options: CompilerOptions, currentDirectory: string) {
+    // This containingFilename needs to match with the one used in managed-side
+    const containingDirectory = options.configFilePath ? getDirectoryPath(options.configFilePath) : currentDirectory;
+    return combinePaths(containingDirectory, inferredTypesContainingFile);
+}
+
 /** @internal */
 export function getInferredLibraryNameResolveFrom(options: CompilerOptions, currentDirectory: string, libFileName: string) {
     const containingDirectory = options.configFilePath ? getDirectoryPath(options.configFilePath) : currentDirectory;
@@ -1762,10 +1770,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
         automaticTypeDirectiveResolutions = createModeAwareCache();
         if (automaticTypeDirectiveNames.length) {
             tracing?.push(tracing.Phase.Program, "processTypeReferences", { count: automaticTypeDirectiveNames.length });
-            // This containingFilename needs to match with the one used in managed-side
-            const containingDirectory = options.configFilePath ? getDirectoryPath(options.configFilePath) : currentDirectory;
-            const containingFilename = combinePaths(containingDirectory, inferredTypesContainingFile);
-            const resolutions = resolveTypeReferenceDirectiveNamesReusingOldState(automaticTypeDirectiveNames, containingFilename);
+            const resolutions = resolveTypeReferenceDirectiveNamesReusingOldState(automaticTypeDirectiveNames, getAutomaticTypeDirectiveContainingFile(options, currentDirectory));
             for (let i = 0; i < automaticTypeDirectiveNames.length; i++) {
                 // under node16/nodenext module resolution, load `types`/ata include names as cjs resolution results by passing an `undefined` mode
                 automaticTypeDirectiveResolutions.set(automaticTypeDirectiveNames[i], /*mode*/ undefined, resolutions[i]);
@@ -2153,8 +2158,6 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
          */
         let result: ResolvedModuleWithFailedLookupLocations[] | undefined;
         let reusedNames: StringLiteralLike[] | undefined;
-        /** A transient placeholder used to mark predicted resolution in the result list. */
-        const predictedToResolveToAmbientModuleMarker: ResolvedModuleWithFailedLookupLocations = emptyResolution;
         const oldSourceFile = oldProgram && oldProgram.getSourceFile(file.fileName);
 
         for (let i = 0; i < moduleNames.length; i++) {
@@ -2197,7 +2200,7 @@ export function createProgram(rootNamesOrOptions: readonly string[] | CreateProg
             }
 
             if (resolvesToAmbientModuleInNonModifiedFile) {
-                (result || (result = new Array(moduleNames.length)))[i] = predictedToResolveToAmbientModuleMarker;
+                (result || (result = new Array(moduleNames.length)))[i] = emptyResolution;
             }
             else {
                 // Resolution failed in the old program, or resolved to an ambient module for which we can't reuse the result.
diff --git a/src/harness/incrementalUtils.ts b/src/harness/incrementalUtils.ts
@@ -204,6 +204,7 @@ export function verifyResolutionCache(
     actualProgram: ts.Program,
     resolutionHostCacheHost: ts.ResolutionCacheHost,
     projectName: string,
+    userResolvedModuleNames?: true,
 ) {
     const currentDirectory = resolutionHostCacheHost.getCurrentDirectory!();
     const expected = ts.createResolutionCache(resolutionHostCacheHost, actual.rootDirForResolution);
@@ -214,6 +215,7 @@ export function verifyResolutionCache(
     const expectedToResolution = new Map<ExpectedResolution, ts.ResolutionWithFailedLookupLocations>();
     const resolutionToExpected = new Map<ts.ResolutionWithFailedLookupLocations, ExpectedResolution>();
     const resolutionToRefs = new Map<ts.ResolutionWithFailedLookupLocations, ResolutionInfo[]>();
+    const inferredTypesPath = resolutionHostCacheHost.toPath(ts.getAutomaticTypeDirectiveContainingFile(actualProgram.getCompilerOptions(), currentDirectory));
     actual.resolvedModuleNames.forEach((resolutions, path) =>
         collectResolutionToRefFromCache(
             "Modules",
@@ -222,6 +224,7 @@ export function verifyResolutionCache(
             getResolvedModuleFileName,
             /*deferWatchingNonRelativeResolution*/ true,
             expected.resolvedModuleNames,
+            (name, mode) => actualProgram.getResolvedModule(actualProgram.getSourceFileByPath(path)!, name, mode),
         )
     );
     actual.resolvedTypeReferenceDirectives.forEach((resolutions, path) =>
@@ -232,6 +235,10 @@ export function verifyResolutionCache(
             getResolvedTypeRefFileName,
             /*deferWatchingNonRelativeResolution*/ false,
             expected.resolvedTypeReferenceDirectives,
+            (name, mode) =>
+                path !== inferredTypesPath ?
+                    actualProgram.getResolvedTypeReferenceDirective(actualProgram.getSourceFileByPath(path)!, name, mode) :
+                    actualProgram.getAutomaticTypeDirectiveResolutions().get(name, mode),
         )
     );
     actual.resolvedLibraries.forEach((resolved, libFileName) => {
@@ -248,6 +255,39 @@ export function verifyResolutionCache(
         );
         expected.resolvedLibraries.set(libFileName, expectedResolution);
     });
+    // Check for resolutions in program but not in cache to empty resolutions
+    if (!userResolvedModuleNames) {
+        actualProgram.forEachResolvedModule((resolution, name, mode, filePath) =>
+            verifyResolutionIsInCache(
+                "Modules",
+                actual.resolvedModuleNames.get(filePath),
+                resolution,
+                name,
+                mode,
+                filePath,
+            )
+        );
+    }
+    actualProgram.forEachResolvedTypeReferenceDirective((resolution, name, mode, filePath) =>
+        verifyResolutionIsInCache(
+            "TypeRefs",
+            actual.resolvedTypeReferenceDirectives.get(filePath),
+            resolution,
+            name,
+            mode,
+            filePath,
+        )
+    );
+    actualProgram.getAutomaticTypeDirectiveResolutions().forEach((resolution, name, mode) =>
+        verifyResolutionIsInCache(
+            "AutoTypeRefs",
+            actual.resolvedTypeReferenceDirectives.get(inferredTypesPath),
+            resolution,
+            name,
+            mode,
+            inferredTypesPath,
+        )
+    );
 
     expected.finishCachingPerDirectoryResolution(actualProgram, /*oldProgram*/ undefined);
 
@@ -260,6 +300,10 @@ export function verifyResolutionCache(
                 `Expected from:: ${JSON.stringify(info, undefined, " ")}` +
                 `Actual from: ${resolution.refCount}`,
         );
+        ts.Debug.assert(
+            !resolution.isInvalidated,
+            `${projectName}:: Resolution should not be invalidated`,
+        );
         ts.Debug.assert(
             resolutionToExpected.get(resolution)!.refCount === resolution.refCount,
             `${projectName}:: Expected Resolution ref count ${resolutionToExpected.get(resolution)!.refCount} but got ${resolution.refCount}`,
@@ -298,24 +342,54 @@ export function verifyResolutionCache(
     ts.Debug.assert(expected.countResolutionsResolvedWithGlobalCache() === 0, `${projectName}:: ResolutionsResolvedWithGlobalCache should be cleared`);
     ts.Debug.assert(expected.countResolutionsResolvedWithoutGlobalCache() === 0, `${projectName}:: ResolutionsResolvedWithoutGlobalCache should be cleared`);
 
+    function verifyResolutionIsInCache<T extends ts.ResolutionWithFailedLookupLocations>(
+        cacheType: string,
+        cache: ts.ModeAwareCache<T> | undefined,
+        resolution: T,
+        name: string,
+        mode: ts.ResolutionMode,
+        fileName: string,
+    ) {
+        if (resolution as unknown !== ts.emptyResolution) {
+            // Resolutions should match
+            ts.Debug.assert(
+                cache?.get(name, mode) === resolution,
+                `${projectName}:: ${cacheType}:: ${name}:: ${mode} Expected resolution in program to be in cache ${fileName}`,
+            );
+        }
+        else {
+            // EmptyResolution is place holder and shouldnt be in the cache
+            ts.Debug.assert(
+                !cache?.has(name, mode),
+                `${projectName}:: ${cacheType}:: ${name}:: ${mode} Ambient moduleResolution, should not be watched ${fileName}`,
+            );
+        }
+    }
+
     function collectResolutionToRefFromCache<T extends ts.ResolutionWithFailedLookupLocations>(
         cacheType: string,
         fileName: ts.Path,
         cache: ts.ModeAwareCache<T> | undefined,
         getResolvedFileName: (resolution: T) => string | undefined,
         deferWatchingNonRelativeResolution: boolean,
-        storeExpcted: Map<ts.Path, ts.ModeAwareCache<ts.ResolutionWithFailedLookupLocations>>,
+        storeExpected: Map<ts.Path, ts.ModeAwareCache<ts.ResolutionWithFailedLookupLocations>>,
+        getProgramResolutions: (name: string, mode: ts.ResolutionMode) => T | undefined,
     ) {
         ts.Debug.assert(
-            actualProgram.getSourceFileByPath(fileName) || ts.endsWith(fileName, ts.inferredTypesContainingFile),
+            actualProgram.getSourceFileByPath(fileName) || inferredTypesPath === fileName,
             `${projectName}:: ${cacheType} ${fileName} Expect cache for file in program or auto type ref`,
         );
         let expectedCache: ts.ModeAwareCache<ts.ResolutionWithFailedLookupLocations> | undefined;
         cache?.forEach((resolved, name, mode) => {
             const resolvedFileName = getResolvedFileName(resolved);
             const expected = collectResolution(cacheType, fileName, resolved, resolvedFileName, name, mode, deferWatchingNonRelativeResolution);
-            if (!expectedCache) storeExpcted.set(fileName, expectedCache = ts.createModeAwareCache());
+            if (!expectedCache) storeExpected.set(fileName, expectedCache = ts.createModeAwareCache());
             expectedCache.set(name, mode, expected);
+            // Resolution in cache should be same as that is in program
+            ts.Debug.assert(
+                resolved === getProgramResolutions(name, mode),
+                `${projectName}:: ${cacheType} ${fileName} ${name} ${mode} Expected resolution in cache to be matched to that in the program`,
+            );
         });
     }
 
diff --git a/src/testRunner/unittests/helpers/tscWatch.ts b/src/testRunner/unittests/helpers/tscWatch.ts
@@ -221,6 +221,7 @@ export interface RunWatchBaseline<T extends ts.BuilderProgram> extends BaselineB
     getPrograms: () => readonly CommandLineProgram[];
     watchOrSolution: WatchOrSolution<T>;
     useSourceOfProjectReferenceRedirect?: () => boolean;
+    userResolvedModuleNames?: true;
 }
 export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanticDiagnosticsBuilderProgram>({
     scenario,
@@ -235,6 +236,7 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
     edits,
     watchOrSolution,
     useSourceOfProjectReferenceRedirect,
+    userResolvedModuleNames,
 }: RunWatchBaseline<T>) {
     baseline.push(`${sys.getExecutingFilePath()} ${commandLineArgs.join(" ")}`);
     let programs = watchBaseline({
@@ -263,6 +265,7 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
                 resolutionCache: (watchOrSolution as ts.WatchOfConfigFile<T> | undefined)?.getResolutionCache?.(),
                 useSourceOfProjectReferenceRedirect,
                 symlinksNotReflected,
+                userResolvedModuleNames,
             });
         }
     }
@@ -284,6 +287,7 @@ export interface WatchBaseline extends BaselineBase, TscWatchCheckOptions {
     resolutionCache?: ts.ResolutionCache;
     useSourceOfProjectReferenceRedirect?: () => boolean;
     symlinksNotReflected?: readonly string[];
+    userResolvedModuleNames?: true;
 }
 export function watchBaseline({
     baseline,
@@ -297,6 +301,7 @@ export function watchBaseline({
     resolutionCache,
     useSourceOfProjectReferenceRedirect,
     symlinksNotReflected,
+    userResolvedModuleNames,
 }: WatchBaseline) {
     if (baselineSourceMap) generateSourceMapBaselineFiles(sys);
     sys.serializeOutput(baseline);
@@ -311,7 +316,15 @@ export function watchBaseline({
     // Verify program structure and resolution cache when incremental edit with tsc --watch (without build mode)
     if (resolutionCache && programs.length) {
         ts.Debug.assert(programs.length === 1);
-        verifyProgramStructureAndResolutionCache(caption!, sys, programs[0][0], resolutionCache, useSourceOfProjectReferenceRedirect, symlinksNotReflected);
+        verifyProgramStructureAndResolutionCache(
+            caption!,
+            sys,
+            programs[0][0],
+            resolutionCache,
+            useSourceOfProjectReferenceRedirect,
+            symlinksNotReflected,
+            userResolvedModuleNames,
+        );
     }
     sys.writtenFiles.clear();
     return programs;
@@ -321,8 +334,9 @@ function verifyProgramStructureAndResolutionCache(
     sys: TscWatchSystem,
     program: ts.Program,
     resolutionCache: ts.ResolutionCache,
-    useSourceOfProjectReferenceRedirect?: () => boolean,
-    symlinksNotReflected?: readonly string[],
+    useSourceOfProjectReferenceRedirect: (() => boolean) | undefined,
+    symlinksNotReflected: readonly string[] | undefined,
+    userResolvedModuleNames: true | undefined,
 ) {
     const options = program.getCompilerOptions();
     const compilerHost = ts.createCompilerHostWorker(options, /*setParentNodes*/ undefined, sys);
@@ -349,24 +363,30 @@ function verifyProgramStructureAndResolutionCache(
         program,
         caption,
     );
-    verifyResolutionCache(resolutionCache, program, {
-        ...compilerHost,
+    verifyResolutionCache(
+        resolutionCache,
+        program,
+        {
+            ...compilerHost,
 
-        getCompilerHost: () => compilerHost,
-        toPath: fileName => sys.toPath(fileName),
-        getCompilationSettings: () => options,
-        fileIsOpen: ts.returnFalse,
-        getCurrentProgram: () => program,
+            getCompilerHost: () => compilerHost,
+            toPath: fileName => sys.toPath(fileName),
+            getCompilationSettings: () => options,
+            fileIsOpen: ts.returnFalse,
+            getCurrentProgram: () => program,
 
-        watchDirectoryOfFailedLookupLocation: ts.returnNoopFileWatcher,
-        watchAffectingFileLocation: ts.returnNoopFileWatcher,
-        onInvalidatedResolution: ts.noop,
-        watchTypeRootsDirectory: ts.returnNoopFileWatcher,
-        onChangedAutomaticTypeDirectiveNames: ts.noop,
-        scheduleInvalidateResolutionsOfFailedLookupLocations: ts.noop,
-        getCachedDirectoryStructureHost: ts.returnUndefined,
-        writeLog: ts.noop,
-    }, caption);
+            watchDirectoryOfFailedLookupLocation: ts.returnNoopFileWatcher,
+            watchAffectingFileLocation: ts.returnNoopFileWatcher,
+            onInvalidatedResolution: ts.noop,
+            watchTypeRootsDirectory: ts.returnNoopFileWatcher,
+            onChangedAutomaticTypeDirectiveNames: ts.noop,
+            scheduleInvalidateResolutionsOfFailedLookupLocations: ts.noop,
+            getCachedDirectoryStructureHost: ts.returnUndefined,
+            writeLog: ts.noop,
+        },
+        caption,
+        userResolvedModuleNames,
+    );
 }
 export interface VerifyTscWatch extends TscWatchCompile {
     baselineIncremental?: boolean;
diff --git a/src/testRunner/unittests/tscWatch/watchApi.ts b/src/testRunner/unittests/tscWatch/watchApi.ts
@@ -125,6 +125,7 @@ describe("unittests:: tsc-watch:: watchAPI:: tsc-watch with custom module resolu
                         },
                     ],
                     watchOrSolution: watch,
+                    userResolvedModuleNames: true,
                 });
             });
         }

Original file line number	Diff line number	Diff line change
`@@ -125,6 +125,7 @@ describe("unittests:: tsc-watch:: watchAPI:: tsc-watch with custom module resolu`
`125`	`125`	`},`
`126`	`126`	`],`
`127`	`127`	`watchOrSolution: watch,`
	`128`	`+ userResolvedModuleNames: true,`
`128`	`129`	`});`
`129`	`130`	`});`
`130`	`131`	`}`