Skip to content

Commit 153f25a

Browse files
sheetalkamatsheetalkamat
committed
Updates the graph before checking if file is present in project for error checking
When file is moved using getEditsForFileRename, the watch notification could be delayed This could result in project updates in background that could be delayed and result in file not present in the project after its synchronised Fixes microsoft#24547
1 parent 04187bd commit 153f25a

2 files changed

Lines changed: 139 additions & 9 deletions

File tree

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 137 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ namespace ts.projectSystem {
6262
getLogFileName: () => undefined,
6363
};
6464

65+
function createHasErrorMessageLogger() {
66+
let hasErrorMsg = false;
67+
const { close, hasLevel, loggingEnabled, startGroup, endGroup, info, getLogFileName, perftrc } = nullLogger;
68+
const logger: server.Logger = {
69+
close, hasLevel, loggingEnabled, startGroup, endGroup, info, getLogFileName, perftrc,
70+
msg: () => {
71+
hasErrorMsg = true;
72+
}
73+
};
74+
return { logger, hasErrorMsg: () => hasErrorMsg };
75+
}
76+
6577
export class TestTypingsInstaller extends TI.TypingsInstaller implements server.ITypingsInstaller {
6678
protected projectService: server.ProjectService;
6779
constructor(
@@ -2917,14 +2929,7 @@ namespace ts.projectSystem {
29172929
});
29182930

29192931
it("Getting errors from closed script info does not throw exception (because of getting project from orphan script info)", () => {
2920-
let hasErrorMsg = false;
2921-
const { close, hasLevel, loggingEnabled, startGroup, endGroup, info, getLogFileName, perftrc } = nullLogger;
2922-
const logger: server.Logger = {
2923-
close, hasLevel, loggingEnabled, startGroup, endGroup, info, getLogFileName, perftrc,
2924-
msg: () => {
2925-
hasErrorMsg = true;
2926-
}
2927-
};
2932+
const { logger, hasErrorMsg } = createHasErrorMessageLogger();
29282933
const f1 = {
29292934
path: "/a/b/app.ts",
29302935
content: "let x = 1;"
@@ -2954,7 +2959,7 @@ namespace ts.projectSystem {
29542959
files: [f1.path]
29552960
}
29562961
});
2957-
assert.isFalse(hasErrorMsg);
2962+
assert.isFalse(hasErrorMsg());
29582963
});
29592964

