From 38b0c2ee41c207f6c0ce810d904c012f72e640ed Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 27 Sep 2023 14:54:33 -0700 Subject: [PATCH] Add incremental test where cache should have same resolutions as whats in the program 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 --- src/compiler/program.ts | 18 ++-- src/harness/incrementalUtils.ts | 85 ++++++++++++++++++- src/testRunner/unittests/helpers/tscWatch.ts | 50 ++++++----- src/testRunner/unittests/tscWatch/watchApi.ts | 1 + 4 files changed, 127 insertions(+), 27 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index c20b5df5498..3ff55e61154 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -1022,7 +1022,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, }; @@ -1201,6 +1202,13 @@ function forEachProjectReference( /** @internal */ export const inferredTypesContainingFile = "__inferred type names__.ts"; +/** @internal */ +export function getAutomaticTypeDirectiveContainingFile(options: CompilerOptions, currentDirectory: string): 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): string { const containingDirectory = options.configFilePath ? getDirectoryPath(options.configFilePath) : currentDirectory; @@ -1874,10 +1882,10 @@ 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]); diff --git a/src/harness/incrementalUtils.ts b/src/harness/incrementalUtils.ts index 10112563ace..31f605e86f1 100644 --- 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, ): void { const currentDirectory = resolutionHostCacheHost.getCurrentDirectory!(); const expected = ts.createResolutionCache(resolutionHostCacheHost, actual.rootDirForResolution); @@ -214,6 +215,12 @@ export function verifyResolutionCache( const expectedToResolution = new Map(); const resolutionToExpected = new Map(); const resolutionToRefs = new Map(); + const inferredTypesPath = resolutionHostCacheHost.toPath( + ts.getAutomaticTypeDirectiveContainingFile( + actualProgram.getCompilerOptions(), + currentDirectory, + ), + ); actual.resolvedModuleNames.forEach((resolutions, path) => collectResolutionToRefFromCache( "Modules", @@ -222,6 +229,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 +240,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 +260,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 +305,10 @@ export function verifyResolutionCache( `Expected from:: ${JSON.stringify(info, undefined, " ")}` + `Actual from: ${resolution.files?.size}`, ); + ts.Debug.assert( + !resolution.isInvalidated, + `${projectName}:: Resolution should not be invalidated`, + ); verifySet(resolutionToExpected.get(resolution)!.files, resolution.files, `${projectName}:: Resolution files`); }); verifyMapOfResolutionSet(expected.resolvedFileToResolution, actual.resolvedFileToResolution, `resolvedFileToResolution`); @@ -295,24 +344,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( + cacheType: string, + cache: ts.ModeAwareCache | 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 in cache or watched ${fileName}`, + ); + } + } + function collectResolutionToRefFromCache( cacheType: string, fileName: ts.Path, cache: ts.ModeAwareCache | undefined, getResolvedFileName: (resolution: T) => string | undefined, deferWatchingNonRelativeResolution: boolean, - storeExpcted: Map>, + storeExpected: Map>, + 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 | 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 index 2e6dbc2b8c9..507be8e1823 100644 --- a/src/testRunner/unittests/helpers/tscWatch.ts +++ b/src/testRunner/unittests/helpers/tscWatch.ts @@ -133,6 +133,7 @@ export interface RunWatchBaseline extends BaselineB getPrograms: () => readonly CommandLineProgram[]; watchOrSolution: WatchOrSolution; useSourceOfProjectReferenceRedirect?: () => boolean; + userResolvedModuleNames?: true; } export function runWatchBaseline({ scenario, @@ -146,6 +147,7 @@ export function runWatchBaseline): void { baseline.push(`${sys.getExecutingFilePath()} ${commandLineArgs.join(" ")}`); let programs = watchBaseline({ @@ -171,6 +173,7 @@ export function runWatchBaseline | undefined)?.getResolutionCache?.(), useSourceOfProjectReferenceRedirect, + userResolvedModuleNames, }); } } @@ -183,6 +186,7 @@ export interface WatchBaseline extends BaselineBase, TscWatchCheckOptions { caption?: string; resolutionCache?: ts.ResolutionCache; useSourceOfProjectReferenceRedirect?: () => boolean; + userResolvedModuleNames?: true; } export function watchBaseline({ baseline, @@ -194,6 +198,7 @@ export function watchBaseline({ caption, resolutionCache, useSourceOfProjectReferenceRedirect, + userResolvedModuleNames, }: WatchBaseline): readonly CommandLineProgram[] { const programs = baselineAfterTscCompile( sys, @@ -213,6 +218,7 @@ export function watchBaseline({ programs[0][0], resolutionCache, useSourceOfProjectReferenceRedirect, + userResolvedModuleNames, ); } return programs; @@ -222,7 +228,8 @@ function verifyProgramStructureAndResolutionCache( sys: TscWatchSystem, program: ts.Program, resolutionCache: ts.ResolutionCache, - useSourceOfProjectReferenceRedirect?: () => boolean, + useSourceOfProjectReferenceRedirect: (() => boolean) | undefined, + userResolvedModuleNames: true | undefined, ) { const options = program.getCompilerOptions(); const compilerHost = ts.createCompilerHostWorker(options, /*setParentNodes*/ undefined, sys); @@ -239,25 +246,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, + preferNonRecursiveWatch: sys.preferNonRecursiveWatch, - getCompilerHost: () => compilerHost, - toPath: fileName => sys.toPath(fileName), - getCompilationSettings: () => options, - fileIsOpen: ts.returnFalse, - getCurrentProgram: () => program, - preferNonRecursiveWatch: sys.preferNonRecursiveWatch, - - 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 index e0f877db4f6..31f907c6df0 100644 --- a/src/testRunner/unittests/tscWatch/watchApi.ts +++ b/src/testRunner/unittests/tscWatch/watchApi.ts @@ -118,6 +118,7 @@ describe("unittests:: tscWatch:: watchAPI:: tsc-watch with custom module resolut }, ], watchOrSolution: watch, + userResolvedModuleNames: true, }); }); }