Skip to content

Commit aa92568

Browse files
authored
Merge pull request microsoft#25249 from Microsoft/openSameFileWithDifferentText
Retain the version information of script infos when they are deleted
2 parents f7dfc7f + e0d5363 commit aa92568

File tree

5 files changed

+144
-14
lines changed

5 files changed

+144
-14
lines changed

src/harness/virtualFileSystemWithWatch.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,13 @@ interface Array<T> {}`
615615
}
616616
}
617617

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+
618625
removeFolder(folderPath: string, recursive?: boolean) {
619626
const path = this.toFullPath(folderPath);
620627
const currentEntry = this.fs.get(path) as FsFolder;

src/server/editorServices.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,12 @@ namespace ts.server {
346346
* Container of all known scripts
347347
*/
348348
private readonly filenameToScriptInfo = createMap<ScriptInfo>();
349+
/**
350+
* Contains all the deleted script info's version information so that
351+
* it does not reset when creating script info again
352+
* (and could have potentially collided with version where contents mismatch)
353+
*/
354+
private readonly filenameToScriptInfoVersion = createMap<ScriptInfoVersion>();
349355
// Set of all '.js' files ever opened.
350356
private readonly allJsFilesForOpenFileTelemetry = createMap<true>();
351357

@@ -1022,6 +1028,7 @@ namespace ts.server {
10221028

10231029
private deleteScriptInfo(info: ScriptInfo) {
10241030
this.filenameToScriptInfo.delete(info.path);
1031+
this.filenameToScriptInfoVersion.set(info.path, info.getVersion());
10251032
const realpath = info.getRealpathIfDifferent();
10261033
if (realpath) {
10271034
this.realpathToScriptInfos!.remove(realpath, info); // TODO: GH#18217
@@ -1860,8 +1867,9 @@ namespace ts.server {
18601867
if (!openedByClient && !isDynamic && !(hostToQueryFileExistsOn || this.host).fileExists(fileName)) {
18611868
return;
18621869
}
1863-
info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path); // TODO: GH#18217
1870+
info = new ScriptInfo(this.host, fileName, scriptKind!, !!hasMixedContent, path, this.filenameToScriptInfoVersion.get(path)); // TODO: GH#18217
18641871
this.filenameToScriptInfo.set(info.path, info);
1872+
this.filenameToScriptInfoVersion.delete(info.path);
18651873
if (!openedByClient) {
18661874
this.watchClosedScriptInfo(info);
18671875
}

src/server/scriptInfo.ts

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
namespace ts.server {
2+
/*@internal*/
3+
export interface ScriptInfoVersion {
4+
svc: number;
5+
text: number;
6+
}
27

38
/* @internal */
49
export class TextStorage {
10+
/*@internal*/
11+
version: ScriptInfoVersion;
12+
513
/**
614
* Generated only on demand (based on edits, or information requested)
715
* The property text is set to undefined when edits happen on the cache
816
*/
917
private svc: ScriptVersionCache | undefined;
10-
private svcVersion = 0;
1118

1219
/**
1320
* Stores the text when there are no changes to the script version cache
@@ -19,7 +26,6 @@ namespace ts.server {
1926
* Line map for the text when there is no script version cache present
2027
*/
2128
private lineMap: number[] | undefined;
22-
private textVersion = 0;
2329

2430
/**
2531
* True if the text is for the file thats open in the editor
@@ -34,13 +40,14 @@ namespace ts.server {
3440
*/
3541
private pendingReloadFromDisk: boolean;
3642

37-
constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath) {
43+
constructor(private readonly host: ServerHost, private readonly fileName: NormalizedPath, initialVersion?: ScriptInfoVersion) {
44+
this.version = initialVersion || { svc: 0, text: 0 };
3845
}
3946

4047
public getVersion() {
4148
return this.svc
42-
? `SVC-${this.svcVersion}-${this.svc.getSnapshotVersion()}`
43-
: `Text-${this.textVersion}`;
49+
? `SVC-${this.version.svc}-${this.svc.getSnapshotVersion()}`
50+
: `Text-${this.version.text}`;
4451
}
4552

4653
public hasScriptVersionCache_TestOnly() {
@@ -55,7 +62,7 @@ namespace ts.server {
5562
this.svc = undefined;
5663
this.text = newText;
5764
this.lineMap = undefined;
58-
this.textVersion++;
65+
this.version.text++;
5966
}
6067

6168
public edit(start: number, end: number, newText: string) {
@@ -163,7 +170,7 @@ namespace ts.server {
163170
private switchToScriptVersionCache(): ScriptVersionCache {
164171
if (!this.svc || this.pendingReloadFromDisk) {
165172
this.svc = ScriptVersionCache.fromString(this.getOrLoadText());
166-
this.svcVersion++;
173+
this.version.svc++;
167174
}
168175
return this.svc;
169176
}
@@ -235,10 +242,11 @@ namespace ts.server {
235242
readonly fileName: NormalizedPath,
236243
readonly scriptKind: ScriptKind,
237244
public readonly hasMixedContent: boolean,
238-
readonly path: Path) {
245+
readonly path: Path,
246+
initialVersion?: ScriptInfoVersion) {
239247
this.isDynamic = isDynamicFileName(fileName);
240248

241-
this.textStorage = new TextStorage(host, fileName);
249+
this.textStorage = new TextStorage(host, fileName, initialVersion);
242250
if (hasMixedContent || this.isDynamic) {
243251
this.textStorage.reload("");
244252
this.realpath = this.path;
@@ -248,6 +256,11 @@ namespace ts.server {
248256
: getScriptKindFromFileName(fileName);
249257
}
250258

259+
/*@internal*/
260+
getVersion() {
261+
return this.textStorage.version;
262+
}
263+
251264
/*@internal*/
252265
public isDynamicOrHasMixedContent() {
253266
return this.hasMixedContent || this.isDynamic;

src/testRunner/unittests/tsserverProjectSystem.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8848,6 +8848,103 @@ export const x = 10;`
88488848
});
88498849
});
88508850

