Skip to content

Commit 7da3383

Browse files
vladimavladima
authored
do not use ScriptVersionCache for closed files (microsoft#12777)
1 parent 9dd769d commit 7da3383

10 files changed

Lines changed: 303 additions & 76 deletions

File tree

‎Jakefile.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ var harnessSources = harnessCoreSources.concat([
250250
"convertToBase64.ts",
251251
"transpile.ts",
252252
"reuseProgramStructure.ts",
253+
"textStorage.ts",
253254
"cachingInServerLSHost.ts",
254255
"moduleResolution.ts",
255256
"tsconfigParsing.ts",

‎src/harness/unittests/textStorage.ts‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/// <reference path="../harness.ts" />
2+
/// <reference path="../../server/scriptVersionCache.ts"/>
3+
/// <reference path="./tsserverProjectSystem.ts" />
4+
5+
namespace ts.textStorage {
6+
describe("Text storage", () => {
7+
const f = {
8+
path: "/a/app.ts",
9+
content: `
10+
let x = 1;
11+
let y = 2;
12+
function bar(a: number) {
13+
return a + 1;
14+
}`
15+
};
16+
17+
it("text based storage should be have exactly the same as script version cache", () => {
18+
19+
debugger
20+
const host = ts.projectSystem.createServerHost([f]);
21+
22+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path));
23+
const ts2 = new server.TextStorage(host, server.asNormalizedPath(f.path));
24+
25+
ts1.useScriptVersionCache();
26+
ts2.useText();
27+
28+
const lineMap = computeLineStarts(f.content);
29+
30+
for (let line = 0; line < lineMap.length; line++) {
31+
const start = lineMap[line];
32+
const end = line === lineMap.length - 1 ? f.path.length : lineMap[line + 1];
33+
34+
for (let offset = 0; offset < end - start; offset++) {
35+
const pos1 = ts1.lineOffsetToPosition(line + 1, offset + 1);
36+
const pos2 = ts2.lineOffsetToPosition(line + 1, offset + 1);
37+
assert.isTrue(pos1 === pos2, `lineOffsetToPosition ${line + 1}-${offset + 1}: expected ${pos1} to equal ${pos2}`);
38+
}
39+
40+
const {start: start1, length: length1 } = ts1.lineToTextSpan(line);
41+
const {start: start2, length: length2 } = ts2.lineToTextSpan(line);
42+
assert.isTrue(start1 === start2, `lineToTextSpan ${line}::start:: expected ${start1} to equal ${start2}`);
43+
assert.isTrue(length1 === length2, `lineToTextSpan ${line}::length:: expected ${length1} to equal ${length2}`);
44+
}
45+
46+
for (let pos = 0; pos < f.content.length; pos++) {
47+
const { line: line1, offset: offset1 } = ts1.positionToLineOffset(pos);
48+
const { line: line2, offset: offset2 } = ts2.positionToLineOffset(pos);
49+
assert.isTrue(line1 === line2, `positionToLineOffset ${pos}::line:: expected ${line1} to equal ${line2}`);
50+
assert.isTrue(offset1 === offset2, `positionToLineOffset ${pos}::offset:: expected ${offset1} to equal ${offset2}`);
51+
}
52+
});
53+
54+
it("should switch to script version cache if necessary", () => {
55+
const host = ts.projectSystem.createServerHost([f]);
56+
const ts1 = new server.TextStorage(host, server.asNormalizedPath(f.path));
57+
58+
ts1.getSnapshot();
59+
assert.isTrue(!ts1.hasScriptVersionCache(), "should not have script version cache - 1");
60+
61+
ts1.edit(0, 5, " ");
62+
assert.isTrue(ts1.hasScriptVersionCache(), "have script version cache - 1");
63+
64+
ts1.useText();
65+
assert.isTrue(!ts1.hasScriptVersionCache(), "should not have script version cache - 2");
66+
67+
ts1.getLineInfo(0);
68+
assert.isTrue(ts1.hasScriptVersionCache(), "have script version cache - 2");
69+
})
70+
});
71+
}

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,7 +1574,7 @@ namespace ts.projectSystem {
15741574
const project = projectService.externalProjects[0];
15751575

15761576
const scriptInfo = project.getScriptInfo(file1.path);
1577-
const snap = scriptInfo.snap();
1577+
const snap = scriptInfo.getSnapshot();
15781578
const actualText = snap.getText(0, snap.getLength());
15791579
assert.equal(actualText, "", `expected content to be empty string, got "${actualText}"`);
15801580

@@ -1587,7 +1587,7 @@ namespace ts.projectSystem {
15871587
projectService.closeClientFile(file1.path);
15881588

15891589
const scriptInfo2 = project.getScriptInfo(file1.path);
1590-
const snap2 = scriptInfo2.snap();
1590+
const snap2 = scriptInfo2.getSnapshot();
15911591
const actualText2 = snap2.getText(0, snap.getLength());
15921592
assert.equal(actualText2, "", `expected content to be empty string, got "${actualText2}"`);
15931593
});
@@ -2285,13 +2285,13 @@ namespace ts.projectSystem {
22852285
p.updateGraph();
22862286

22872287
const scriptInfo = p.getScriptInfo(f.path);
2288-
checkSnapLength(scriptInfo.snap(), f.content.length);
2288+
checkSnapLength(scriptInfo.getSnapshot(), f.content.length);
22892289

22902290
// open project and replace its content with empty string
22912291
projectService.openClientFile(f.path, "");
2292-
checkSnapLength(scriptInfo.snap(), 0);
2292+
checkSnapLength(scriptInfo.getSnapshot(), 0);
22932293
});
2294-
function checkSnapLength(snap: server.LineIndexSnapshot, expectedLength: number) {
2294+
function checkSnapLength(snap: IScriptSnapshot, expectedLength: number) {
22952295
assert.equal(snap.getLength(), expectedLength, "Incorrect snapshot size");
22962296
}
22972297
});
@@ -2444,7 +2444,6 @@ namespace ts.projectSystem {
24442444
const cwd = {
24452445
path: "/a/c"
24462446
};
2447-
debugger;
24482447
const host = createServerHost([f1, config, node, cwd], { currentDirectory: cwd.path });
24492448
const projectService = createProjectService(host);
24502449
projectService.openClientFile(f1.path);
@@ -2715,7 +2714,7 @@ namespace ts.projectSystem {
27152714

27162715
// verify content
27172716
const projectServiice = session.getProjectService();
2718-
const snap1 = projectServiice.getScriptInfo(f1.path).snap();
2717+
const snap1 = projectServiice.getScriptInfo(f1.path).getSnapshot();
27192718
assert.equal(snap1.getText(0, snap1.getLength()), tmp.content, "content should be equal to the content of temp file");
27202719

27212720
// reload from original file file
@@ -2727,7 +2726,7 @@ namespace ts.projectSystem {
27272726
});
27282727

27292728
// verify content
2730-
const snap2 = projectServiice.getScriptInfo(f1.path).snap();
2729+
const snap2 = projectServiice.getScriptInfo(f1.path).getSnapshot();
27312730
assert.equal(snap2.getText(0, snap2.getLength()), f1.content, "content should be equal to the content of original file");
27322731

27332732
});

