From 2668f505773fbacb3dbce920e7638833a95bb9e2 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 24 Oct 2017 13:02:41 -0700 Subject: [PATCH 1/2] Reload contents of file from disk irrespective of project presence and file already containing its own text Fixes #19336 --- src/server/project.ts | 10 --------- src/server/scriptInfo.ts | 46 +++++++++++++++++++++------------------- src/server/session.ts | 12 ++++++----- 3 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/server/project.ts b/src/server/project.ts index 7542b30df0..a42926e64e 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -946,16 +946,6 @@ namespace ts.server { } } - reloadScript(filename: NormalizedPath, tempFileName?: NormalizedPath): boolean { - const script = this.projectService.getScriptInfoForNormalizedPath(filename); - if (script) { - Debug.assert(script.isAttached(this)); - script.reloadFromFile(tempFileName); - return true; - } - return false; - } - /* @internal */ getChangesSinceVersion(lastKnownVersion?: number): ProjectFilesWithTSDiagnostics { this.updateGraph(); diff --git a/src/server/scriptInfo.ts b/src/server/scriptInfo.ts index 245b27dd58..9f029c00f0 100644 --- a/src/server/scriptInfo.ts +++ b/src/server/scriptInfo.ts @@ -67,7 +67,10 @@ namespace ts.server { this.lineMap = undefined; } - /** returns true if text changed */ + /** + * Set the contents as newText + * returns true if text changed + */ public reload(newText: string) { Debug.assert(newText !== undefined); @@ -87,31 +90,31 @@ namespace ts.server { } } - /** returns true if text changed */ - public reloadFromDisk() { - let reloaded = false; - if (!this.pendingReloadFromDisk && !this.ownFileText) { - reloaded = this.reload(this.getFileText()); - this.ownFileText = true; - } + /** + * Reads the contents from tempFile(if supplied) or own file and sets it as contents + * returns true if text changed + */ + public reloadWithFileText(tempFileName?: string) { + const reloaded = this.reload(this.getFileText(tempFileName)); + this.ownFileText = !tempFileName || tempFileName === this.fileName; return reloaded; } + /** + * Reloads the contents from the file if there is no pending reload from disk or the contents of file are same as file text + * returns true if text changed + */ + public reloadFromDisk() { + if (!this.pendingReloadFromDisk && !this.ownFileText) { + return this.reloadWithFileText(); + } + return false; + } + public delayReloadFromFileIntoText() { this.pendingReloadFromDisk = true; } - /** returns true if text changed */ - public reloadFromFile(tempFileName: string) { - let reloaded = false; - // Reload if different file or we dont know if we are working with own file text - if (tempFileName !== this.fileName || !this.ownFileText) { - reloaded = this.reload(this.getFileText(tempFileName)); - this.ownFileText = !tempFileName || tempFileName === this.fileName; - } - return reloaded; - } - public getSnapshot(): IScriptSnapshot { return this.useScriptVersionCacheIfValidOrOpen() ? this.svc.getSnapshot() @@ -180,8 +183,7 @@ namespace ts.server { private getOrLoadText() { if (this.text === undefined || this.pendingReloadFromDisk) { Debug.assert(!this.svc || this.pendingReloadFromDisk, "ScriptVersionCache should not be set when reloading from disk"); - this.reload(this.getFileText()); - this.ownFileText = true; + this.reloadWithFileText(); } return this.text; } @@ -385,7 +387,7 @@ namespace ts.server { this.markContainingProjectsAsDirty(); } else { - if (this.textStorage.reloadFromFile(tempFileName)) { + if (this.textStorage.reloadWithFileText(tempFileName)) { this.markContainingProjectsAsDirty(); } } diff --git a/src/server/session.ts b/src/server/session.ts index e8ce752745..9f24979000 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1311,11 +1311,13 @@ namespace ts.server { private reload(args: protocol.ReloadRequestArgs, reqSeq: number) { const file = toNormalizedPath(args.file); const tempFileName = args.tmpfile && toNormalizedPath(args.tmpfile); - const project = this.projectService.getDefaultProjectForFile(file, /*ensureProject*/ true); - this.changeSeq++; - // make sure no changes happen before this one is finished - if (project.reloadScript(file, tempFileName)) { - this.doOutput(/*info*/ undefined, CommandNames.Reload, reqSeq, /*success*/ true); + const info = this.projectService.getScriptInfoForNormalizedPath(file); + if (info) { + this.changeSeq++; + // make sure no changes happen before this one is finished + if (info.reloadFromFile(tempFileName)) { + this.doOutput(/*info*/ undefined, CommandNames.Reload, reqSeq, /*success*/ true); + } } } From f9003b3444d3a393bc5b0f728920e3ff18b2bf73 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Wed, 25 Oct 2017 13:45:48 -0700 Subject: [PATCH 2/2] Add test case to verify reload works without open project --- .../unittests/tsserverProjectSystem.ts | 90 +++++++++++++++++++ .../reference/api/tsserverlibrary.d.ts | 1 - 2 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index f0de849163..455068549d 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3893,6 +3893,96 @@ namespace ts.projectSystem { assert.equal(snap2.getText(0, snap2.getLength()), f1.content, "content should be equal to the content of original file"); }); + + it("should work when script info doesnt have any project open", () => { + const f1 = { + path: "/a/b/app.ts", + content: "let x = 1" + }; + const tmp = { + path: "/a/b/app.tmp", + content: "const y = 42" + }; + const host = createServerHost([f1, tmp, libFile]); + const session = createSession(host); + const openContent = "let z = 1"; + // send open request + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Open, + arguments: { file: f1.path, fileContent: openContent } + }); + + const projectService = session.getProjectService(); + checkNumberOfProjects(projectService, { inferredProjects: 1 }); + const info = projectService.getScriptInfo(f1.path); + assert.isDefined(info); + checkScriptInfoContents(openContent, "contents set during open request"); + + // send close request + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Close, + arguments: { file: f1.path } + }); + checkScriptInfoAndProjects(0, f1.content, "contents of closed file"); + + // Can reload contents of the file when its not open and has no project + // reload from temp file + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Reload, + arguments: { file: f1.path, tmpfile: tmp.path } + }); + checkScriptInfoAndProjects(0, tmp.content, "contents of temp file"); + + // reload from own file + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Reload, + arguments: { file: f1.path } + }); + checkScriptInfoAndProjects(0, f1.content, "contents of closed file"); + + // Open file again without setting its content + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Open, + arguments: { file: f1.path } + }); + checkScriptInfoAndProjects(1, f1.content, "contents of file when opened without specifying contents"); + const snap = info.getSnapshot(); + + // send close request + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Close, + arguments: { file: f1.path } + }); + checkScriptInfoAndProjects(0, f1.content, "contents of closed file"); + assert.strictEqual(info.getSnapshot(), snap); + + // reload from temp file + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Reload, + arguments: { file: f1.path, tmpfile: tmp.path } + }); + checkScriptInfoAndProjects(0, tmp.content, "contents of temp file"); + assert.notStrictEqual(info.getSnapshot(), snap); + + // reload from own file + session.executeCommandSeq({ + command: server.protocol.CommandTypes.Reload, + arguments: { file: f1.path } + }); + checkScriptInfoAndProjects(0, f1.content, "contents of closed file"); + assert.notStrictEqual(info.getSnapshot(), snap); + + function checkScriptInfoAndProjects(inferredProjects: number, contentsOfInfo: string, captionForContents: string) { + checkNumberOfProjects(projectService, { inferredProjects }); + assert.strictEqual(projectService.getScriptInfo(f1.path), info); + checkScriptInfoContents(contentsOfInfo, captionForContents); + } + + function checkScriptInfoContents(contentsOfInfo: string, captionForContents: string) { + const snap = info.getSnapshot(); + assert.equal(snap.getText(0, snap.getLength()), contentsOfInfo, "content should be equal to " + captionForContents); + } + }); }); describe("Inferred projects", () => { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 0f20bfc868..0e21b87a96 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -7227,7 +7227,6 @@ declare namespace ts.server { getScriptInfo(uncheckedFileName: string): ScriptInfo; filesToString(writeProjectFileNames: boolean): string; setCompilerOptions(compilerOptions: CompilerOptions): void; - reloadScript(filename: NormalizedPath, tempFileName?: NormalizedPath): boolean; protected removeRoot(info: ScriptInfo): void; } /**