8851+
describe("tsserverProjectSystem syntax operations", () => {
8852+
function navBarFull(session: TestSession, file: File) {
8853+
return JSON.stringify(session.executeCommandSeq<protocol.FileRequest>({
8854+
command: protocol.CommandTypes.NavBarFull,
8855+
arguments: { file: file.path }
8856+
}).response);
8857+
}
8858+
8859+
function openFile(session: TestSession, file: File) {
8860+
session.executeCommandSeq<protocol.OpenRequest>({
8861+
command: protocol.CommandTypes.Open,
8862+
arguments: { file: file.path, fileContent: file.content }
8863+
});
8864+
}
8865+
8866+
it("works when file is removed and added with different content", () => {
8867+
const projectRoot = "/user/username/projects/myproject";
8868+
const app: File = {
8869+
path: `${projectRoot}/app.ts`,
8870+
content: "console.log('Hello world');"
8871+
};
8872+
const unitTest1: File = {
8873+
path: `${projectRoot}/unitTest1.ts`,
8874+
content: `import assert = require('assert');
8875+
8876+
describe("Test Suite 1", () => {
8877+
it("Test A", () => {
8878+
assert.ok(true, "This shouldn't fail");
8879+
});
8880+
8881+
it("Test B", () => {
8882+
assert.ok(1 === 1, "This shouldn't fail");
8883+
assert.ok(false, "This should fail");
8884+
});
8885+
});`
8886+
};
8887+
const tsconfig: File = {
8888+
path: `${projectRoot}/tsconfig.json`,
8889+
content: "{}"
8890+
};
8891+
const files = [app, libFile, tsconfig];
8892+
const host = createServerHost(files);
8893+
const session = createSession(host);
8894+
const service = session.getProjectService();
8895+
openFile(session, app);
8896+
8897+
checkNumberOfProjects(service, { configuredProjects: 1 });
8898+
const project = service.configuredProjects.get(tsconfig.path)!;
8899+
const expectedFilesWithoutUnitTest1 = files.map(f => f.path);
8900+
checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
8901+
8902+
host.writeFile(unitTest1.path, unitTest1.content);
8903+
host.runQueuedTimeoutCallbacks();
8904+
const expectedFilesWithUnitTest1 = expectedFilesWithoutUnitTest1.concat(unitTest1.path);
8905+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8906+
8907+
openFile(session, unitTest1);
8908+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8909+
8910+
const navBarResultUnitTest1 = navBarFull(session, unitTest1);
8911+
host.removeFile(unitTest1.path);
8912+
host.checkTimeoutQueueLengthAndRun(2);
8913+
checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
8914+
8915+
session.executeCommandSeq<protocol.CloseRequest>({
8916+
command: protocol.CommandTypes.Close,
8917+
arguments: { file: unitTest1.path }
8918+
});
8919+
checkProjectActualFiles(project, expectedFilesWithoutUnitTest1);
8920+
8921+
const unitTest1WithChangedContent: File = {
8922+
path: unitTest1.path,
8923+
content: `import assert = require('assert');
8924+
8925+
export function Test1() {
8926+
assert.ok(true, "This shouldn't fail");
8927+
};
8928+
8929+
export function Test2() {
8930+
assert.ok(1 === 1, "This shouldn't fail");
8931+
assert.ok(false, "This should fail");
8932+
};`
8933+
};
8934+
host.writeFile(unitTest1.path, unitTest1WithChangedContent.content);
8935+
host.runQueuedTimeoutCallbacks();
8936+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8937+
8938+
openFile(session, unitTest1WithChangedContent);
8939+
checkProjectActualFiles(project, expectedFilesWithUnitTest1);
8940+
const sourceFile = project.getLanguageService().getNonBoundSourceFile(unitTest1WithChangedContent.path);
8941+
assert.strictEqual(sourceFile.text, unitTest1WithChangedContent.content);
8942+
8943+
const navBarResultUnitTest1WithChangedContent = navBarFull(session, unitTest1WithChangedContent);
8944+
assert.notStrictEqual(navBarResultUnitTest1WithChangedContent, navBarResultUnitTest1, "With changes in contents of unitTest file, we should see changed naviagation bar item result");
8945+
});
8946+
});
8947+
88518948
function makeSampleProjects() {
88528949
const aTs: File = {
88538950
path: "/a/a.ts",

tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13342,18 +13342,21 @@ declare namespace ts.server.protocol {
1334213342
}
1334313343
}
1334413344
declare namespace ts.server {
13345+
interface ScriptInfoVersion {
13346+
svc: number;
13347+
text: number;
13348+
}
1334513349
class TextStorage {
1334613350
private readonly host;
1334713351
private readonly fileName;
13352+
version: ScriptInfoVersion;
1334813353
private svc;
13349-
private svcVersion;
1335013354
private text;
1335113355
private lineMap;
13352-
private textVersion;
1335313356
isOpen: boolean;
1335413357
private ownFileText;
1335513358
private pendingReloadFromDisk;
13356-
constructor(host: ServerHost, fileName: NormalizedPath);
13359+
constructor(host: ServerHost, fileName: NormalizedPath, initialVersion?: ScriptInfoVersion);
1335713360
getVersion(): string;
1335813361
hasScriptVersionCache_TestOnly(): boolean;
1335913362
useScriptVersionCache_TestOnly(): void;
@@ -13393,7 +13396,8 @@ declare namespace ts.server {
1339313396
readonly isDynamic: boolean;
1339413397
private realpath;
1339513398
cacheSourceFile: DocumentRegistrySourceFileCache;
13396-
constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path);
13399+
constructor(host: ServerHost, fileName: NormalizedPath, scriptKind: ScriptKind, hasMixedContent: boolean, path: Path, initialVersion?: ScriptInfoVersion);
13400+
getVersion(): ScriptInfoVersion;
1339713401
isDynamicOrHasMixedContent(): boolean;
1339813402
isScriptOpen(): boolean;
1339913403
open(newText: string): void;
@@ -13803,6 +13807,7 @@ declare namespace ts.server {
1380313807
readonly typingsCache: TypingsCache;
1380413808
readonly documentRegistry: DocumentRegistry;
1380513809
private readonly filenameToScriptInfo;
13810+
private readonly filenameToScriptInfoVersion;
1380613811
private readonly allJsFilesForOpenFileTelemetry;
1380713812
readonly realpathToScriptInfos: MultiMap<ScriptInfo> | undefined;
1380813813
private readonly externalProjectToConfiguredProjectMap;

0 commit comments

Comments
 (0)