‎src/server/editorServices.ts‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ namespace ts.server {
424424
this.handleDeletedFile(info);
425425
}
426426
else {
427-
if (info && (!info.isOpen)) {
427+
if (info && (!info.isScriptOpen())) {
428428
// file has been changed which might affect the set of referenced files in projects that include
429429
// this file and set of inferred projects
430430
info.reloadFromFile();
@@ -440,7 +440,7 @@ namespace ts.server {
440440

441441
// TODO: handle isOpen = true case
442442

443-
if (!info.isOpen) {
443+
if (!info.isScriptOpen()) {
444444
this.filenameToScriptInfo.remove(info.path);
445445
this.lastDeletedFile = info;
446446

@@ -634,10 +634,9 @@ namespace ts.server {
634634
// Closing file should trigger re-reading the file content from disk. This is
635635
// because the user may chose to discard the buffer content before saving
636636
// to the disk, and the server's version of the file can be out of sync.
637-
info.reloadFromFile();
637+
info.close();
638638

639639
removeItemFromSet(this.openFiles, info);
640-
info.isOpen = false;
641640

642641
// collect all projects that should be removed
643642
let projectsToRemove: Project[];
@@ -989,7 +988,7 @@ namespace ts.server {
989988
}
990989
if (toAdd) {
991990
for (const f of toAdd) {
992-
if (f.isOpen && isRootFileInInferredProject(f)) {
991+
if (f.isScriptOpen() && isRootFileInInferredProject(f)) {
993992
// if file is already root in some inferred project
994993
// - remove the file from that project and delete the project if necessary
995994
const inferredProject = f.containingProjects[0];
@@ -1089,32 +1088,31 @@ namespace ts.server {
10891088
getOrCreateScriptInfoForNormalizedPath(fileName: NormalizedPath, openedByClient: boolean, fileContent?: string, scriptKind?: ScriptKind, hasMixedContent?: boolean) {
10901089
let info = this.getScriptInfoForNormalizedPath(fileName);
10911090
if (!info) {
1092-
let content: string;
1093-
if (this.host.fileExists(fileName)) {
1094-
// by default pick whatever content was supplied as the argument
1095-
// if argument was not given - then for mixed content files assume that its content is empty string
1096-
content = fileContent || (hasMixedContent ? "" : this.host.readFile(fileName));
1097-
}
1098-
if (!content) {
1091+
if (openedByClient || this.host.fileExists(fileName)) {
1092+
info = new ScriptInfo(this.host, fileName, scriptKind, hasMixedContent);
1093+
1094+
this.filenameToScriptInfo.set(info.path, info);
1095+
10991096
if (openedByClient) {
1100-
content = "";
1097+
if (fileContent === undefined) {
1098+
// if file is opened by client and its content is not specified - use file text
1099+
fileContent = this.host.readFile(fileName) || "";
1100+
}
11011101
}
1102-
}
1103-
if (content !== undefined) {
1104-
info = new ScriptInfo(this.host, fileName, content, scriptKind, openedByClient, hasMixedContent);
1105-
// do not watch files with mixed content - server doesn't know how to interpret it
1106-
this.filenameToScriptInfo.set(info.path, info);
1107-
if (!info.isOpen && !hasMixedContent) {
1108-
info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
1102+
else {
1103+
// do not watch files with mixed content - server doesn't know how to interpret it
1104+
if (!hasMixedContent) {
1105+
info.setWatcher(this.host.watchFile(fileName, _ => this.onSourceFileChanged(fileName)));
1106+
}
11091107
}
11101108
}
11111109
}
11121110
if (info) {
1113-
if (fileContent !== undefined) {
1114-
info.reload(fileContent);
1111+
if (openedByClient && !info.isScriptOpen()) {
1112+
info.open(fileContent);
11151113
}
1116-
if (openedByClient) {
1117-
info.isOpen = true;
1114+
else if (fileContent !== undefined) {
1115+
info.reload(fileContent);
11181116
}
11191117
}
11201118
return info;
@@ -1230,7 +1228,6 @@ namespace ts.server {
12301228
const info = this.getScriptInfoForNormalizedPath(toNormalizedPath(uncheckedFileName));
12311229
if (info) {
12321230
this.closeOpenFile(info);
1233-
info.isOpen = false;
12341231
}
12351232
this.printProjects();
12361233
}
@@ -1255,7 +1252,7 @@ namespace ts.server {
12551252
if (openFiles) {
12561253
for (const file of openFiles) {
12571254
const scriptInfo = this.getScriptInfo(file.fileName);
1258-
Debug.assert(!scriptInfo || !scriptInfo.isOpen);
1255+
Debug.assert(!scriptInfo || !scriptInfo.isScriptOpen());
12591256
const normalizedPath = scriptInfo ? scriptInfo.fileName : toNormalizedPath(file.fileName);
12601257
this.openClientFileWithNormalizedPath(normalizedPath, file.content, tryConvertScriptKindName(file.scriptKind), file.hasMixedContent);
12611258
}

‎src/server/lsHost.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ namespace ts.server {
169169
getScriptSnapshot(filename: string): ts.IScriptSnapshot {
170170
const scriptInfo = this.project.getScriptInfoLSHost(filename);
171171
if (scriptInfo) {
172-
return scriptInfo.snap();
172+
return scriptInfo.getSnapshot();
173173
}
174174
}
175175

‎src/server/project.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ namespace ts.server {
448448

449449
containsFile(filename: NormalizedPath, requireOpen?: boolean) {
450450
const info = this.projectService.getScriptInfoForNormalizedPath(filename);
451-
if (info && (info.isOpen || !requireOpen)) {
451+
if (info && (info.isScriptOpen() || !requireOpen)) {
452452
return this.containsScriptInfo(info);
453453
}
454454
}

0 commit comments

Comments
 (0)