Merge pull request microsoft#25249 from Microsoft/openSameFileWithDifferentText

sheetalkamat · web-flow · commit aa925682d0ac · 2018-07-02T12:50:57.000-07:00
Retain the version information of script infos when they are deleted
diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts
@@ -615,6 +615,13 @@ interface Array<T> {}`
             }
         }
 
+        removeFile(filePath: string) {
+            const path = this.toFullPath(filePath);
+            const currentEntry = this.fs.get(path) as FsFile;
+            Debug.assert(isFsFile(currentEntry));
+            this.removeFileOrFolder(currentEntry, returnFalse);
+        }
+
         removeFolder(folderPath: string, recursive?: boolean) {
             const path = this.toFullPath(folderPath);
             const currentEntry = this.fs.get(path) as FsFolder;
diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts
@@ -346,6 +346,12 @@ namespace ts.server {
          * Container of all known scripts
          */
         private readonly filenameToScriptInfo = createMap<ScriptInfo>();
+        /**
+         * Contains all the deleted script info's version information so that
+         * it does not reset when creating script info again
+         * (and could have potentially collided with version where contents mismatch)
+         */
+        private readonly filenameToScriptInfoVersion = createMap<ScriptInfoVersion>();
         // Set of all '.js' files ever opened.
         private readonly allJsFilesForOpenFileTelemetry = createMap<true>();
 
@@ -1022,6 +1028,7 @@ namespace ts.server {
 
         private deleteScriptInfo(info: ScriptInfo) {
             this.filenameToScriptInfo.delete(info.path);
+            this.filenameToScriptInfoVersion.set(info.path, info.getVersion());
             const realpath = info.getRealpathIfDifferent();
             if (realpath) {
                 this.realpathToScriptInfos!.remove(realpath, info); // TODO: GH#18217
@@ -1860,8 +1867,9 @@ namespace ts.server {
                 if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) {
                     return;
                 }
-                info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path); // TODO: GH#18217
+                info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path, this.filenameToScriptInfoVersion.get(path)); // TODO: GH#18217
                 this.filenameToScriptInfo.set(info.path, info);
+                this.filenameToScriptInfoVersion.delete(info.path);
                 if (!openedByClient) {
                     this.watchClosedScriptInfo(info);
                 }
diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts
@@ -1,13 +1,20 @@
 namespace ts.server {
+    /*@internal*/
+    export interface ScriptInfoVersion {
+        svc: number;
+        text: number;
+    }
 
     /* @internal */
     export class TextStorage {
+        /*@internal*/
+        version: ScriptInfoVersion;
+
         /**
          * Generated only on demand (based on edits, or information requested)
          * The property text is set to undefined when edits happen on the cache
          */
         private svc: ScriptVersionCache | undefined;
-        private svcVersion = 0;
 
         /**
          * Stores the text when there are no changes to the script version cache
@@ -19,7 +26,6 @@ namespace ts.server {
          * Line map for the text when there is no script version cache present
          */
         private lineMap: number[] | undefined;
-        private textVersion = 0;
 
         /**
          * True if the text is for the file thats open in the editor
@@ -34,13 +40,14 @@ namespace ts.server {
          */
         private pendingReloadFromDisk: boolean;
 
-        constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath) {
+        constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath, initialVersion?: ScriptInfoVersion) {
+            this.version = initialVersion || { svc: 0, text: 0 };
         }
 
         public getVersion() {
             return this.svc
-                ? `SVC-${this.svcVersion}-${this.svc.getSnapshotVersion()}`
-                : `Text-${this.textVersion}`;
+                ? `SVC-${this.version.svc}-${this.svc.getSnapshotVersion()}`
+                : `Text-${this.version.text}`;
         }
 
         public hasScriptVersionCache_TestOnly() {
@@ -55,7 +62,7 @@ namespace ts.server {
             this.svc = undefined;
             this.text = newText;
             this.lineMap = undefined;
-            this.textVersion++;
+            this.version.text++;
         }
 
         public edit(start: number, end: number, newText: string) {
@@ -163,7 +170,7 @@ namespace ts.server {
         private switchToScriptVersionCache(): ScriptVersionCache {
             if (!this.svc || this.pendingReloadFromDisk) {
                 this.svc = ScriptVersionCache.fromString(this.getOrLoadText());
-                this.svcVersion++;
+                this.version.svc++;
             }
             return this.svc;
         }
@@ -235,10 +242,11 @@ namespace ts.server {
             readonly fileName: NormalizedPath,
             readonly scriptKind: ScriptKind,
             public readonly hasMixedContent: boolean,
-            readonly path: Path) {
+            readonly path: Path,
+            initialVersion?: ScriptInfoVersion) {
             this.isDynamic = isDynamicFileName(fileName);
 
-            this.textStorage = new TextStorage(host, fileName);
+            this.textStorage = new TextStorage(host, fileName, initialVersion);
             if (hasMixedContent || this.isDynamic) {
                 this.textStorage.reload("");
                 this.realpath = this.path;
@@ -248,6 +256,11 @@ namespace ts.server {
                 : getScriptKindFromFileName(fileName);
         }
 
+        /*@internal*/
+        getVersion() {
+            return this.textStorage.version;
+        }
+
         /*@internal*/
         public isDynamicOrHasMixedContent() {
             return this.hasMixedContent || this.isDynamic;
diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts
@@ -8848,6 +8848,103 @@ export const x = 10;`
         });
     });
 
