do not use ScriptVersionCache for closed files (microsoft#12777)

Jakefile.js

@@ -250,6 +250,7 @@ var harnessSources = harnessCoreSources.concat([
     "convertToBase64.ts",
     "transpile.ts",
     "reuseProgramStructure.ts",
+    "textStorage.ts",
     "cachingInServerLSHost.ts",
     "moduleResolution.ts",
     "tsconfigParsing.ts",

src/harness/unittests/textStorage.ts

@@ -0,0 +1,71 @@
+/// <reference path="../harness.ts" />
+/// <reference path="../../server/scriptVersionCache.ts"/>
+/// <reference path="./tsserverProjectSystem.ts" />
+
+namespace ts.textStorage {
+    describe("Text storage", () => {
+        const f = {
+            path: "/a/app.ts",
+            content: `
+                let x = 1;
+                let y = 2;
+                function bar(a: number) {
+                    return a + 1;
+                }`
+        };
+
+        it("text based storage should be have exactly the same as script version cache", () => {
+
+            debugger
+            const host = ts.projectSystem.createServerHost([f]);
+
+            const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path));
+            const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path));
+
+            ts1.useScriptVersionCache();
+            ts2.useText();
+
+            const lineMap = computeLineStarts(f.content);
+
+            for (let line = 0; line < lineMap.length; line++) {
+                const start = lineMap[line];
+                const end = line === lineMap.length - 1 ? f.path.length : lineMap[line + 1];
+
+                for (let offset = 0; offset < end - start; offset++) {
+                    const pos1 = ts1.lineOffsetToPosition(line + 1, offset + 1);
+                    const pos2 = ts2.lineOffsetToPosition(line + 1, offset + 1);
+                    assert.isTrue(pos1 === pos2, `lineOffsetToPosition ${line + 1}-${offset + 1}: expected ${pos1} to equal ${pos2}`);
+                }
+
+                const {start: start1, length: length1 } = ts1.lineToTextSpan(line);
+                const {start: start2, length: length2 } = ts2.lineToTextSpan(line);
+                assert.isTrue(start1 === start2, `lineToTextSpan ${line}::start:: expected ${start1} to equal ${start2}`);
+                assert.isTrue(length1 === length2, `lineToTextSpan ${line}::length:: expected ${length1} to equal ${length2}`);
+            }
+
+            for (let pos = 0; pos < f.content.length; pos++) {
+                const { line: line1, offset: offset1 } = ts1.positionToLineOffset(pos);
+                const { line: line2, offset: offset2 } = ts2.positionToLineOffset(pos);
+                assert.isTrue(line1 === line2, `positionToLineOffset ${pos}::line:: expected ${line1} to equal ${line2}`);
+                assert.isTrue(offset1 === offset2, `positionToLineOffset ${pos}::offset:: expected ${offset1} to equal ${offset2}`);
+            }
+        });
+
+        it("should switch to script version cache if necessary", () => {
+            const host = ts.projectSystem.createServerHost([f]);
+            const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path));
+
+            ts1.getSnapshot();
+            assert.isTrue(!ts1.hasScriptVersionCache(), "should not have script version cache - 1");
+
+            ts1.edit(0, 5, "   ");
+            assert.isTrue(ts1.hasScriptVersionCache(), "have script version cache - 1");
+
+            ts1.useText();
+            assert.isTrue(!ts1.hasScriptVersionCache(), "should not have script version cache - 2");
+
+            ts1.getLineInfo(0);
+            assert.isTrue(ts1.hasScriptVersionCache(), "have script version cache - 2");
+        })
+    });
+}

src/harness/unittests/tsserverProjectSystem.ts

