Fix the crash when reporting errors of file that was referenced by inferred project root, is opened right after closing the root file

sheetalkamat · sheetalkamat · commit 6ec763884ec9 · 2018-04-23T15:36:32.000-07:00
Fixes the crash reported in microsoft#23255 (comment)
diff --git 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<string> = [], 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<protocol.OpenRequest>({
+                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<protocol.OpenRequest>({
+                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<protocol.CloseRequest>({
+                command: protocol.CommandTypes.Close,
+                arguments: {
+                    file: backendTest.path
+                }
+            });
+            session.executeCommandSeq<protocol.OpenRequest>({
+                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<protocol.GeterrRequest>({
+                    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<string> = [], 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
@@ -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
@@ -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;
