From 6ec763884ec9a2bd2ea8c0a1465fc5bbcc13ccd7 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 23 Apr 2018 14:57:10 -0700 Subject: [PATCH] Fix the crash when reporting errors of file that was referenced by inferred project root, is opened right after closing the root file Fixes the crash reported in https://github.com/Microsoft/TypeScript/issues/23255#issuecomment-382653325 --- .../unittests/tsserverProjectSystem.ts | 93 ++++++++++++++++++- src/server/editorServices.ts | 19 ++-- .../reference/api/tsserverlibrary.d.ts | 1 - 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index a35f797eaa1..1c0dd0beb5c 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -480,6 +480,10 @@ namespace ts.projectSystem { checkNthEvent(session, server.toEvent(eventName, diagnostics), 0, isMostRecent); } + function createDiagnostic(start: protocol.Location, end: protocol.Location, message: DiagnosticMessage, args: ReadonlyArray = [], category = diagnosticCategoryName(message), reportsUnnecessary?: {}): protocol.Diagnostic { + return { start, end, text: formatStringFromArgs(message.message, args), code: message.code, category, reportsUnnecessary, source: undefined }; + } + function checkCompleteEvent(session: TestSession, numberOfCurrentEvents: number, expectedSequenceId: number, isMostRecent = true): void { checkNthEvent(session, server.toEvent("requestCompleted", { request_seq: expectedSequenceId }), numberOfCurrentEvents - 1, isMostRecent); } @@ -496,7 +500,7 @@ namespace ts.projectSystem { function checkNthEvent(session: TestSession, expectedEvent: protocol.Event, index: number, isMostRecent: boolean) { const events = session.events; - assert.deepEqual(events[index], expectedEvent); + assert.deepEqual(events[index], expectedEvent, `Expected ${JSON.stringify(expectedEvent)} at ${index} in ${JSON.stringify(events)}`); const outputs = session.host.getOutput(); assert.equal(outputs[index], server.formatMessage(expectedEvent, nullLogger, Utils.byteLength, session.host.newLine)); @@ -3333,6 +3337,89 @@ namespace ts.projectSystem { checkCompleteEvent(session, 1, expectedSequenceId); session.clearMessages(); }); + + it("Reports errors correctly when file referenced by inferred project root, is opened right after closing the root file", () => { + const projectRoot = "/user/username/projects/myproject"; + const app: FileOrFolder = { + path: `${projectRoot}/src/client/app.js`, + content: "" + }; + const serverUtilities: FileOrFolder = { + path: `${projectRoot}/src/server/utilities.js`, + content: `function getHostName() { return "hello"; } export { getHostName };` + }; + const backendTest: FileOrFolder = { + path: `${projectRoot}/test/backend/index.js`, + content: `import { getHostName } from '../../src/server/utilities';export default getHostName;` + }; + const files = [libFile, app, serverUtilities, backendTest]; + const host = createServerHost(files); + const session = createSession(host, { useInferredProjectPerProjectRoot: true, canUseEvents: true }); + session.executeCommandSeq({ + command: protocol.CommandTypes.Open, + arguments: { + file: app.path, + projectRootPath: projectRoot + } + }); + const service = session.getProjectService(); + checkNumberOfProjects(service, { inferredProjects: 1 }); + const project = service.inferredProjects[0]; + checkProjectActualFiles(project, [libFile.path, app.path]); + session.executeCommandSeq({ + command: protocol.CommandTypes.Open, + arguments: { + file: backendTest.path, + projectRootPath: projectRoot + } + }); + checkNumberOfProjects(service, { inferredProjects: 1 }); + checkProjectActualFiles(project, files.map(f => f.path)); + checkErrors([backendTest.path, app.path]); + session.executeCommandSeq({ + command: protocol.CommandTypes.Close, + arguments: { + file: backendTest.path + } + }); + session.executeCommandSeq({ + command: protocol.CommandTypes.Open, + arguments: { + file: serverUtilities.path, + projectRootPath: projectRoot + } + }); + checkErrors([serverUtilities.path, app.path]); + + function checkErrors(openFiles: [string, string]) { + const expectedSequenceId = session.getNextSeq(); + session.executeCommandSeq({ + command: protocol.CommandTypes.Geterr, + arguments: { + delay: 0, + files: openFiles + } + }); + + for (const openFile of openFiles) { + session.clearMessages(); + host.checkTimeoutQueueLength(3); + host.runQueuedTimeoutCallbacks(host.getNextTimeoutId() - 1); + + checkErrorMessage(session, "syntaxDiag", { file: openFile, diagnostics: [] }); + session.clearMessages(); + + host.runQueuedImmediateCallbacks(); + checkErrorMessage(session, "semanticDiag", { file: openFile, diagnostics: [] }); + session.clearMessages(); + + host.runQueuedImmediateCallbacks(1); + checkErrorMessage(session, "suggestionDiag", { file: openFile, diagnostics: [] }); + } + checkCompleteEvent(session, 2, expectedSequenceId); + session.clearMessages(); + } + }); }); describe("tsserverProjectSystem autoDiscovery", () => { @@ -4293,10 +4380,6 @@ namespace ts.projectSystem { session.clearMessages(); }); - - function createDiagnostic(start: protocol.Location, end: protocol.Location, message: DiagnosticMessage, args: ReadonlyArray = [], category = diagnosticCategoryName(message), reportsUnnecessary?: {}): protocol.Diagnostic { - return { start, end, text: formatStringFromArgs(message.message, args), code: message.code, category, reportsUnnecessary, source: undefined }; - } }); describe("tsserverProjectSystem Configure file diagnostics events", () => { diff --git a/src/server/editorServices.ts b/src/server/editorServices.ts index 1271b98c1c7..dfd5539cca3 100644 --- a/src/server/editorServices.ts +++ b/src/server/editorServices.ts @@ -310,6 +310,10 @@ namespace ts.server { return `Project: ${project ? project.getProjectName() : ""} WatchType: ${watchType}`; } + function updateProjectIfDirty(project: Project) { + return project.dirty && project.updateGraph(); + } + export class ProjectService { /*@internal*/ @@ -673,7 +677,7 @@ namespace ts.server { let hasChanges = this.pendingEnsureProjectForOpenFiles; this.pendingProjectUpdates.clear(); const updateGraph = (project: Project) => { - hasChanges = this.updateProjectIfDirty(project) || hasChanges; + hasChanges = updateProjectIfDirty(project) || hasChanges; }; this.externalProjects.forEach(updateGraph); @@ -684,10 +688,6 @@ namespace ts.server { } } - private updateProjectIfDirty(project: Project) { - return project.dirty && project.updateGraph(); - } - getFormatCodeOptions(file: NormalizedPath) { const info = this.getScriptInfoForNormalizedPath(file); return info && info.getFormatCodeSettings() || this.hostConfiguration.formatCodeOptions; @@ -1980,7 +1980,7 @@ namespace ts.server { } }); this.pendingEnsureProjectForOpenFiles = false; - this.inferredProjects.forEach(p => this.updateProjectIfDirty(p)); + this.inferredProjects.forEach(updateProjectIfDirty); this.logger.info("Structure after ensureProjectForOpenFiles:"); this.printProjects(); @@ -2027,7 +2027,7 @@ namespace ts.server { } else { // Ensure project is ready to check if it contains opened script info - project.updateGraph(); + updateProjectIfDirty(project); } } } @@ -2036,6 +2036,11 @@ namespace ts.server { // - external project search, which updates the project before checking if info is present in it // - configured project - either created or updated to ensure we know correct status of info + // At this point we need to ensure that containing projects of the info are uptodate + // This will ensure that later question of info.isOrphan() will return correct answer + // and we correctly create inferred project for the info + info.containingProjects.forEach(updateProjectIfDirty); + // At this point if file is part of any any configured or external project, then it would be present in the containing projects // So if it still doesnt have any containing projects, it needs to be part of inferred project if (info.isOrphan()) { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 9bd2de6aa41..1f5757ff4ce 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -8050,7 +8050,6 @@ declare namespace ts.server { * ensure that each open script info has project */ private ensureProjectStructuresUptoDate; - private updateProjectIfDirty; getFormatCodeOptions(file: NormalizedPath): FormatCodeSettings; getPreferences(file: NormalizedPath): UserPreferences; private onSourceFileChanged;