fix: #271 - Fix SourceFile.move not deleting previous file on file sy…

…stem.
dsherret · Mar 8, 2018 · a4dfda9 · a4dfda9
1 parent 38a753e
commit a4dfda9
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 20 deletions.
diff --git a/src/Project.ts b/src/Project.ts
@@ -325,8 +325,12 @@ export class Project {
     getSourceFile(fileNameOrSearchFunction: string | ((file: SourceFile) => boolean)): SourceFile | undefined {
         let searchFunction = fileNameOrSearchFunction as ((file: SourceFile) => boolean);
 
-        if (typeof fileNameOrSearchFunction === "string")
-            searchFunction = def => FileUtils.pathEndsWith(def.getFilePath(), fileNameOrSearchFunction);
+        if (typeof fileNameOrSearchFunction === "string") {
+            if (FileUtils.pathIsAbsolute(fileNameOrSearchFunction))
+                return this.global.compilerFactory.getSourceFileFromCacheFromFilePath(fileNameOrSearchFunction);
+            else
+                searchFunction = def => FileUtils.pathEndsWith(def.getFilePath(), fileNameOrSearchFunction);
+        }
 
         return ArrayUtils.find(this.global.compilerFactory.getSourceFilesByDirectoryDepth(), searchFunction);
     }
@@ -359,7 +363,7 @@ export class Project {
             const matchedPaths = matchGlobs(sourceFilePaths, globPatterns!, fileSystemWrapper.getCurrentDirectory());
 
             for (const matchedPath of matchedPaths)
-                yield compilerFactory.getSourceFileFromFilePath(matchedPath)!;
+                yield compilerFactory.getSourceFileFromCacheFromFilePath(matchedPath)!;
 
             function* getSourceFilePaths() {
                 for (const sourceFile of sourceFiles)

diff --git a/src/compiler/file/SourceFile.ts b/src/compiler/file/SourceFile.ts
@@ -204,9 +204,10 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
      */
     move(filePath: string, options: SourceFileMoveOptions = {}): SourceFile {
         const {overwrite = false} = options;
+        const oldFilePath = this.getFilePath();
         filePath = this.global.fileSystemWrapper.getStandardizedAbsolutePath(filePath, this.getDirectoryPath());
 
-        if (filePath === this.getFilePath())
+        if (filePath === oldFilePath)
             return this;
 
         // todo: first check to see if this has any exports or imports (add isExternalModule()?)
@@ -226,6 +227,7 @@ export class SourceFile extends SourceFileBase<ts.SourceFile> {
             newFilePath: filePath,
             sourceFile: this
         });
+        this.global.fileSystemWrapper.queueDelete(oldFilePath);
 
         // update the import & export declarations in this file
         for (const item of fileImportAndExportsWithSourceFiles)

diff --git a/src/compiler/tools/LanguageService.ts b/src/compiler/tools/LanguageService.ts
@@ -53,7 +53,7 @@ export class LanguageService {
             useCaseSensitiveFileNames: () => true,
             readFile: (path, encoding) => {
                 if (this.global.compilerFactory.containsSourceFileAtPath(path))
-                    return this.global.compilerFactory.getSourceFileFromFilePath(path)!.getFullText();
+                    return this.global.compilerFactory.getSourceFileFromCacheFromFilePath(path)!.getFullText();
                 return this.global.fileSystemWrapper.readFileSync(path, encoding);
             },
             fileExists: fileExistsSync,
@@ -235,7 +235,7 @@ export class LanguageService {
      * @param settings - Format code settings.
      */
     getFormattedDocumentText(filePath: string, settings: FormatCodeSettings) {
-        const sourceFile = this.global.compilerFactory.getSourceFileFromFilePath(filePath);
+        const sourceFile = this.global.compilerFactory.getSourceFileFromCacheFromFilePath(filePath);
         if (sourceFile == null)
             throw new errors.FileNotFoundError(filePath);
 

diff --git a/src/compiler/tools/results/DefinitionInfo.ts b/src/compiler/tools/results/DefinitionInfo.ts
@@ -12,13 +12,16 @@ export class DefinitionInfo<TCompilerObject extends ts.DefinitionInfo = ts.Defin
     protected readonly global: GlobalContainer;
     /** @internal */
     private readonly _compilerObject: TCompilerObject;
+    /** @internal */
+    private readonly sourceFile: SourceFile;
 
     /**
      * @internal
      */
     constructor(global: GlobalContainer, compilerObject: TCompilerObject) {
         this.global = global;
         this._compilerObject = compilerObject;
+        this.sourceFile = this.global.compilerFactory.getSourceFileFromCacheFromFilePath(this.compilerObject.fileName)!;
     }
 
     /**
@@ -32,7 +35,7 @@ export class DefinitionInfo<TCompilerObject extends ts.DefinitionInfo = ts.Defin
      * Gets the source file this reference is in.
      */
     getSourceFile(): SourceFile {
-        return this.global.compilerFactory.getSourceFileFromFilePath(this.compilerObject.fileName)!;
+        return this.sourceFile;
     }
 
     /**

diff --git a/src/compiler/tools/results/DocumentSpan.ts b/src/compiler/tools/results/DocumentSpan.ts
@@ -14,6 +14,8 @@ export class DocumentSpan<TCompilerObject extends ts.DocumentSpan = ts.DocumentS
     private readonly _compilerObject: TCompilerObject;
     /** @internal */
     private readonly node: Node;
+    /** @internal */
+    private readonly sourceFile: SourceFile;
 
     /**
      * @internal
@@ -23,6 +25,7 @@ export class DocumentSpan<TCompilerObject extends ts.DocumentSpan = ts.DocumentS
         this._compilerObject = compilerObject;
 
         // store this node so that it's start doesn't go out of date because of manipulation (though the text span may)
+        this.sourceFile = this.global.compilerFactory.getSourceFileFromCacheFromFilePath(this.compilerObject.fileName)!;
         this.node = this.getSourceFile().getDescendantAtStartWithWidth(this.getTextSpan().getStart(), this.getTextSpan().getLength())!;
     }
 
@@ -37,7 +40,7 @@ export class DocumentSpan<TCompilerObject extends ts.DocumentSpan = ts.DocumentS
      * Gets the source file this reference is in.
      */
     getSourceFile(): SourceFile {
-        return this.global.compilerFactory.getSourceFileFromFilePath(this.compilerObject.fileName)!;
+        return this.sourceFile;
     }
 
     /**

diff --git a/src/factories/CompilerFactory.ts b/src/factories/CompilerFactory.ts
@@ -385,16 +385,17 @@ export class CompilerFactory {
 
         if (nodeToReplace.kind === SyntaxKind.SourceFile && (nodeToReplace as ts.SourceFile).fileName !== (newNode as ts.SourceFile).fileName) {
             const oldFilePath = (nodeToReplace as ts.SourceFile).fileName;
-            const sourceFile = this.sourceFileCacheByFilePath.get(oldFilePath)!;
+            const sourceFile = node! as SourceFile;
             this.removeCompilerNodeFromCache(nodeToReplace);
-            this.addSourceFileToCache(sourceFile);
+            sourceFile.replaceCompilerNodeFromFactory(newNode as ts.SourceFile);
             this.nodeCache.set(newNode, sourceFile);
+            this.addSourceFileToCache(sourceFile);
         }
-        else
+        else {
             this.nodeCache.replaceKey(nodeToReplace, newNode);
-
-        if (node != null)
-            node.replaceCompilerNodeFromFactory(newNode);
+            if (node != null)
+                node.replaceCompilerNodeFromFactory(newNode);
+        }
     }
 
     /**

diff --git a/src/fileSystem/Directory.ts b/src/fileSystem/Directory.ts
@@ -152,10 +152,7 @@ export class Directory {
     getSourceFile(pathOrCondition: string | ((sourceFile: SourceFile) => boolean)) {
         if (typeof pathOrCondition === "string") {
             const path = this.global.fileSystemWrapper.getStandardizedAbsolutePath(pathOrCondition, this.getPath());
-            if (this.global.compilerFactory.containsSourceFileAtPath(path))
-                return this.global.compilerFactory.getSourceFileFromFilePath(path);
-            else
-                return undefined;
+            return this.global.compilerFactory.getSourceFileFromCacheFromFilePath(path);
         }
 
         return ArrayUtils.find(this.getSourceFiles(), pathOrCondition);

diff --git a/src/tests/compiler/file/sourceFileTests.ts b/src/tests/compiler/file/sourceFileTests.ts
@@ -124,7 +124,9 @@ describe(nameof(SourceFile), () => {
         function doTest(filePath: string, newFilePath: string, absoluteNewFilePath?: string, overwrite?: boolean) {
             const fileText = "    interface Identifier {}    ";
             const {sourceFile, project} = getInfoFromText(fileText, { filePath });
+            const fileSystem = project.getFileSystem();
             const existingFile = project.createSourceFile("/existingFile.ts");
+            project.saveSync();
             const interfaceDec = sourceFile.getInterfaceOrThrow("Identifier");
             const newFile = sourceFile.move(newFilePath, { overwrite });
             const isRemovingExisting = overwrite && newFilePath === "/existingFile.ts";
@@ -133,8 +135,13 @@ describe(nameof(SourceFile), () => {
             expect(newFile).to.equal(sourceFile);
             expect(sourceFile.getFilePath()).to.equal(absoluteNewFilePath || newFilePath);
             expect(sourceFile.getFullText()).to.equal(fileText);
+            expect(project.getSourceFile(filePath)).to.be.undefined;
+            expect(project.getSourceFile(absoluteNewFilePath || newFilePath)).to.not.be.undefined;
             expect(interfaceDec.wasForgotten()).to.be.false;
             expect(project.getSourceFiles().length).to.equal(isRemovingExisting ? 1 : 2);
+            project.saveSync();
+            expect(fileSystem.fileExistsSync(absoluteNewFilePath || newFilePath)).to.be.true;
+            expect(fileSystem.fileExistsSync(filePath)).to.be.false;
         }
 
         it("should throw if the file already exists", () => {
@@ -150,7 +157,7 @@ describe(nameof(SourceFile), () => {
             doTest("/file.ts", "/newFile.ts", undefined, true);
         });
 
-        it("should copy to a relative file path", () => {
+        it("should move to a relative file path", () => {
             doTest("/dir/file.ts", "../subDir/existingFile.ts", "/subDir/existingFile.ts");
         });
 

diff --git a/src/tests/projectTests.ts b/src/tests/projectTests.ts
@@ -511,7 +511,14 @@ describe(nameof(Project), () => {
             expect(project.getSourceFile("file.ts")!.getFilePath()).to.equal(expectedFile.getFilePath());
         });
 
-        it("should get the first match based on the directory structure when swapping the order fo what was created first", () => {
+        it("should get when specifying an absolute path", () => {
+            const project = new Project({ useVirtualFileSystem: true });
+            project.createSourceFile("dir/file.ts");
+            const expectedFile = project.createSourceFile("file.ts");
+            expect(project.getSourceFile("/file.ts")!.getFilePath()).to.equal(expectedFile.getFilePath());
+        });
+
+        it("should get the first match based on the directory structure when swapping the order of what was created first", () => {
             const project = new Project({ useVirtualFileSystem: true });
             const expectedFile = project.createSourceFile("file.ts");
             project.createSourceFile("dir/file.ts");

diff --git a/src/utils/FileUtils.ts b/src/utils/FileUtils.ts
@@ -63,6 +63,14 @@ export class FileUtils {
         return FileUtils.standardizeSlashes(path.join(...paths));
     }
 
+    /**
+     * Gets if the path is absolute.
+     * @param fileOrDirPath - File or directory path.
+     */
+    static pathIsAbsolute(fileOrDirPath: string) {
+        return path.isAbsolute(fileOrDirPath);
+    }
+
     /**
      * Gets the standardized absolute path.
      * @param fileSystem - File system.