From fc2424f97028c30179cefd1e92d9d809f2d31b3d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 8 May 2018 13:41:03 -0700 Subject: [PATCH] Fix debug failure from fixUnusedIdentifier --- src/services/codefixes/fixUnusedIdentifier.ts | 22 +++---------------- src/services/textChanges.ts | 4 ++++ ...xUnusedIdentifier_formatterDebugFailure.ts | 15 +++++++++++++ .../fourslash/unusedParameterInLambda1.ts | 3 +-- .../fourslash/unusedParameterInLambda2.ts | 3 +-- 5 files changed, 24 insertions(+), 23 deletions(-) create mode 100644 tests/cases/fourslash/codeFixUnusedIdentifier_formatterDebugFailure.ts diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index fd360c73ea1ad..426acca10ba6d 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -146,25 +146,9 @@ namespace ts.codefix { break; } - if (isArrowFunction(oldFunction) && oldFunction.parameters.length === 1) { - // 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, - 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); + if (isArrowFunction(oldFunction) && oldFunction.parameters.length === 1 && !findChildOfKind(oldFunction, SyntaxKind.OpenParenToken, sourceFile)) { + // `x => {}` becomes `() => {}` + changes.replaceNodeWithText(sourceFile, oldFunction.parameters[0], "()"); } else { changes.deleteNodeInList(sourceFile, parent); diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index eca1c639196e9..ff935e76d1878 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -315,6 +315,10 @@ namespace ts.textChanges { return this.replaceRangeWithNodes(sourceFile, getAdjustedRange(sourceFile, oldNode, oldNode, options), newNodes, options); } + public replaceNodeWithText(sourceFile: SourceFile, oldNode: Node, text: string, options: ChangeNodeOptions = useNonAdjustedPositions) { + return this.replaceRangeWithText(sourceFile, getAdjustedRange(sourceFile, oldNode, oldNode, options), 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); } diff --git a/tests/cases/fourslash/codeFixUnusedIdentifier_formatterDebugFailure.ts b/tests/cases/fourslash/codeFixUnusedIdentifier_formatterDebugFailure.ts new file mode 100644 index 0000000000000..cd7f46c2442a2 --- /dev/null +++ b/tests/cases/fourslash/codeFixUnusedIdentifier_formatterDebugFailure.ts @@ -0,0 +1,15 @@ +/// + +// Regression test for https://github.com/Microsoft/TypeScript/issues/23942 + +// @allowJs: true +// @target: esnext + +// @Filename: /a.js +////(o){" + +verify.codeFix({ + description: "Remove declaration for: 'o'", + index: 0, + newFileContent: '(){"', +}); diff --git a/tests/cases/fourslash/unusedParameterInLambda1.ts b/tests/cases/fourslash/unusedParameterInLambda1.ts index fdb53a8a05fea..60b7b3a99909d 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 e2b1be346b88e..b7c7da6dc2621 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*/", });