From 3f198e6751892a531d20524300bba34544c02bf2 Mon Sep 17 00:00:00 2001 From: Jason Ramsay Date: Fri, 17 Feb 2017 13:54:07 -0800 Subject: [PATCH] Adding cancellation token checks for lower priority tasks --- .../unittests/tsserverProjectSystem.ts | 82 +++++++ src/services/navigationBar.ts | 232 +++++++++--------- src/services/outliningElementsCollector.ts | 4 +- src/services/services.ts | 6 +- 4 files changed, 207 insertions(+), 117 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 92ec0b4ee0e..01e484fd4ba 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3460,6 +3460,88 @@ namespace ts.projectSystem { return JSON.parse(server.extractMessage(host.getOutput()[n])); } }); + it("Lower priority tasks are cancellable", () => { + const f1 = { + path: "/a/app.ts", + content: `{ let x = 1; } var foo = "foo"; var bar = "bar"; var fooBar = "fooBar";` + }; + const config = { + path: "/a/tsconfig.json", + content: JSON.stringify({ + compilerOptions: {} + }) + }; + + let requestToCancel = -1; + let isCancellationRequestedCount = 0; + let cancelAfterRequest = 3; + let operationCanceledExceptionThrown = false; + const cancellationToken: server.ServerCancellationToken = (function () { + let currentId: number; + return { + setRequest(requestId) { + currentId = requestId; + }, + resetRequest(requestId) { + assert.equal(requestId, currentId, "unexpected request id in cancellation") + currentId = undefined; + }, + isCancellationRequested() { + isCancellationRequestedCount++; + return requestToCancel === currentId && isCancellationRequestedCount >= cancelAfterRequest; + } + } + })(); + const host = createServerHost([f1, config]); + const session = createSession(host, /*typingsInstaller*/ undefined, () => { }, cancellationToken); + { + session.executeCommandSeq({ + command: "open", + arguments: { file: f1.path } + }); + + // send navbar request (normal priority) + session.executeCommandSeq({ + command: "navbar", + arguments: { file: f1.path } + }); + + // ensure the nav bar request can be canceled + verifyExecuteCommandSeqIsCancellable({ + command: "navbar", + arguments: { file: f1.path } + }); + + // send outlining spans request (normal priority) + session.executeCommandSeq({ + command: "outliningSpans", + arguments: { file: f1.path } + }); + + // ensure the outlining spans request can be canceled + verifyExecuteCommandSeqIsCancellable({ + command: "outliningSpans", + arguments: { file: f1.path } + }); + } + + function verifyExecuteCommandSeqIsCancellable(request: Partial) { + // Set the next request to be cancellable + // The cancellation token will cancel the request the third time + // isCancellationRequested() is called. + requestToCancel = session.getNextSeq(); + isCancellationRequestedCount = 0; + operationCanceledExceptionThrown = false; + + try { + session.executeCommandSeq(request); + } catch (e) { + assert(e instanceof OperationCanceledException); + operationCanceledExceptionThrown = true; + } + assert(operationCanceledExceptionThrown); + } + }); }); describe("maxNodeModuleJsDepth for inferred projects", () => { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 25059d1f57d..37b6bedbd5b 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -14,16 +14,16 @@ namespace ts.NavigationBar { indent: number; // # of parents } - export function getNavigationBarItems(sourceFile: SourceFile): NavigationBarItem[] { + export function getNavigationBarItems(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarItem[] { curSourceFile = sourceFile; - const result = map(topLevelItems(rootNavigationBarNode(sourceFile)), convertToTopLevelItem); + const result = map(topLevelItems(rootNavigationBarNode(sourceFile, cancellationToken)), convertToTopLevelItem); curSourceFile = undefined; return result; } - export function getNavigationTree(sourceFile: SourceFile): NavigationTree { + export function getNavigationTree(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationTree { curSourceFile = sourceFile; - const result = convertToTree(rootNavigationBarNode(sourceFile)); + const result = convertToTree(rootNavigationBarNode(sourceFile, cancellationToken)); curSourceFile = undefined; return result; } @@ -55,12 +55,12 @@ namespace ts.NavigationBar { const parentsStack: NavigationBarNode[] = []; let parent: NavigationBarNode; - function rootNavigationBarNode(sourceFile: SourceFile): NavigationBarNode { + function rootNavigationBarNode(sourceFile: SourceFile, cancellationToken: CancellationToken): NavigationBarNode { Debug.assert(!parentsStack.length); const root: NavigationBarNode = { node: sourceFile, additionalNodes: undefined, parent: undefined, children: undefined, indent: 0 }; parent = root; for (const statement of sourceFile.statements) { - addChildrenRecursively(statement); + addChildrenRecursively(statement, cancellationToken); } endNode(); Debug.assert(!parent && !parentsStack.length); @@ -103,138 +103,144 @@ namespace ts.NavigationBar { parent = parentsStack.pop(); } - function addNodeWithRecursiveChild(node: Node, child: Node): void { + function addNodeWithRecursiveChild(node: Node, child: Node, addChildrenRecursively: (node: Node) => void): void { startNode(node); addChildrenRecursively(child); endNode(); } /** Look for navigation bar items in node's subtree, adding them to the current `parent`. */ - function addChildrenRecursively(node: Node): void { - if (!node || isToken(node)) { - return; - } + function addChildrenRecursively(node: Node, cancellationToken: CancellationToken): void { + function addChildrenRecursively(node: Node): void { + cancellationToken.throwIfCancellationRequested(); - switch (node.kind) { - case SyntaxKind.Constructor: - // Get parameter properties, and treat them as being on the *same* level as the constructor, not under it. - const ctr = node; - addNodeWithRecursiveChild(ctr, ctr.body); + if (!node || isToken(node)) { + return; + } - // Parameter properties are children of the class, not the constructor. - for (const param of ctr.parameters) { - if (isParameterPropertyDeclaration(param)) { - addLeafNode(param); + switch (node.kind) { + case SyntaxKind.Constructor: + // Get parameter properties, and treat them as being on the *same* level as the constructor, not under it. + const ctr = node; + addNodeWithRecursiveChild(ctr, ctr.body, addChildrenRecursively); + + // Parameter properties are children of the class, not the constructor. + for (const param of ctr.parameters) { + if (isParameterPropertyDeclaration(param)) { + addLeafNode(param); + } } - } - break; + break; - case SyntaxKind.MethodDeclaration: - case SyntaxKind.GetAccessor: - case SyntaxKind.SetAccessor: - case SyntaxKind.MethodSignature: - if (!hasDynamicName((node))) { - addNodeWithRecursiveChild(node, (node).body); - } - break; + case SyntaxKind.MethodDeclaration: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + case SyntaxKind.MethodSignature: + if (!hasDynamicName((node))) { + addNodeWithRecursiveChild(node, (node).body, addChildrenRecursively); + } + break; - case SyntaxKind.PropertyDeclaration: - case SyntaxKind.PropertySignature: - if (!hasDynamicName((node))) { - addLeafNode(node); - } - break; + case SyntaxKind.PropertyDeclaration: + case SyntaxKind.PropertySignature: + if (!hasDynamicName((node))) { + addLeafNode(node); + } + break; - case SyntaxKind.ImportClause: - const importClause = node; - // Handle default import case e.g.: - // import d from "mod"; - if (importClause.name) { - addLeafNode(importClause); - } + case SyntaxKind.ImportClause: + const importClause = node; + // Handle default import case e.g.: + // import d from "mod"; + if (importClause.name) { + addLeafNode(importClause); + } - // Handle named bindings in imports e.g.: - // import * as NS from "mod"; - // import {a, b as B} from "mod"; - const {namedBindings} = importClause; - if (namedBindings) { - if (namedBindings.kind === SyntaxKind.NamespaceImport) { - addLeafNode(namedBindings); + // Handle named bindings in imports e.g.: + // import * as NS from "mod"; + // import {a, b as B} from "mod"; + const {namedBindings} = importClause; + if (namedBindings) { + if (namedBindings.kind === SyntaxKind.NamespaceImport) { + addLeafNode(namedBindings); + } + else { + for (const element of (namedBindings).elements) { + addLeafNode(element); + } + } + } + break; + + case SyntaxKind.BindingElement: + case SyntaxKind.VariableDeclaration: + const decl = node; + const name = decl.name; + if (isBindingPattern(name)) { + addChildrenRecursively(name); + } + else if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { + // For `const x = function() {}`, just use the function node, not the const. + addChildrenRecursively(decl.initializer); } else { - for (const element of (namedBindings).elements) { - addLeafNode(element); + addNodeWithRecursiveChild(decl, decl.initializer, addChildrenRecursively); + } + break; + + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.FunctionExpression: + addNodeWithRecursiveChild(node, (node).body, addChildrenRecursively); + break; + + case SyntaxKind.EnumDeclaration: + startNode(node); + for (const member of (node).members) { + if (!isComputedProperty(member)) { + addLeafNode(member); } } - } - break; + endNode(); + break; - case SyntaxKind.BindingElement: - case SyntaxKind.VariableDeclaration: - const decl = node; - const name = decl.name; - if (isBindingPattern(name)) { - addChildrenRecursively(name); - } - else if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { - // For `const x = function() {}`, just use the function node, not the const. - addChildrenRecursively(decl.initializer); - } - else { - addNodeWithRecursiveChild(decl, decl.initializer); - } - break; - - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.FunctionExpression: - addNodeWithRecursiveChild(node, (node).body); - break; - - case SyntaxKind.EnumDeclaration: - startNode(node); - for (const member of (node).members) { - if (!isComputedProperty(member)) { - addLeafNode(member); + case SyntaxKind.ClassDeclaration: + case SyntaxKind.ClassExpression: + case SyntaxKind.InterfaceDeclaration: + startNode(node); + for (const member of (node).members) { + addChildrenRecursively(member); } - } - endNode(); - break; + endNode(); + break; - case SyntaxKind.ClassDeclaration: - case SyntaxKind.ClassExpression: - case SyntaxKind.InterfaceDeclaration: - startNode(node); - for (const member of (node).members) { - addChildrenRecursively(member); - } - endNode(); - break; + case SyntaxKind.ModuleDeclaration: + addNodeWithRecursiveChild(node, getInteriorModule(node).body, addChildrenRecursively); + break; - case SyntaxKind.ModuleDeclaration: - addNodeWithRecursiveChild(node, getInteriorModule(node).body); - break; + case SyntaxKind.ExportSpecifier: + case SyntaxKind.ImportEqualsDeclaration: + case SyntaxKind.IndexSignature: + case SyntaxKind.CallSignature: + case SyntaxKind.ConstructSignature: + case SyntaxKind.TypeAliasDeclaration: + addLeafNode(node); + break; - case SyntaxKind.ExportSpecifier: - case SyntaxKind.ImportEqualsDeclaration: - case SyntaxKind.IndexSignature: - case SyntaxKind.CallSignature: - case SyntaxKind.ConstructSignature: - case SyntaxKind.TypeAliasDeclaration: - addLeafNode(node); - break; - - default: - forEach(node.jsDoc, jsDoc => { - forEach(jsDoc.tags, tag => { - if (tag.kind === SyntaxKind.JSDocTypedefTag) { - addLeafNode(tag); - } + default: + forEach(node.jsDoc, jsDoc => { + forEach(jsDoc.tags, tag => { + if (tag.kind === SyntaxKind.JSDocTypedefTag) { + addLeafNode(tag); + } + }); }); - }); - forEachChild(node, addChildrenRecursively); + forEachChild(node, addChildrenRecursively); + } } + + addChildrenRecursively(node); } /** Merge declarations of the same kind. */ diff --git a/src/services/outliningElementsCollector.ts b/src/services/outliningElementsCollector.ts index 2ad20a7ed0c..96a7c9a3c4a 100644 --- a/src/services/outliningElementsCollector.ts +++ b/src/services/outliningElementsCollector.ts @@ -1,6 +1,6 @@ /* @internal */ namespace ts.OutliningElementsCollector { - export function collectElements(sourceFile: SourceFile): OutliningSpan[] { + export function collectElements(sourceFile: SourceFile, cancellationToken: CancellationToken): OutliningSpan[] { const elements: OutliningSpan[] = []; const collapseText = "..."; @@ -38,6 +38,7 @@ namespace ts.OutliningElementsCollector { let singleLineCommentCount = 0; for (const currentComment of comments) { + cancellationToken.throwIfCancellationRequested(); // For single line comments, combine consecutive ones (2 or more) into // a single span from the start of the first till the end of the last @@ -84,6 +85,7 @@ namespace ts.OutliningElementsCollector { let depth = 0; const maxDepth = 20; function walk(n: Node): void { + cancellationToken.throwIfCancellationRequested(); if (depth > maxDepth) { return; } diff --git a/src/services/services.ts b/src/services/services.ts index c1b719d3477..cef307eb8fd 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1541,11 +1541,11 @@ namespace ts { } function getNavigationBarItems(fileName: string): NavigationBarItem[] { - return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName)); + return NavigationBar.getNavigationBarItems(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken); } function getNavigationTree(fileName: string): NavigationTree { - return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName)); + return NavigationBar.getNavigationTree(syntaxTreeCache.getCurrentSourceFile(fileName), cancellationToken); } function isTsOrTsxFile(fileName: string): boolean { @@ -1584,7 +1584,7 @@ namespace ts { function getOutliningSpans(fileName: string): OutliningSpan[] { // doesn't use compiler - no need to synchronize with host const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - return OutliningElementsCollector.collectElements(sourceFile); + return OutliningElementsCollector.collectElements(sourceFile, cancellationToken); } function getBraceMatchingAtPosition(fileName: string, position: number) {