From d73eabd35ac852a7fd7532cafc8779bd0b128584 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 16 Apr 2019 11:23:57 -0700 Subject: [PATCH] Special rules for binding elements, extend brace/whitespace logic to other kinds of bookended lists --- src/services/selectionRange.ts | 44 +++++++++--- .../unittests/tsserver/selectionRange.ts | 72 +++++++++++++++++++ 2 files changed, 107 insertions(+), 9 deletions(-) diff --git a/src/services/selectionRange.ts b/src/services/selectionRange.ts index 07a7abd7d26..536efbfb8d1 100644 --- a/src/services/selectionRange.ts +++ b/src/services/selectionRange.ts @@ -21,8 +21,13 @@ namespace ts.SelectionRange { // Blocks are effectively redundant with SyntaxLists. // TemplateSpans, along with the SyntaxLists containing them, // are a somewhat unintuitive grouping of things that should be - // considered independently. Dive in without pushing a selection range. - if (isBlock(node) || isTemplateSpan(node) || isTemplateHead(node) || prevNode && isTemplateHead(prevNode)) { + // considered independently. A VariableStatement’s children are just + // a VaraiableDeclarationList and a semicolon. + // Dive in without pushing a selection range. + if (isBlock(node) + || isTemplateSpan(node) || isTemplateHead(node) + || prevNode && isTemplateHead(prevNode) + || isVariableDeclarationList(node) && isVariableStatement(parentNode)) { parentNode = node; break; } @@ -34,14 +39,14 @@ namespace ts.SelectionRange { pushSelectionRange(start, end, node.kind); } - // Blocks with braces on separate lines should be selected from brace to brace, - // including whitespace but not including the braces themselves. - const isBetweenMultiLineBraces = isSyntaxList(node) - && prevNode && prevNode.kind === SyntaxKind.OpenBraceToken - && nextNode && nextNode.kind === SyntaxKind.CloseBraceToken + // Blocks with braces, brackets, parens, or JSX tags on separate lines should be + // selected from open to close, including whitespace but not including the braces/etc. themselves. + const isBetweenMultiLineBookends = isSyntaxList(node) + && isListOpener(prevNode) + && isListCloser(nextNode) && !positionsAreOnSameLine(prevNode.getStart(), nextNode.getStart(), sourceFile); - const start = isBetweenMultiLineBraces ? prevNode.getEnd() : node.getStart(); - const end = isBetweenMultiLineBraces ? nextNode.getStart() : node.getEnd(); + const start = isBetweenMultiLineBookends ? prevNode.getEnd() : node.getStart(); + const end = isBetweenMultiLineBookends ? nextNode.getStart() : node.getEnd(); pushSelectionRange(start, end, node.kind); // String literals should have a stop both inside and outside their quotes. @@ -156,6 +161,11 @@ namespace ts.SelectionRange { return splitChildren(groupedWithQuestionToken, ({ kind }) => kind === SyntaxKind.EqualsToken); } + // Pivot on '=' + if (isBindingElement(node)) { + return splitChildren(node.getChildren(), ({ kind }) => kind === SyntaxKind.EqualsToken); + } + return node.getChildren(); } @@ -226,4 +236,20 @@ namespace ts.SelectionRange { syntaxList._children = children; return syntaxList; } + + function isListOpener(token: Node | undefined): token is Node { + const kind = token && token.kind; + return kind === SyntaxKind.OpenBraceToken + || kind === SyntaxKind.OpenBracketToken + || kind === SyntaxKind.OpenParenToken + || kind === SyntaxKind.JsxOpeningElement; + } + + function isListCloser(token: Node | undefined): token is Node { + const kind = token && token.kind; + return kind === SyntaxKind.CloseBraceToken + || kind === SyntaxKind.CloseBracketToken + || kind === SyntaxKind.CloseParenToken + || kind === SyntaxKind.JsxClosingElement; + } } diff --git a/src/testRunner/unittests/tsserver/selectionRange.ts b/src/testRunner/unittests/tsserver/selectionRange.ts index 5a4bbb41f19..759ae0dd254 100644 --- a/src/testRunner/unittests/tsserver/selectionRange.ts +++ b/src/testRunner/unittests/tsserver/selectionRange.ts @@ -533,6 +533,78 @@ function f(p, q?, ...r: any[] = []) {}`); }); }); + it("works for binding elements", () => { + const getSelectionRange = setup("/file.ts", ` +const { x, y: a, ...zs = {} } = {};`); + const locations = getSelectionRange([ + { line: 2, offset: 9 }, // x + { line: 2, offset: 15 }, // a + { line: 2, offset: 21 }, // zs + ]); + + // Don’t care about checking first two locations, because + // they’re pretty boring, just want to make sure they don’t cause a crash + assert.deepEqual(locations![2], { + textSpan: { // zs + start: { line: 2, offset: 21 }, + end: { line: 2, offset: 23 } }, + parent: { + textSpan: { // ...zs + start: { line: 2, offset: 18 }, + end: { line: 2, offset: 23 } }, + parent: { + textSpan: { // ...zs = {} + start: { line: 2, offset: 18 }, + end: { line: 2, offset: 28 } }, + parent: { + textSpan: { // x, y: a, ...zs = {} + start: { line: 2, offset: 9 }, + end: { line: 2, offset: 28 } }, + parent: { + textSpan: { // { x, y: a, ...zs = {} } + start: { line: 2, offset: 7 }, + end: { line: 2, offset: 30 } }, + parent: { + textSpan: { // { x, y: a, ...zs = {} } = {} + start: { line: 2, offset: 7 }, + end: { line: 2, offset: 35 } }, + parent: { + textSpan: { // whole line + start: { line: 2, offset: 1 }, + end: { line: 2, offset: 36 } }, + parent: { + textSpan: { + start: { line: 1, offset: 1 }, + end: { line: 2, offset: 36 } } } } } } } } }, + }); + }); + + it("consumes all whitespace in a multi-line function parameter list", () => { + const getSelectionRange = setup("/file.ts", ` +function f( + a, + b +) {}`); + const locations = getSelectionRange([{ line: 4, offset: 5 }]); // b + assert.deepEqual(locations, [{ + textSpan: { // b + start: { line: 4, offset: 5 }, + end: { line: 4, offset: 6 } }, + parent: { // all params and whitespace inside parens + textSpan: { + start: { line: 2, offset: 12 }, + end: { line: 5, offset: 1 } }, + parent: { + textSpan: { // whole function declaration + start: { line: 2, offset: 1 }, + end: { line: 5, offset: 5 } }, + parent: { + textSpan: { // SourceFile + start: { line: 1, offset: 1 }, + end: { line: 5, offset: 5 } } } } } + }]); + }); + it("snaps to nodes directly behind the cursor instead of trivia ahead of the cursor", () => { const getSelectionRange = setup("/file.ts", `let x: string`); const locations = getSelectionRange([{ line: 1, offset: 4 }]);