From 4c22bf786ea01f9ad382e5d8e915326674a6a173 Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 18 May 2018 16:42:42 -0700 Subject: [PATCH] getEditsForFileRename: Do fresh module resolution instead of relying on cache (#24211) * getEditsForFileRename: Do fresh module resolution instead of relying on cache * Add host.resolveModuleNameWithFailedLookupLocations method * Make host.resolveModuleNameWithFailedLookupLocations mandatory, and implement for Project * Add test, and no need to check host.fileExists * Change method name and always use cache * Update name in string --- src/compiler/resolutionCache.ts | 13 ++++++-- .../unittests/tsserverProjectSystem.ts | 31 +++++++++++++++++++ src/server/project.ts | 4 +++ src/services/getEditsForFileRename.ts | 8 +++-- src/services/types.ts | 3 ++ .../reference/api/tsserverlibrary.d.ts | 2 ++ tests/baselines/reference/api/typescript.d.ts | 1 + 7 files changed, 56 insertions(+), 6 deletions(-) diff --git a/src/compiler/resolutionCache.ts b/src/compiler/resolutionCache.ts index 9e0f3712ab9..dea5463b2ff 100644 --- a/src/compiler/resolutionCache.ts +++ b/src/compiler/resolutionCache.ts @@ -6,6 +6,7 @@ namespace ts { finishRecordingFilesWithChangedResolutions(): Path[]; resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined): ResolvedModuleFull[]; + getResolvedModuleWithFailedLookupLocationsFromCache(moduleName: string, containingFile: string): ResolvedModuleWithFailedLookupLocations; resolveTypeReferenceDirectives(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; invalidateResolutionOfFile(filePath: Path): void; @@ -73,7 +74,7 @@ namespace ts { export const maxNumberOfFilesToIterateForInvalidation = 256; type GetResolutionWithResolvedFileName = - (resolution: T) => R; + (resolution: T) => R | undefined; export function createResolutionCache(resolutionHost: ResolutionCacheHost, rootDirForResolution: string, logChangesWhenResolvingModule: boolean): ResolutionCache { let filesWithChangedSetOfUnresolvedImports: Path[] | undefined; @@ -124,6 +125,7 @@ namespace ts { startCachingPerDirectoryResolution: clearPerDirectoryResolutions, finishCachingPerDirectoryResolution, resolveModuleNames, + getResolvedModuleWithFailedLookupLocationsFromCache, resolveTypeReferenceDirectives, removeResolutionsOfFile, invalidateResolutionOfFile, @@ -320,7 +322,7 @@ namespace ts { } function resolveTypeReferenceDirectives(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[] { - return resolveNamesWithLocalCache( + return resolveNamesWithLocalCache( typeDirectiveNames, containingFile, resolvedTypeReferenceDirectives, perDirectoryResolvedTypeReferenceDirectives, resolveTypeReferenceDirective, getResolvedTypeReferenceDirective, @@ -329,7 +331,7 @@ namespace ts { } function resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames: string[] | undefined): ResolvedModuleFull[] { - return resolveNamesWithLocalCache( + return resolveNamesWithLocalCache( moduleNames, containingFile, resolvedModuleNames, perDirectoryResolvedModuleNames, resolveModuleName, getResolvedModule, @@ -337,6 +339,11 @@ namespace ts { ); } + function getResolvedModuleWithFailedLookupLocationsFromCache(moduleName: string, containingFile: string): ResolvedModuleWithFailedLookupLocations | undefined { + const cache = resolvedModuleNames.get(resolutionHost.toPath(containingFile)); + return cache && cache.get(moduleName); + } + function isNodeModulesDirectory(dirPath: Path) { return endsWith(dirPath, "/node_modules"); } diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 8524d46e094..7a5f97b06d5 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -8383,4 +8383,35 @@ new C();` verifyCompletionListWithNewFileInSubFolder(TestFSWithWatch.Tsc_WatchDirectory.DynamicPolling); }); }); + + describe("tsserverProjectSystem getEditsForFileRename", () => { + it("works for host implementing 'resolveModuleNames' and 'getResolvedModuleWithFailedLookupLocationsFromCache'", () => { + const userTs: File = { + path: "/user.ts", + content: 'import { x } from "./old";', + }; + + const host = createServerHost([userTs]); + const projectService = createProjectService(host); + projectService.openClientFile(userTs.path); + const project = first(projectService.inferredProjects); + + Debug.assert(!!project.resolveModuleNames); + + const edits = project.getLanguageService().getEditsForFileRename("/old.ts", "/new.ts", testFormatOptions); + assert.deepEqual>(edits, [{ + fileName: "/user.ts", + textChanges: [{ + span: textSpanFromSubstring(userTs.content, "./old"), + newText: "./new", + }], + }]); + }); + }); + + function textSpanFromSubstring(str: string, substring: string): TextSpan { + const start = str.indexOf(substring); + Debug.assert(start !== -1); + return createTextSpan(start, substring.length); + } } diff --git a/src/server/project.ts b/src/server/project.ts index 5b1dbddfc20..754d061235d 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -358,6 +358,10 @@ namespace ts.server { return this.resolutionCache.resolveModuleNames(moduleNames, containingFile, reusedNames); } + getResolvedModuleWithFailedLookupLocationsFromCache(moduleName: string, containingFile: string): ResolvedModuleWithFailedLookupLocations { + return this.resolutionCache.getResolvedModuleWithFailedLookupLocationsFromCache(moduleName, containingFile); + } + resolveTypeReferenceDirectives(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[] { return this.resolutionCache.resolveTypeReferenceDirectives(typeDirectiveNames, containingFile); } diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts index f8bcf02d036..d05d22a98d9 100644 --- a/src/services/getEditsForFileRename.ts +++ b/src/services/getEditsForFileRename.ts @@ -4,7 +4,7 @@ namespace ts { const pathUpdater = getPathUpdater(oldFilePath, newFilePath, host); return textChanges.ChangeTracker.with({ host, formatContext }, changeTracker => { updateTsconfigFiles(program, changeTracker, oldFilePath, newFilePath); - for (const { sourceFile, toUpdate } of getImportsToUpdate(program, oldFilePath)) { + for (const { sourceFile, toUpdate } of getImportsToUpdate(program, oldFilePath, host)) { const newPath = pathUpdater(isRef(toUpdate) ? toUpdate.fileName : toUpdate.text); if (newPath !== undefined) { const range = isRef(toUpdate) ? toUpdate : createStringRange(toUpdate, sourceFile); @@ -30,7 +30,7 @@ namespace ts { return "fileName" in toUpdate; } - function getImportsToUpdate(program: Program, oldFilePath: string): ReadonlyArray { + function getImportsToUpdate(program: Program, oldFilePath: string, host: LanguageServiceHost): ReadonlyArray { const checker = program.getTypeChecker(); const result: ToUpdate[] = []; for (const sourceFile of program.getSourceFiles()) { @@ -44,7 +44,9 @@ namespace ts { // If it resolved to something already, ignore. if (checker.getSymbolAtLocation(importStringLiteral)) continue; - const resolved = program.getResolvedModuleWithFailedLookupLocationsFromCache(importStringLiteral.text, sourceFile.fileName); + const resolved = host.resolveModuleNames + ? host.getResolvedModuleWithFailedLookupLocationsFromCache && host.getResolvedModuleWithFailedLookupLocationsFromCache(importStringLiteral.text, sourceFile.fileName) + : program.getResolvedModuleWithFailedLookupLocationsFromCache(importStringLiteral.text, sourceFile.fileName); if (resolved && contains(resolved.failedLookupLocations, oldFilePath)) { result.push({ sourceFile, toUpdate: importStringLiteral }); } diff --git a/src/services/types.ts b/src/services/types.ts index 0e9dbccbd1b..b8e2c488b17 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -207,8 +207,11 @@ namespace ts { * LS host can optionally implement this method if it wants to be completely in charge of module name resolution. * if implementation is omitted then language service will use built-in module resolution logic and get answers to * host specific questions using 'getScriptSnapshot'. + * + * If this is implemented, `getResolvedModuleWithFailedLookupLocationsFromCache` should be too. */ resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; + getResolvedModuleWithFailedLookupLocationsFromCache?(modulename: string, containingFile: string): ResolvedModuleWithFailedLookupLocations; resolveTypeReferenceDirectives?(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; /* @internal */ hasInvalidatedResolution?: HasInvalidatedResolution; /* @internal */ hasChangedAutomaticTypeDirectiveNames?: boolean; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index ae163fb4f8d..cbf39a2a48f 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4477,6 +4477,7 @@ declare namespace ts { fileExists?(path: string): boolean; getTypeRootsVersion?(): number; resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; + getResolvedModuleWithFailedLookupLocationsFromCache?(modulename: string, containingFile: string): ResolvedModuleWithFailedLookupLocations; resolveTypeReferenceDirectives?(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; getDirectories?(directoryName: string): string[]; /** @@ -7864,6 +7865,7 @@ declare namespace ts.server { readFile(fileName: string): string | undefined; fileExists(file: string): boolean; resolveModuleNames(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModuleFull[]; + getResolvedModuleWithFailedLookupLocationsFromCache(moduleName: string, containingFile: string): ResolvedModuleWithFailedLookupLocations; resolveTypeReferenceDirectives(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; directoryExists(path: string): boolean; getDirectories(path: string): string[]; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 89552e727c2..b449fc853c6 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4477,6 +4477,7 @@ declare namespace ts { fileExists?(path: string): boolean; getTypeRootsVersion?(): number; resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; + getResolvedModuleWithFailedLookupLocationsFromCache?(modulename: string, containingFile: string): ResolvedModuleWithFailedLookupLocations; resolveTypeReferenceDirectives?(typeDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; getDirectories?(directoryName: string): string[]; /**