From fed910fd0c9e911319dd492b51d410c1e655a090 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 18 Apr 2019 14:22:19 -0700 Subject: [PATCH] Add stop for JSDoc comments --- src/services/selectionRange.ts | 14 ++- .../unittests/tsserver/selectionRange.ts | 85 +++++++++++++------ 2 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/services/selectionRange.ts b/src/services/selectionRange.ts index 536efbfb8d1..1c2a525167a 100644 --- a/src/services/selectionRange.ts +++ b/src/services/selectionRange.ts @@ -17,7 +17,7 @@ namespace ts.SelectionRange { break outer; } - if (positionShouldSnapToNode(pos, node, nextNode, sourceFile)) { + if (positionShouldSnapToNode(pos, node, nextNode)) { // Blocks are effectively redundant with SyntaxLists. // TemplateSpans, along with the SyntaxLists containing them, // are a somewhat unintuitive grouping of things that should be @@ -45,8 +45,12 @@ namespace ts.SelectionRange { && isListOpener(prevNode) && isListCloser(nextNode) && !positionsAreOnSameLine(prevNode.getStart(), nextNode.getStart(), sourceFile); + const jsDocCommentStart = hasJSDocNodes(node) && node.jsDoc![0].getStart(); const start = isBetweenMultiLineBookends ? prevNode.getEnd() : node.getStart(); const end = isBetweenMultiLineBookends ? nextNode.getStart() : node.getEnd(); + if (isNumber(jsDocCommentStart)) { + pushSelectionRange(jsDocCommentStart, end); + } pushSelectionRange(start, end, node.kind); // String literals should have a stop both inside and outside their quotes. @@ -84,8 +88,12 @@ namespace ts.SelectionRange { * @param nextNode The next sibling node in the tree. * @param sourceFile The source file containing the nodes. */ - function positionShouldSnapToNode(pos: number, node: Node, nextNode: Node | undefined, sourceFile: SourceFile) { - if (positionBelongsToNode(node, pos, sourceFile)) { + function positionShouldSnapToNode(pos: number, node: Node, nextNode: Node | undefined) { + // Can’t use 'ts.positionBelongsToNode()' here because it cleverly accounts + // for missing nodes, which can’t really be considered when deciding what + // to select. + Debug.assert(node.pos <= pos); + if (pos < node.end) { return true; } const nodeEnd = node.getEnd(); diff --git a/src/testRunner/unittests/tsserver/selectionRange.ts b/src/testRunner/unittests/tsserver/selectionRange.ts index 759ae0dd254..3c0a12a58f7 100644 --- a/src/testRunner/unittests/tsserver/selectionRange.ts +++ b/src/testRunner/unittests/tsserver/selectionRange.ts @@ -92,44 +92,47 @@ export interface IService { _serviceBrand: any; open(host: number, data: any): Promise; + bar(): void }`); const locations = getSelectionRange([ - { - line: 5, - offset: 12, - }, + { line: 5, offset: 12 }, // ho/**/st + { line: 6, offset: 16 }, // void/**/ ]); - assert.deepEqual(locations, [ - { - textSpan: { // host + assert.deepEqual(locations![0], { + textSpan: { // host + start: { line: 5, offset: 10 }, + end: { line: 5, offset: 14 } }, + parent: { + textSpan: { // host: number start: { line: 5, offset: 10 }, - end: { line: 5, offset: 14 } }, + end: { line: 5, offset: 22 } }, parent: { - textSpan: { // host: number + textSpan: { // host: number, data: any start: { line: 5, offset: 10 }, - end: { line: 5, offset: 22 } }, + end: { line: 5, offset: 33 } }, parent: { - textSpan: { // host: number, data: any - start: { line: 5, offset: 10 }, - end: { line: 5, offset: 33 } }, + textSpan: { // open(host: number, data: any): Promise; + start: { line: 5, offset: 5 }, + end: { line: 5, offset: 49 } }, parent: { - textSpan: { // open(host: number, data: any): Promise; - start: { line: 5, offset: 5 }, - end: { line: 5, offset: 49 } }, + textSpan: { // SyntaxList + whitespace (body of interface) + start: { line: 2, offset: 28 }, + end: { line: 7, offset: 1 } }, parent: { - textSpan: { // SyntaxList + whitespace (body of interface) - start: { line: 2, offset: 28 }, - end: { line: 6, offset: 1 } }, + textSpan: { // InterfaceDeclaration + start: { line: 2, offset: 1 }, + end: { line: 7, offset: 2 } }, parent: { - textSpan: { // InterfaceDeclaration - start: { line: 2, offset: 1 }, - end: { line: 6, offset: 2 } }, - parent: { - textSpan: { // SourceFile - start: { line: 1, offset: 1 }, - end: { line: 6, offset: 2 } } } } } } } } }, - ]); + textSpan: { // SourceFile + start: { line: 1, offset: 1 }, + end: { line: 7, offset: 2 } } } } } } } } }); + + // Ensures positions after a zero-width node work, because ts.positionBelongsToNode + // treats them strangely. + assert.deepEqual(locations![1].textSpan, { // void + start: { line: 6, offset: 12 }, + end: { line: 6, offset: 16 }}); }); it("works for complex TypeScript", () => { @@ -613,5 +616,33 @@ function f( end: { line: 1, offset: 4 }, }); }); + + it("creates a stop for JSDoc ranges", () => { + const getSelectionRange = setup("/file.js", "" + +`// Not a JSDoc comment +/** + * @param {number} x The number to square + */ +function square(x) { + return x * x; +}`); + const locations = getSelectionRange([{ line: 5, offset: 10 }]); // square(x) + assert.deepEqual(locations, [{ + textSpan: { // square + start: { line: 5 , offset: 10 }, + end: { line: 5, offset: 16 } }, + parent: { // whole function declaration + textSpan: { + start: { line: 5, offset: 1 }, + end: { line: 7, offset: 2 } }, + parent: { + textSpan: { // add JSDoc + start: { line: 2, offset: 1 }, + end: { line: 7, offset: 2 } }, + parent: { + textSpan: { // SourceFile + start: { line: 1, offset: 1 }, + end: { line: 7, offset: 2 } } } } } }]); + }); }); }