+    describe("tsserverProjectSystem syntax operations", () => {
+        function navBarFull(session: TestSession, file: File) {
+            return JSON.stringify(session.executeCommandSeq<protocol.FileRequest>({
+                command: protocol.CommandTypes.NavBarFull,
+                arguments: { file: file.path }
+            }).response);
+        }
+
+        function openFile(session: TestSession, file: File) {
+            session.executeCommandSeq<protocol.OpenRequest>({
+                command: protocol.CommandTypes.Open,
+                arguments: { file: file.path, fileContent: file.content }
+            });
+        }
+
+        it("works when file is removed and added with different content", () => {
+            const projectRoot = "/user/username/projects/myproject";
+            const app: File = {
+                path: `${projectRoot}/app.ts`,
+                content: "console.log('Hello world');"
+            };
+            const unitTest1: File = {
+                path: `${projectRoot}/unitTest1.ts`,
+                content: `import assert = require('assert');
+
+describe("Test Suite 1", () => {
+    it("Test A", () => {
+        assert.ok(true, "This shouldn't fail");
+    });
+
+    it("Test B", () => {
+        assert.ok(1 === 1, "This shouldn't fail");
+        assert.ok(false, "This should fail");
+    });
+});`
+            };
+            const tsconfig: File = {
+                path: `${projectRoot}/tsconfig.json`,
+                content: "{}"
+            };
+            const files = [app, libFile, tsconfig];
+            const host = createServerHost(files);
+            const session = createSession(host);
+            const service = session.getProjectService();
+            openFile(session, app);
+
+            checkNumberOfProjects(service, { configuredProjects: 1 });
+            const project = service.configuredProjects.get(tsconfig.path)!;
+            const expectedFilesWithoutUnitTest1 = files.map(f => f.path);
+            checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
+
+            host.writeFile(unitTest1.path, unitTest1.content);
+            host.runQueuedTimeoutCallbacks();
+            const expectedFilesWithUnitTest1 = expectedFilesWithoutUnitTest1.concat(unitTest1.path);
+            checkProjectActualFiles(project, expectedFilesWithUnitTest1);
+
+            openFile(session, unitTest1);
+            checkProjectActualFiles(project, expectedFilesWithUnitTest1);
+
+            const navBarResultUnitTest1 = navBarFull(session, unitTest1);
+            host.removeFile(unitTest1.path);
+            host.checkTimeoutQueueLengthAndRun(2);
+            checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
+
+            session.executeCommandSeq<protocol.CloseRequest>({
+                command: protocol.CommandTypes.Close,
+                arguments: { file: unitTest1.path }
+            });
+            checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
+
+            const unitTest1WithChangedContent: File = {
+                path: unitTest1.path,
+                content: `import assert = require('assert');
+
+export function Test1() {
+    assert.ok(true, "This shouldn't fail");
+};
+
+export function Test2() {
+    assert.ok(1 === 1, "This shouldn't fail");
+    assert.ok(false, "This should fail");
+};`
+            };
+            host.writeFile(unitTest1.path, unitTest1WithChangedContent.content);
+            host.runQueuedTimeoutCallbacks();
+            checkProjectActualFiles(project, expectedFilesWithUnitTest1);
+
+            openFile(session, unitTest1WithChangedContent);
+            checkProjectActualFiles(project, expectedFilesWithUnitTest1);
+            const sourceFile = project.getLanguageService().getNonBoundSourceFile(unitTest1WithChangedContent.path);
+            assert.strictEqual(sourceFile.text, unitTest1WithChangedContent.content);
+
+            const navBarResultUnitTest1WithChangedContent = navBarFull(session, unitTest1WithChangedContent);
+            assert.notStrictEqual(navBarResultUnitTest1WithChangedContent, navBarResultUnitTest1, "With changes in contents of unitTest file, we should see changed naviagation bar item result");
+        });
+    });
+
     function makeSampleProjects() {
         const aTs: File = {
             path: "/a/a.ts",
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -13342,18 +13342,21 @@ declare namespace ts.server.protocol {
     }
 }
 declare namespace ts.server {
+    interface ScriptInfoVersion {
+        svc: number;
+        text: number;
+    }
     class TextStorage {
         private readonly host;
         private readonly fileName;
+        version: ScriptInfoVersion;
         private svc;
-        private svcVersion;
         private text;
         private lineMap;
-        private textVersion;
         isOpen: boolean;
         private ownFileText;
         private pendingReloadFromDisk;
-        constructor(host: ServerHost, fileName: NormalizedPath);
+        constructor(host: ServerHost, fileName: NormalizedPath, initialVersion?: ScriptInfoVersion);
         getVersion(): string;
         hasScriptVersionCache_TestOnly(): boolean;
         useScriptVersionCache_TestOnly(): void;
@@ -13393,7 +13396,8 @@ declare namespace ts.server {
         readonly isDynamic: boolean;
         private realpath;
         cacheSourceFile: DocumentRegistrySourceFileCache;
-        constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path);
+        constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: ScriptInfoVersion);
+        getVersion(): ScriptInfoVersion;
         isDynamicOrHasMixedContent(): boolean;
         isScriptOpen(): boolean;
         open(newText: string): void;
@@ -13803,6 +13807,7 @@ declare namespace ts.server {
         readonly typingsCache: TypingsCache;
         readonly documentRegistry: DocumentRegistry;
         private readonly filenameToScriptInfo;
+        private readonly filenameToScriptInfoVersion;
         private readonly allJsFilesForOpenFileTelemetry;
         readonly realpathToScriptInfos: MultiMap<ScriptInfo> | undefined;
         private readonly externalProjectToConfiguredProjectMap;

Original file line number	Diff line number	Diff line change
@@ -615,6 +615,13 @@ interface Array<T> {}`
`615`	`615`	`}`
`616`	`616`	`}`
`617`	`617`
	`618`	`+ removeFile(filePath: string) {`
	`619`	`+ const path = this.toFullPath(filePath);`
	`620`	`+ const currentEntry = this.fs.get(path) as FsFile;`
	`621`	`+ Debug.assert(isFsFile(currentEntry));`
	`622`	`+ this.removeFileOrFolder(currentEntry, returnFalse);`
	`623`	`+ }`
	`624`	`+`
`618`	`625`	`removeFolder(folderPath: string, recursive?: boolean) {`
`619`	`626`	`const path = this.toFullPath(folderPath);`
`620`	`627`	`const currentEntry = this.fs.get(path) as FsFolder;`