29602965
it("Changed module resolution reflected when specifying files list", () => {
@@ -3320,6 +3325,129 @@ namespace ts.projectSystem {
33203325
assert.equal(info.containingProjects.length, 0);
33213326
}
33223327
});
3328+
3329+
it("handles delayed directory watch invoke on file creation", () => {
3330+
const projectRootPath = "/users/username/projects/project";
3331+
const fileB: File = {
3332+
path: `${projectRootPath}/b.ts`,
3333+
content: "export const b = 10;"
3334+
};
3335+
const fileA: File = {
3336+
path: `${projectRootPath}/a.ts`,
3337+
content: "export const a = 10;"
3338+
};
3339+
const fileSubA: File = {
3340+
path: `${projectRootPath}/sub/a.ts`,
3341+
content: fileA.content
3342+
};
3343+
const config: File = {
3344+
path: `${projectRootPath}/tsconfig.json`,
3345+
content: "{}"
3346+
};
3347+
const files = [fileSubA, fileB, config, libFile];
3348+
const host = createServerHost(files);
3349+
const { logger, hasErrorMsg } = createHasErrorMessageLogger();
3350+
const session = createSession(host, { canUseEvents: true, noGetErrOnBackgroundUpdate: true, logger });
3351+
openFile(fileB);
3352+
openFile(fileSubA);
3353+
3354+
const services = session.getProjectService();
3355+
checkNumberOfProjects(services, { configuredProjects: 1 });
3356+
checkProjectActualFiles(services.configuredProjects.get(config.path)!, files.map(f => f.path));
3357+
host.checkTimeoutQueueLengthAndRun(0);
3358+
3359+
// This should schedule 2 timeouts for ensuring project structure and ensuring projects for open file
3360+
const filesWithFileA = files.map(f => f === fileSubA ? fileA : f);
3361+
host.reloadFS(files.map(f => f === fileSubA ? fileA : f));
3362+
host.checkTimeoutQueueLength(2);
3363+
3364+
closeFile(fileSubA);
3365+
// This should cancel existing updates and schedule new ones
3366+
host.checkTimeoutQueueLength(2);
3367+
checkNumberOfProjects(services, { configuredProjects: 1 });
3368+
checkProjectActualFiles(services.configuredProjects.get(config.path)!, files.map(f => f.path));
3369+
3370+
// Open the fileA (as if rename)
3371+
openFile(fileA);
3372+
3373+
// config project is updated to check if fileA is present in it
3374+
checkNumberOfProjects(services, { configuredProjects: 1 });
3375+
checkProjectActualFiles(services.configuredProjects.get(config.path)!, filesWithFileA.map(f => f.path));
3376+
3377+
// Run the timeout for updating configured project and ensuring projects for open file
3378+
host.checkTimeoutQueueLengthAndRun(2);
3379+
checkProjectActualFiles(services.configuredProjects.get(config.path)!, filesWithFileA.map(f => f.path));
3380+
3381+
// file is deleted but watches are not yet invoked
3382+
const originalFileExists = host.fileExists;
3383+
host.fileExists = s => s === fileA.path ? false : originalFileExists.call(host, s);
3384+
closeFile(fileA);
3385+
host.checkTimeoutQueueLength(2); // Update configured project and projects for open file
3386+
checkProjectActualFiles(services.configuredProjects.get(config.path)!, filesWithFileA.map(f => f.path));
3387+
3388+
// host.fileExists = originalFileExists;
3389+
openFile(fileSubA);
3390+
// This should create inferred project since fileSubA not on the disk
3391+
checkProjectActualFiles(services.configuredProjects.get(config.path)!, mapDefined(filesWithFileA, f => f === fileA ? undefined : f.path));
3392+
checkProjectActualFiles(services.inferredProjects[0], [fileSubA.path, libFile.path]);
3393+
3394+
host.checkTimeoutQueueLengthAndRun(2); // Update configured project and projects for open file
3395+
host.fileExists = originalFileExists;
3396+
3397+
// Actually trigger the file move
3398+
host.reloadFS(files);
3399+
host.checkTimeoutQueueLength(2);
3400+
const fileBErrorTimeoutId = host.getNextTimeoutId();
3401+
3402+
session.executeCommandSeq<protocol.GeterrRequest>({
3403+
command: protocol.CommandTypes.Geterr,
3404+
arguments: {
3405+
files: [fileB.path, fileSubA.path],
3406+
delay: 0
3407+
}
3408+
});
3409+
const getErrSeqId = session.getSeq();
3410+
host.checkTimeoutQueueLength(3);
3411+
3412+
session.clearMessages();
3413+
host.runQueuedTimeoutCallbacks(fileBErrorTimeoutId);
3414+
checkErrorMessage(session, "syntaxDiag", { file: fileB.path, diagnostics: [] });
3415+
3416+
session.clearMessages();
3417+
host.runQueuedImmediateCallbacks();
3418+
checkErrorMessage(session, "semanticDiag", { file: fileB.path, diagnostics: [] });
3419+
3420+
session.clearMessages();
3421+
const fileSubAErrorTimeoutId = host.getNextTimeoutId();
3422+
host.runQueuedImmediateCallbacks();
3423+
checkErrorMessage(session, "suggestionDiag", { file: fileB.path, diagnostics: [] });
3424+
3425+
session.clearMessages();
3426+
host.checkTimeoutQueueLength(3);
3427+
host.runQueuedTimeoutCallbacks(fileSubAErrorTimeoutId);
3428+
checkCompleteEvent(session, 1, getErrSeqId);
3429+
assert.isFalse(hasErrorMsg());
3430+
3431+
function openFile(file: File) {
3432+
session.executeCommandSeq<protocol.OpenRequest>({
3433+
command: protocol.CommandTypes.Open,
3434+
arguments: {
3435+
file: file.path,
3436+
fileContent: file.content,
3437+
projectRootPath
3438+
}
3439+
});
3440+
}
3441+
3442+
function closeFile(file: File) {
3443+
session.executeCommandSeq<protocol.CloseRequest>({
3444+
command: protocol.CommandTypes.Close,
3445+
arguments: {
3446+
file: file.path
3447+
}
3448+
});
3449+
}
3450+
});
33233451
});
33243452

33253453
describe("tsserverProjectSystem Proper errors", () => {

‎src/server/session.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,8 @@ namespace ts.server {
511511

512512
const { fileName, project } = checkList[index];
513513
index++;
514+
// Ensure the project is upto date before checking if this file is present in the project
515+
project.updateGraph();
514516
if (!project.containsFile(fileName, requireOpen)) {
515517
return;
516518
}

0 commit comments

Comments
 (0)