@@ -1574,7 +1574,7 @@ namespace ts.projectSystem {
             const project = projectService.externalProjects[0];
 
             const scriptInfo = project.getScriptInfo(file1.path);
-            const snap = scriptInfo.snap();
+            const snap = scriptInfo.getSnapshot();
             const actualText = snap.getText(0, snap.getLength());
             assert.equal(actualText, "", `expected content to be empty string, got "${actualText}"`);
 
@@ -1587,7 +1587,7 @@ namespace ts.projectSystem {
             projectService.closeClientFile(file1.path);
 
             const scriptInfo2 = project.getScriptInfo(file1.path);
-            const snap2 = scriptInfo2.snap();
+            const snap2 = scriptInfo2.getSnapshot();
             const actualText2 = snap2.getText(0, snap.getLength());
             assert.equal(actualText2, "", `expected content to be empty string, got "${actualText2}"`);
         });
@@ -2285,13 +2285,13 @@ namespace ts.projectSystem {
             p.updateGraph();
 
             const scriptInfo = p.getScriptInfo(f.path);
-            checkSnapLength(scriptInfo.snap(), f.content.length);
+            checkSnapLength(scriptInfo.getSnapshot(), f.content.length);
 
             // open project and replace its content with empty string
             projectService.openClientFile(f.path, "");
-            checkSnapLength(scriptInfo.snap(), 0);
+            checkSnapLength(scriptInfo.getSnapshot(), 0);
         });
-        function checkSnapLength(snap: server.LineIndexSnapshot, expectedLength: number) {
+        function checkSnapLength(snap: IScriptSnapshot, expectedLength: number) {
             assert.equal(snap.getLength(), expectedLength, "Incorrect snapshot size");
         }
     });
@@ -2444,7 +2444,6 @@ namespace ts.projectSystem {
             const cwd = {
                 path: "/a/c"
             };
-            debugger;
             const host = createServerHost([f1, config, node, cwd], { currentDirectory: cwd.path });
             const projectService = createProjectService(host);
             projectService.openClientFile(f1.path);
@@ -2715,7 +2714,7 @@ namespace ts.projectSystem {
 
             // verify content
             const projectServiice = session.getProjectService();
-            const snap1 = projectServiice.getScriptInfo(f1.path).snap();
+            const snap1 = projectServiice.getScriptInfo(f1.path).getSnapshot();
             assert.equal(snap1.getText(0, snap1.getLength()), tmp.content, "content should be equal to the content of temp file");
 
             // reload from original file file
@@ -2727,7 +2726,7 @@ namespace ts.projectSystem {
             });
 
             // verify content
-            const snap2 = projectServiice.getScriptInfo(f1.path).snap();
+            const snap2 = projectServiice.getScriptInfo(f1.path).getSnapshot();
             assert.equal(snap2.getText(0, snap2.getLength()), f1.content, "content should be equal to the content of original file");
 
         });

src/server/editorServices.ts

