From b9bc8f0e7ffd0dc4cafd9d4fc6c86d832250d0ba Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 14 Jun 2018 10:17:04 -0700 Subject: [PATCH 1/2] getEditsForFileRename: Don't update unrelated import --- src/services/getEditsForFileRename.ts | 23 ++++++++++++++----- .../cases/fourslash/getEditsForFileRename.ts | 4 ++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts index a7e41d2014f31..5898eb24720a3 100644 --- a/src/services/getEditsForFileRename.ts +++ b/src/services/getEditsForFileRename.ts @@ -104,6 +104,7 @@ namespace ts { ): void { for (const sourceFile of program.getSourceFiles()) { const newImportFromPath = oldToNew(sourceFile.fileName) || sourceFile.fileName; + const importingSourceFileMoved = newImportFromPath !== sourceFile.fileName; const newImportFromDirectory = getDirectoryPath(newImportFromPath); const oldFromNew: string | undefined = newToOld(sourceFile.fileName); @@ -123,7 +124,10 @@ namespace ts { // TODO:GH#18217 ? getSourceFileToImportFromResolved(resolveModuleName(importLiteral.text, oldImportFromPath, program.getCompilerOptions(), host as ModuleResolutionHost), oldToNew, program) : getSourceFileToImport(importLiteral, sourceFile, program, host, oldToNew); - return toImport === undefined ? undefined : moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport, host, preferences); + // If neither the importing source file nor the imported file moved, do nothing. + return toImport === undefined || !toImport.updated && !importingSourceFileMoved + ? undefined + : moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport.newFileName, host, preferences); }); } } @@ -135,12 +139,18 @@ namespace ts { return ensurePathIsNonModuleName(combineNormal(pathA, pathB)); } - function getSourceFileToImport(importLiteral: StringLiteralLike, importingSourceFile: SourceFile, program: Program, host: LanguageServiceHost, oldToNew: PathUpdater): string | undefined { + interface ToImport { + readonly newFileName: string; + /** True if the imported file was renamed. */ + readonly updated: boolean; + } + function getSourceFileToImport(importLiteral: StringLiteralLike, importingSourceFile: SourceFile, program: Program, host: LanguageServiceHost, oldToNew: PathUpdater): ToImport | undefined { const symbol = program.getTypeChecker().getSymbolAtLocation(importLiteral); if (symbol) { if (symbol.declarations.some(d => isAmbientModule(d))) return undefined; // No need to update if it's an ambient module const oldFileName = find(symbol.declarations, isSourceFile)!.fileName; - return oldToNew(oldFileName) || oldFileName; + const newFileName = oldToNew(oldFileName); + return newFileName === undefined ? { newFileName: oldFileName, updated: false } : { newFileName, updated: true }; } else { const resolved = host.resolveModuleNames @@ -150,14 +160,15 @@ namespace ts { } } - function getSourceFileToImportFromResolved(resolved: ResolvedModuleWithFailedLookupLocations | undefined, oldToNew: PathUpdater, program: Program): string | undefined { + function getSourceFileToImportFromResolved(resolved: ResolvedModuleWithFailedLookupLocations | undefined, oldToNew: PathUpdater, program: Program): ToImport | undefined { return resolved && ( (resolved.resolvedModule && getIfInProgram(resolved.resolvedModule.resolvedFileName)) || firstDefined(resolved.failedLookupLocations, getIfInProgram)); - function getIfInProgram(oldLocation: string): string | undefined { + function getIfInProgram(oldLocation: string): ToImport | undefined { const newLocation = oldToNew(oldLocation); + return program.getSourceFile(oldLocation) || newLocation !== undefined && program.getSourceFile(newLocation) - ? newLocation || oldLocation + ? newLocation !== undefined ? { newFileName: newLocation, updated: true } : { newFileName: oldLocation, updated: false } : undefined; } } diff --git a/tests/cases/fourslash/getEditsForFileRename.ts b/tests/cases/fourslash/getEditsForFileRename.ts index 03aab26097841..6cd7f9bb046bf 100644 --- a/tests/cases/fourslash/getEditsForFileRename.ts +++ b/tests/cases/fourslash/getEditsForFileRename.ts @@ -14,6 +14,10 @@ /////// ////import old from "../old"; +// @Filename: /unrelated.ts +// Don't update an unrelated import +////import { x } from "././src/./foo/./a"; + // @Filename: /src/new.ts //// From 5e456d980b54e55ee8167e666ddb9601aa604837 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 14 Jun 2018 15:05:25 -0700 Subject: [PATCH 2/2] Importing source file moved if it's the old path *or* new path --- src/services/getEditsForFileRename.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts index 5898eb24720a3..8960950bf38cd 100644 --- a/src/services/getEditsForFileRename.ts +++ b/src/services/getEditsForFileRename.ts @@ -103,14 +103,16 @@ namespace ts { preferences: UserPreferences, ): void { for (const sourceFile of program.getSourceFiles()) { - const newImportFromPath = oldToNew(sourceFile.fileName) || sourceFile.fileName; - const importingSourceFileMoved = newImportFromPath !== sourceFile.fileName; + const newFromOld = oldToNew(sourceFile.fileName); + const newImportFromPath = newFromOld !== undefined ? newFromOld : sourceFile.fileName; const newImportFromDirectory = getDirectoryPath(newImportFromPath); const oldFromNew: string | undefined = newToOld(sourceFile.fileName); const oldImportFromPath: string = oldFromNew || sourceFile.fileName; const oldImportFromDirectory = getDirectoryPath(oldImportFromPath); + const importingSourceFileMoved = newFromOld !== undefined || oldFromNew !== undefined; + updateImportsWorker(sourceFile, changeTracker, referenceText => { if (!pathIsRelative(referenceText)) return undefined;