From 1c4590341f927cea82c4bfa9748a08f61431d5e2 Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 29 Oct 2018 16:40:30 -0700 Subject: [PATCH] Avoid reformatting body of arrow function with single unused parameter (#28217) --- src/services/textChanges.ts | 28 +++++++------------ .../codeFixUnusedIdentifier_all_delete.ts | 7 +++++ .../fourslash/unusedParameterInLambda1.ts | 3 +- .../fourslash/unusedParameterInLambda2.ts | 3 +- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index cf850cd63ab..47d8d0784e3 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -276,11 +276,15 @@ namespace ts.textChanges { return this.replaceRangeWithNodes(sourceFile, getAdjustedRange(sourceFile, oldNode, oldNode, options), newNodes, options); } + public replaceNodeWithText(sourceFile: SourceFile, oldNode: Node, text: string): void { + this.replaceRangeWithText(sourceFile, getAdjustedRange(sourceFile, oldNode, oldNode, useNonAdjustedPositions), text); + } + public replaceNodeRangeWithNodes(sourceFile: SourceFile, startNode: Node, endNode: Node, newNodes: ReadonlyArray, options: ReplaceWithMultipleNodesOptions & ConfigurableStartEnd = useNonAdjustedPositions) { return this.replaceRangeWithNodes(sourceFile, getAdjustedRange(sourceFile, startNode, endNode, options), newNodes, options); } - private nextCommaToken (sourceFile: SourceFile, node: Node): Node | undefined { + private nextCommaToken(sourceFile: SourceFile, node: Node): Node | undefined { const next = findNextToken(node, node.parent, sourceFile); return next && next.kind === SyntaxKind.CommaToken ? next : undefined; } @@ -690,7 +694,7 @@ namespace ts.textChanges { } private finishDeleteDeclarations(): void { - const deletedNodesInLists = new NodeSet(); // Stores ids of nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`. + const deletedNodesInLists = new NodeSet(); // Stores nodes in lists that we already deleted. Used to avoid deleting `, ` twice in `a, b`. for (const { sourceFile, node } of this.deletedNodes) { if (!this.deletedNodes.some(d => d.sourceFile === sourceFile && rangeContainsRangeExclusive(d.node, node))) { if (isArray(node)) { @@ -1053,25 +1057,13 @@ namespace ts.textChanges { switch (node.kind) { case SyntaxKind.Parameter: { const oldFunction = node.parent; - if (isArrowFunction(oldFunction) && oldFunction.parameters.length === 1) { + if (isArrowFunction(oldFunction) && + oldFunction.parameters.length === 1 && + !findChildOfKind(oldFunction, SyntaxKind.OpenParenToken, sourceFile)) { // Lambdas with exactly one parameter are special because, after removal, there // must be an empty parameter list (i.e. `()`) and this won't necessarily be the // case if the parameter is simply removed (e.g. in `x => 1`). - const newFunction = updateArrowFunction( - oldFunction, - oldFunction.modifiers, - oldFunction.typeParameters, - /*parameters*/ undefined!, // TODO: GH#18217 - oldFunction.type, - oldFunction.equalsGreaterThanToken, - oldFunction.body); - - // Drop leading and trailing trivia of the new function because we're only going - // to replace the span (vs the full span) of the old function - the old leading - // and trailing trivia will remain. - suppressLeadingAndTrailingTrivia(newFunction); - - changes.replaceNode(sourceFile, oldFunction, newFunction); + changes.replaceNodeWithText(sourceFile, node, "()"); } else { deleteNodeInList(changes, deletedNodesInLists, sourceFile, node); diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts index 2f6ef293521..b169ffb8848 100644 --- a/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_all_delete.ts @@ -31,6 +31,10 @@ ////takesCb((x, y) => { x; }); ////takesCb((x, y) => { y; }); //// +////x => { +//// const y = 0; +////}; +//// ////{ //// let a, b; ////} @@ -72,6 +76,9 @@ takesCb(() => {}); takesCb((x) => { x; }); takesCb((x, y) => { y; }); +() => { +}; + { } for (; ;) {} diff --git a/tests/cases/fourslash/unusedParameterInLambda1.ts b/tests/cases/fourslash/unusedParameterInLambda1.ts index fdb53a8a05f..60b7b3a9990 100644 --- a/tests/cases/fourslash/unusedParameterInLambda1.ts +++ b/tests/cases/fourslash/unusedParameterInLambda1.ts @@ -4,9 +4,8 @@ // @noUnusedParameters: true ////[|/*~a*/(/*~b*/x/*~c*/:/*~d*/number/*~e*/)/*~f*/ => /*~g*/{/*~h*/}/*~i*/|] -// In a perfect world, /*~f*/ and /*~h*/ would probably be retained. verify.codeFix({ description: "Remove declaration for: 'x'", index: 0, - newRangeContent: "/*~a*/() => /*~g*/ { }/*~i*/", + newRangeContent: "/*~a*/(/*~e*/)/*~f*/ => /*~g*/{/*~h*/}/*~i*/", }); diff --git a/tests/cases/fourslash/unusedParameterInLambda2.ts b/tests/cases/fourslash/unusedParameterInLambda2.ts index e2b1be346b8..b7c7da6dc26 100644 --- a/tests/cases/fourslash/unusedParameterInLambda2.ts +++ b/tests/cases/fourslash/unusedParameterInLambda2.ts @@ -4,9 +4,8 @@ // @noUnusedParameters: true ////[|/*~a*/x/*~b*/ /*~c*/=>/*~d*/ {/*~e*/}/*~f*/|] -// In a perfect world, /*~c*/ and /*~e*/ would probably be retained. verify.codeFix({ description: "Remove declaration for: 'x'", index: 0, - newRangeContent: "/*~a*/() => /*~d*/ { }/*~f*/", + newRangeContent: "/*~a*/()/*~b*/ /*~c*/=>/*~d*/ {/*~e*/}/*~f*/", });