@@ -424,7 +424,7 @@ namespace ts.server {
                 this.handleDeletedFile(info);
             }
             else {
-                if (info && (!info.isOpen)) {
+                if (info && (!info.isScriptOpen())) {
                     // file has been changed which might affect the set of referenced files in projects that include 
                     // this file and set of inferred projects
                     info.reloadFromFile();
@@ -440,7 +440,7 @@ namespace ts.server {
 
             // TODO: handle isOpen = true case
 
-            if (!info.isOpen) {
+            if (!info.isScriptOpen()) {
                 this.filenameToScriptInfo.remove(info.path);
                 this.lastDeletedFile = info;
 
@@ -634,10 +634,9 @@ namespace ts.server {
             // Closing file should trigger re-reading the file content from disk. This is
             // because the user may chose to discard the buffer content before saving
             // to the disk, and the server's version of the file can be out of sync.
-            info.reloadFromFile();
+            info.close();
 
             removeItemFromSet(this.openFiles, info);
-            info.isOpen = false;
 
             // collect all projects that should be removed
             let projectsToRemove: Project[];
@@ -989,7 +988,7 @@ namespace ts.server {
                 }
                 if (toAdd) {
                     for (const f of toAdd) {
-                        if (f.isOpen && isRootFileInInferredProject(f)) {
+                        if (f.isScriptOpen() && isRootFileInInferredProject(f)) {
                             // if file is already root in some inferred project 
                             // - remove the file from that project and delete the project if necessary
                             const inferredProject = f.containingProjects[0];
@@ -1089,32 +1088,31 @@ namespace ts.server {
         getOrCreateScriptInfoForNormalizedPath(fileName: NormalizedPath, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean) {
             let info = this.getScriptInfoForNormalizedPath(fileName);
             if (!info) {
-                let content: string;
-                if (this.host.fileExists(fileName)) {
-                    // by default pick whatever content was supplied as the argument
-                    // if argument was not given - then for mixed content files assume that its content is empty string
-                    content = fileContent || (hasMixedContent ? "" : this.host.readFile(fileName));
-                }
-                if (!content) {
+                if (openedByClient || this.host.fileExists(fileName)) {
+                    info = new ScriptInfo(this.host, fileName, scriptKind, hasMixedContent);
+
+                    this.filenameToScriptInfo.set(info.path, info);
+
                     if (openedByClient) {
-                        content = "";
+                        if (fileContent === undefined) {
+                            // if file is opened by client and its content is not specified - use file text
+                            fileContent = this.host.readFile(fileName) || "";
+                        }
                     }
-                }
-                if (content !== undefined) {
-                    info = new ScriptInfo(this.host, fileName, content, scriptKind, openedByClient, hasMixedContent);
-                    // do not watch files with mixed content - server doesn't know how to interpret it
-                    this.filenameToScriptInfo.set(info.path, info);
-                    if (!info.isOpen && !hasMixedContent) {
-                        info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
+                    else {
+                        // do not watch files with mixed content - server doesn't know how to interpret it
+                        if (!hasMixedContent) {
+                            info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
+                        }
                     }
                 }
             }
             if (info) {
-                if (fileContent !== undefined) {
-                    info.reload(fileContent);
+                if (openedByClient && !info.isScriptOpen()) {
+                    info.open(fileContent);
                 }
-                if (openedByClient) {
-                    info.isOpen = true;
+                else if (fileContent !== undefined) {
+                    info.reload(fileContent);
                 }
             }
             return info;
@@ -1230,7 +1228,6 @@ namespace ts.server {
             const info = this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName));
             if (info) {
                 this.closeOpenFile(info);
-                info.isOpen = false;
             }
             this.printProjects();
         }
@@ -1255,7 +1252,7 @@ namespace ts.server {
             if (openFiles) {
                 for (const file of openFiles) {
                     const scriptInfo = this.getScriptInfo(file.fileName);
-                    Debug.assert(!scriptInfo || !scriptInfo.isOpen);
+                    Debug.assert(!scriptInfo || !scriptInfo.isScriptOpen());
                     const normalizedPath = scriptInfo ? scriptInfo.fileName : toNormalizedPath(file.fileName);
                     this.openClientFileWithNormalizedPath(normalizedPath, file.content, tryConvertScriptKindName(file.scriptKind), file.hasMixedContent);
                 }

src/server/lsHost.ts

@@ -169,7 +169,7 @@ namespace ts.server {
         getScriptSnapshot(filename: string): ts.IScriptSnapshot {
             const scriptInfo = this.project.getScriptInfoLSHost(filename);
             if (scriptInfo) {
-                return scriptInfo.snap();
+                return scriptInfo.getSnapshot();
             }
         }
 

src/server/project.ts

@@ -448,7 +448,7 @@ namespace ts.server {
 
         containsFile(filename: NormalizedPath, requireOpen?: boolean) {
             const info = this.projectService.getScriptInfoForNormalizedPath(filename);
-            if (info && (info.isOpen || !requireOpen)) {
+            if (info && (info.isScriptOpen() || !requireOpen)) {
                 return this.containsScriptInfo(info);
             }
         }