From 5a69d9c2550a3bdc8f7b92b688388c5a5cce80d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Mon, 16 Apr 2018 14:31:37 +0800 Subject: [PATCH 01/12] add support for add or remove braces to arrow function --- src/compiler/diagnosticMessages.json | 12 +++ src/harness/tsconfig.json | 1 + src/server/tsconfig.json | 1 + src/server/tsconfig.library.json | 1 + .../refactors/convertArrowFunction.ts | 90 +++++++++++++++++++ src/services/tsconfig.json | 1 + .../refactorAddBracesToArrowFunction1.ts | 11 +++ .../refactorAddBracesToArrowFunction2.ts | 11 +++ .../refactorAddBracesToArrowFunction3.ts | 11 +++ .../refactorAddBracesToArrowFunction4.ts | 11 +++ .../refactorAddBracesToArrowFunction5.ts | 11 +++ .../refactorAddBracesToArrowFunction6.ts | 11 +++ .../refactorAddBracesToArrowFunction7.ts | 6 ++ .../refactorAddBracesToArrowFunction8.ts | 9 ++ 14 files changed, 187 insertions(+) create mode 100644 src/services/refactors/convertArrowFunction.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts create mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 59007e610b..6660b29fd6 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4280,5 +4280,17 @@ "Remove all unused labels": { "category": "Message", "code": 95054 + }, + "Convert arrow function": { + "category": "Message", + "code": 95055 + }, + "Add braces to arrow function": { + "category": "Message", + "code": 95056 + }, + "Remove braces from arrow function": { + "category": "Message", + "code": 95057 } } diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json index 359701a02c..4749520822 100644 --- a/src/harness/tsconfig.json +++ b/src/harness/tsconfig.json @@ -121,6 +121,7 @@ "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", "../services/refactors/moveToNewFile.ts", + "../services/refactors/convertArrowFunction.ts", "../services/sourcemaps.ts", "../services/services.ts", "../services/breakpoints.ts", diff --git a/src/server/tsconfig.json b/src/server/tsconfig.json index 8ae6974baf..7d81e06864 100644 --- a/src/server/tsconfig.json +++ b/src/server/tsconfig.json @@ -117,6 +117,7 @@ "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", "../services/refactors/moveToNewFile.ts", + "../services/refactors/convertArrowFunction.ts", "../services/sourcemaps.ts", "../services/services.ts", "../services/breakpoints.ts", diff --git a/src/server/tsconfig.library.json b/src/server/tsconfig.library.json index 922af11e87..df72b37e53 100644 --- a/src/server/tsconfig.library.json +++ b/src/server/tsconfig.library.json @@ -123,6 +123,7 @@ "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", "../services/refactors/moveToNewFile.ts", + "../services/refactors/convertArrowFunction.ts", "../services/sourcemaps.ts", "../services/services.ts", "../services/breakpoints.ts", diff --git a/src/services/refactors/convertArrowFunction.ts b/src/services/refactors/convertArrowFunction.ts new file mode 100644 index 0000000000..af1d232ad3 --- /dev/null +++ b/src/services/refactors/convertArrowFunction.ts @@ -0,0 +1,90 @@ +/* @internal */ +namespace ts.refactor.convertArrowFunction { + const refactorName = "Convert arrow function"; + const refactorDescription = Diagnostics.Convert_arrow_function.message; + const addBracesActionName = "Add braces to arrow function"; + const removeBracesActionName = "Remove braces from arrow function"; + const addBracesActionDescription = Diagnostics.Add_braces_to_arrow_function.message; + const removeBracesActionDescription = Diagnostics.Remove_braces_from_arrow_function.message; + registerRefactor(refactorName, { getEditsForAction, getAvailableActions }); + + interface Info { + container: ArrowFunction; + expression: Expression; + addBraces: boolean; + } + + function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined { + const { file, startPosition } = context; + const info = getConvertibleArrowFunctionAtPosition(file, startPosition); + if (!info) return undefined; + + const actions: RefactorActionInfo[] = [ + info.addBraces ? + { + name: addBracesActionName, + description: addBracesActionDescription + } : { + name: removeBracesActionName, + description: removeBracesActionDescription + } + ]; + + return [{ + name: refactorName, + description: refactorDescription, + actions + }]; + } + + function getEditsForAction(context: RefactorContext, _actionName: string): RefactorEditInfo | undefined { + const { file, startPosition } = context; + const info = getConvertibleArrowFunctionAtPosition(file, startPosition); + if (!info) return undefined; + + const { addBraces, expression, container } = info; + const changeTracker = textChanges.ChangeTracker.fromContext(context); + updateBraces(changeTracker, file, container, expression, addBraces); + + return { + renameFilename: undefined, + renameLocation: undefined, + edits: changeTracker.getChanges() + }; + } + + function updateBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, expression: Expression, addBraces: boolean) { + const body = addBraces ? createBlock([createReturn(expression)]) : expression; + + const arrowFunction = updateArrowFunction( + container, + container.modifiers, + container.typeParameters, + container.parameters, + container.type, + body); + changeTracker.replaceNode(file, container, arrowFunction); + } + + function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined { + const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); + const container = getContainingFunction(node); + if (!container || !isArrowFunction(container)) return undefined; + + if (isExpression(container.body)) { + return { + container, + addBraces: true, + expression: container.body + }; + } + else if (container.body.statements.length === 1 && isReturnStatement(first(container.body.statements))) { + return { + container, + addBraces: false, + expression: (first(container.body.statements)).expression + }; + } + return undefined; + } +} diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 7e1ccc9c3a..f425c569e3 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -114,6 +114,7 @@ "refactors/extractSymbol.ts", "refactors/generateGetAccessorAndSetAccessor.ts", "refactors/moveToNewFile.ts", + "refactors/convertArrowFunction.ts", "sourcemaps.ts", "services.ts", "breakpoints.ts", diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts new file mode 100644 index 0000000000..fde5a54656 --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => a + 1; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert arrow function", + actionName: "Add braces to arrow function", + actionDescription: "Add braces to arrow function", + newContent: `const foo = a => { return a + 1; };`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts new file mode 100644 index 0000000000..30f4de0e9a --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => ({ a: 1 }); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert arrow function", + actionName: "Add braces to arrow function", + actionDescription: "Add braces to arrow function", + newContent: `const foo = a => { return ({ a: 1 }); };`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts new file mode 100644 index 0000000000..cd2d773449 --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => 1; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert arrow function", + actionName: "Add braces to arrow function", + actionDescription: "Add braces to arrow function", + newContent: `const foo = a => { return 1; };`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts new file mode 100644 index 0000000000..f113d63add --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return a + 1; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => a + 1;`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts new file mode 100644 index 0000000000..bc0cdfd648 --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return { a: 1 }; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => ({ a: 1 });`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts new file mode 100644 index 0000000000..999fba0519 --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return 1; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => 1;`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts new file mode 100644 index 0000000000..7f7a520751 --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts @@ -0,0 +1,6 @@ +/// + +//// const foo = /*a*/a/*b*/ => { }; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Convert arrow function"); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts new file mode 100644 index 0000000000..1c7f0bd954 --- /dev/null +++ b/tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts @@ -0,0 +1,9 @@ +/// + +//// const foo = /*a*/a/*b*/ => { +//// const b = 1; +//// return a + b; +//// }; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Convert arrow function"); From 32be0c70993dbf36de3cb6166f3ed724c345fa4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Tue, 17 Apr 2018 10:33:21 +0800 Subject: [PATCH 02/12] add tests and fix --- src/compiler/diagnosticMessages.json | 2 +- src/harness/tsconfig.json | 2 +- src/server/tsconfig.json | 2 +- src/server/tsconfig.library.json | 2 +- ...ts => addOrRemoveBracesToArrowFunction.ts} | 69 ++++++++++++------- src/services/tsconfig.json | 2 +- .../refactorAddBracesToArrowFunction7.ts | 6 -- ...actorAddOrRemoveBracesToArrowFunction1.ts} | 2 +- ...actorAddOrRemoveBracesToArrowFunction10.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction11.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction12.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction13.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction14.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction15.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction16.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction17.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction18.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction19.ts | 11 +++ ...actorAddOrRemoveBracesToArrowFunction2.ts} | 2 +- ...actorAddOrRemoveBracesToArrowFunction3.ts} | 2 +- ...actorAddOrRemoveBracesToArrowFunction4.ts} | 4 +- ...actorAddOrRemoveBracesToArrowFunction5.ts} | 2 +- ...actorAddOrRemoveBracesToArrowFunction6.ts} | 2 +- ...actorAddOrRemoveBracesToArrowFunction8.ts} | 2 +- ...factorAddOrRemoveBracesToArrowFunction9.ts | 11 +++ 25 files changed, 178 insertions(+), 44 deletions(-) rename src/services/refactors/{convertArrowFunction.ts => addOrRemoveBracesToArrowFunction.ts} (53%) delete mode 100644 tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts rename tests/cases/fourslash/{refactorAddBracesToArrowFunction1.ts => refactorAddOrRemoveBracesToArrowFunction1.ts} (81%) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction10.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction11.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction12.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction13.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction14.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction16.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction17.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction18.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction19.ts rename tests/cases/fourslash/{refactorAddBracesToArrowFunction2.ts => refactorAddOrRemoveBracesToArrowFunction2.ts} (82%) rename tests/cases/fourslash/{refactorAddBracesToArrowFunction3.ts => refactorAddOrRemoveBracesToArrowFunction3.ts} (81%) rename tests/cases/fourslash/{refactorAddBracesToArrowFunction4.ts => refactorAddOrRemoveBracesToArrowFunction4.ts} (70%) rename tests/cases/fourslash/{refactorAddBracesToArrowFunction5.ts => refactorAddOrRemoveBracesToArrowFunction5.ts} (82%) rename tests/cases/fourslash/{refactorAddBracesToArrowFunction6.ts => refactorAddOrRemoveBracesToArrowFunction6.ts} (82%) rename tests/cases/fourslash/{refactorAddBracesToArrowFunction8.ts => refactorAddOrRemoveBracesToArrowFunction8.ts} (66%) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 6660b29fd6..b5cb450bfe 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -4281,7 +4281,7 @@ "category": "Message", "code": 95054 }, - "Convert arrow function": { + "Add or remove braces in an arrow function": { "category": "Message", "code": 95055 }, diff --git a/src/harness/tsconfig.json b/src/harness/tsconfig.json index 4749520822..22b7f49670 100644 --- a/src/harness/tsconfig.json +++ b/src/harness/tsconfig.json @@ -121,7 +121,7 @@ "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", "../services/refactors/moveToNewFile.ts", - "../services/refactors/convertArrowFunction.ts", + "../services/refactors/addOrRemoveBracesToArrowFunction.ts", "../services/sourcemaps.ts", "../services/services.ts", "../services/breakpoints.ts", diff --git a/src/server/tsconfig.json b/src/server/tsconfig.json index 7d81e06864..6c5144093a 100644 --- a/src/server/tsconfig.json +++ b/src/server/tsconfig.json @@ -117,7 +117,7 @@ "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", "../services/refactors/moveToNewFile.ts", - "../services/refactors/convertArrowFunction.ts", + "../services/refactors/addOrRemoveBracesToArrowFunction.ts", "../services/sourcemaps.ts", "../services/services.ts", "../services/breakpoints.ts", diff --git a/src/server/tsconfig.library.json b/src/server/tsconfig.library.json index df72b37e53..59ff8cc9e8 100644 --- a/src/server/tsconfig.library.json +++ b/src/server/tsconfig.library.json @@ -123,7 +123,7 @@ "../services/refactors/extractSymbol.ts", "../services/refactors/generateGetAccessorAndSetAccessor.ts", "../services/refactors/moveToNewFile.ts", - "../services/refactors/convertArrowFunction.ts", + "../services/refactors/addOrRemoveBracesToArrowFunction.ts", "../services/sourcemaps.ts", "../services/services.ts", "../services/breakpoints.ts", diff --git a/src/services/refactors/convertArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts similarity index 53% rename from src/services/refactors/convertArrowFunction.ts rename to src/services/refactors/addOrRemoveBracesToArrowFunction.ts index af1d232ad3..df7cacadf3 100644 --- a/src/services/refactors/convertArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -1,7 +1,7 @@ /* @internal */ -namespace ts.refactor.convertArrowFunction { - const refactorName = "Convert arrow function"; - const refactorDescription = Diagnostics.Convert_arrow_function.message; +namespace ts.refactor.addOrRemoveBracesToArrowFunction { + const refactorName = "Add or remove braces in an arrow function"; + const refactorDescription = Diagnostics.Add_or_remove_braces_in_an_arrow_function.message; const addBracesActionName = "Add braces to arrow function"; const removeBracesActionName = "Remove braces from arrow function"; const addBracesActionDescription = Diagnostics.Add_braces_to_arrow_function.message; @@ -19,21 +19,19 @@ namespace ts.refactor.convertArrowFunction { const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return undefined; - const actions: RefactorActionInfo[] = [ - info.addBraces ? - { - name: addBracesActionName, - description: addBracesActionDescription - } : { - name: removeBracesActionName, - description: removeBracesActionDescription - } - ]; - return [{ name: refactorName, description: refactorDescription, - actions + actions: [ + info.addBraces ? + { + name: addBracesActionName, + description: addBracesActionDescription + } : { + name: removeBracesActionName, + description: removeBracesActionDescription + } + ] }]; } @@ -42,9 +40,18 @@ namespace ts.refactor.convertArrowFunction { const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return undefined; - const { addBraces, expression, container } = info; + const { expression, container } = info; const changeTracker = textChanges.ChangeTracker.fromContext(context); - updateBraces(changeTracker, file, container, expression, addBraces); + + if (_actionName === addBracesActionName) { + addBraces(changeTracker, file, container, expression); + } + else if (_actionName === removeBracesActionName) { + removeBraces(changeTracker, file, container, expression); + } + else { + Debug.fail("invalid action"); + } return { renameFilename: undefined, @@ -53,9 +60,18 @@ namespace ts.refactor.convertArrowFunction { }; } - function updateBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, expression: Expression, addBraces: boolean) { - const body = addBraces ? createBlock([createReturn(expression)]) : expression; + function addBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, expression: Expression) { + updateBraces(changeTracker, file, container, createBlock([createReturn(expression)])); + } + function removeBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, expression: Expression) { + if (!isLiteralExpression(expression) && !isIdentifier(expression) && !isParenthesizedExpression(expression) && expression.kind !== SyntaxKind.NullKeyword) { + expression = createParen(expression); + } + updateBraces(changeTracker, file, container, expression); + } + + function updateBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, body: ConciseBody) { const arrowFunction = updateArrowFunction( container, container.modifiers, @@ -78,12 +94,15 @@ namespace ts.refactor.convertArrowFunction { expression: container.body }; } - else if (container.body.statements.length === 1 && isReturnStatement(first(container.body.statements))) { - return { - container, - addBraces: false, - expression: (first(container.body.statements)).expression - }; + else if (container.body.statements.length === 1) { + const firstStatement = first(container.body.statements); + if (isReturnStatement(firstStatement)) { + return { + container, + addBraces: false, + expression: firstStatement.expression + }; + } } return undefined; } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index f425c569e3..4d94facef3 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -114,7 +114,7 @@ "refactors/extractSymbol.ts", "refactors/generateGetAccessorAndSetAccessor.ts", "refactors/moveToNewFile.ts", - "refactors/convertArrowFunction.ts", + "refactors/addOrRemoveBracesToArrowFunction.ts", "sourcemaps.ts", "services.ts", "breakpoints.ts", diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts b/tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts deleted file mode 100644 index 7f7a520751..0000000000 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction7.ts +++ /dev/null @@ -1,6 +0,0 @@ -/// - -//// const foo = /*a*/a/*b*/ => { }; - -goTo.select("a", "b"); -verify.not.refactorAvailable("Convert arrow function"); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts similarity index 81% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts index fde5a54656..1d8d52e201 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction1.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts @@ -4,7 +4,7 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Convert arrow function", + refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", newContent: `const foo = a => { return a + 1; };`, diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction10.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction10.ts new file mode 100644 index 0000000000..373172a2b3 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction10.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return (1, 2, 3); }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => (1, 2, 3);`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction11.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction11.ts new file mode 100644 index 0000000000..c634db625c --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction11.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return 1, 2, 3; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => (1, 2, 3);`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction12.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction12.ts new file mode 100644 index 0000000000..aecf37c8b5 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction12.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return "foo"; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => "foo";`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction13.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction13.ts new file mode 100644 index 0000000000..64d1a02db6 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction13.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return null; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => null;`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction14.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction14.ts new file mode 100644 index 0000000000..b15f1cfad3 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction14.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return undefined; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => undefined;`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts new file mode 100644 index 0000000000..af2927f8f9 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return void 0; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => (void 0);`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction16.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction16.ts new file mode 100644 index 0000000000..87bdcfe575 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction16.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return {}; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => ({});`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction17.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction17.ts new file mode 100644 index 0000000000..e71809ac38 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction17.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return `abc{a}`; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => \`abc{a}\`;`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction18.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction18.ts new file mode 100644 index 0000000000..0e2d85ddd2 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction18.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return `abc`; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => \`abc\`;`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction19.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction19.ts new file mode 100644 index 0000000000..9d56a4d5f6 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction19.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return a; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => a;`, +}); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts similarity index 82% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts index 30f4de0e9a..d875018c39 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction2.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts @@ -4,7 +4,7 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Convert arrow function", + refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", newContent: `const foo = a => { return ({ a: 1 }); };`, diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts similarity index 81% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts index cd2d773449..ead5867a17 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction3.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts @@ -4,7 +4,7 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Convert arrow function", + refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", newContent: `const foo = a => { return 1; };`, diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts similarity index 70% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts index f113d63add..a2147af1d4 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction4.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts @@ -4,8 +4,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Convert arrow function", + refactorName: "Add or remove braces in an arrow function", actionName: "Remove braces from arrow function", actionDescription: "Remove braces from arrow function", - newContent: `const foo = a => a + 1;`, + newContent: `const foo = a => (a + 1);`, }); diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction5.ts similarity index 82% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction5.ts index bc0cdfd648..355f3a8f33 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction5.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction5.ts @@ -4,7 +4,7 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Convert arrow function", + refactorName: "Add or remove braces in an arrow function", actionName: "Remove braces from arrow function", actionDescription: "Remove braces from arrow function", newContent: `const foo = a => ({ a: 1 });`, diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction6.ts similarity index 82% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction6.ts index 999fba0519..31f865b439 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction6.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction6.ts @@ -4,7 +4,7 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Convert arrow function", + refactorName: "Add or remove braces in an arrow function", actionName: "Remove braces from arrow function", actionDescription: "Remove braces from arrow function", newContent: `const foo = a => 1;`, diff --git a/tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction8.ts similarity index 66% rename from tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts rename to tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction8.ts index 1c7f0bd954..a2ec70e1e8 100644 --- a/tests/cases/fourslash/refactorAddBracesToArrowFunction8.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction8.ts @@ -6,4 +6,4 @@ //// }; goTo.select("a", "b"); -verify.not.refactorAvailable("Convert arrow function"); +verify.not.refactorAvailable("Add or remove braces in an arrow function"); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts new file mode 100644 index 0000000000..610e19cff4 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => (1, 2, 3); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Add braces to arrow function", + actionDescription: "Add braces to arrow function", + newContent: `const foo = a => { return (1, 2, 3); };`, +}); From bd9a8b5c0cad409085583b07edcdf234319f805f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 18 Apr 2018 17:34:30 +0800 Subject: [PATCH 03/12] stash --- .../codefixes/convertFunctionToEs6Class.ts | 16 ------ .../addOrRemoveBracesToArrowFunction.ts | 55 ++++++++++--------- src/services/utilities.ts | 16 ++++++ ...actorAddOrRemoveBracesToArrowFunction15.ts | 2 +- ...actorAddOrRemoveBracesToArrowFunction20.ts | 14 +++++ ...actorAddOrRemoveBracesToArrowFunction21.ts | 11 ++++ ...factorAddOrRemoveBracesToArrowFunction4.ts | 2 +- 7 files changed, 72 insertions(+), 44 deletions(-) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts diff --git a/src/services/codefixes/convertFunctionToEs6Class.ts b/src/services/codefixes/convertFunctionToEs6Class.ts index cd1baeb14a..4ce540595e 100644 --- a/src/services/codefixes/convertFunctionToEs6Class.ts +++ b/src/services/codefixes/convertFunctionToEs6Class.ts @@ -202,22 +202,6 @@ namespace ts.codefix { } } - function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile) { - forEachLeadingCommentRange(sourceFile.text, sourceNode.pos, (pos, end, kind, htnl) => { - if (kind === SyntaxKind.MultiLineCommentTrivia) { - // Remove leading /* - pos += 2; - // Remove trailing */ - end -= 2; - } - else { - // Remove leading // - pos += 2; - } - addSyntheticLeadingComment(targetNode, kind, sourceFile.text.slice(pos, end), htnl); - }); - } - function getModifierKindFromSource(source: Node, kind: SyntaxKind): ReadonlyArray | undefined { return filter(source.modifiers, modifier => modifier.kind === kind); } diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index df7cacadf3..e6fed3ebaf 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -9,7 +9,7 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { registerRefactor(refactorName, { getEditsForAction, getAvailableActions }); interface Info { - container: ArrowFunction; + func: ArrowFunction; expression: Expression; addBraces: boolean; } @@ -35,24 +35,31 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { }]; } - function getEditsForAction(context: RefactorContext, _actionName: string): RefactorEditInfo | undefined { + function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { const { file, startPosition } = context; const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return undefined; - const { expression, container } = info; + const { expression, func } = info; const changeTracker = textChanges.ChangeTracker.fromContext(context); - if (_actionName === addBracesActionName) { - addBraces(changeTracker, file, container, expression); + let body: ConciseBody; + if (actionName === addBracesActionName) { + const returnStatement = createReturn(expression); + body = createBlock([returnStatement]); + copyComments(expression, returnStatement, file, SyntaxKind.SingleLineCommentTrivia, true); } - else if (_actionName === removeBracesActionName) { - removeBraces(changeTracker, file, container, expression); + else if (actionName === removeBracesActionName) { + const returnStatement = expression.parent; + body = needsParentheses(expression) ? createParen(expression) : expression; + copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, false); } else { - Debug.fail("invalid action"); + Debug.fail('invalid action'); } + updateBody(changeTracker, file, func, body); + return { renameFilename: undefined, renameLocation: undefined, @@ -60,18 +67,13 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { }; } - function addBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, expression: Expression) { - updateBraces(changeTracker, file, container, createBlock([createReturn(expression)])); + function needsParentheses(expression: Expression) { + if (isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.CommaToken) return true; + if (isObjectLiteralExpression(expression)) return true; + return false; } - function removeBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, expression: Expression) { - if (!isLiteralExpression(expression) && !isIdentifier(expression) && !isParenthesizedExpression(expression) && expression.kind !== SyntaxKind.NullKeyword) { - expression = createParen(expression); - } - updateBraces(changeTracker, file, container, expression); - } - - function updateBraces(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, body: ConciseBody) { + function updateBody(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, body: ConciseBody) { const arrowFunction = updateArrowFunction( container, container.modifiers, @@ -84,21 +86,22 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined { const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); - const container = getContainingFunction(node); - if (!container || !isArrowFunction(container)) return undefined; + const func = getContainingFunction(node); + if (!func || !isArrowFunction(func)) return undefined; - if (isExpression(container.body)) { + if (isExpression(func.body)) { return { - container, + func, addBraces: true, - expression: container.body + expression: func.body }; } - else if (container.body.statements.length === 1) { - const firstStatement = first(container.body.statements); + else if (func.body.statements.length === 1) { + const firstStatement = first(func.body.statements); if (isReturnStatement(firstStatement)) { + return { - container, + func, addBraces: false, expression: firstStatement.expression }; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 54f8864607..987398f36f 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1642,4 +1642,20 @@ namespace ts { Debug.assert(lastPos >= 0); return lastPos; } + + export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, explicitKind?: CommentKind, explicitHtnl?: boolean) { + forEachLeadingCommentRange(sourceFile.text, sourceNode.pos, (pos, end, kind, htnl) => { + if (kind === SyntaxKind.MultiLineCommentTrivia) { + // Remove leading /* + pos += 2; + // Remove trailing */ + end -= 2; + } + else { + // Remove leading // + pos += 2; + } + addSyntheticLeadingComment(targetNode, explicitKind || kind, sourceFile.text.slice(pos, end), explicitHtnl !== undefined ? explicitHtnl : htnl); + }); + } } diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts index af2927f8f9..7776a8b473 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction15.ts @@ -7,5 +7,5 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Remove braces from arrow function", actionDescription: "Remove braces from arrow function", - newContent: `const foo = a => (void 0);`, + newContent: `const foo = a => void 0;`, }); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts new file mode 100644 index 0000000000..9dde36a8f2 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts @@ -0,0 +1,14 @@ +/// + +//// const foo = /*a*/a/*b*/ => { +//// // return comment +//// return a; +//// }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => /* return comment */ a;`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts new file mode 100644 index 0000000000..97ca16bcb2 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => /* expression comment */ a + 1 + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => { /* expression comment */ return a + 1; }`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts index a2147af1d4..eb4e0939e2 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction4.ts @@ -7,5 +7,5 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Remove braces from arrow function", actionDescription: "Remove braces from arrow function", - newContent: `const foo = a => (a + 1);`, + newContent: `const foo = a => a + 1;`, }); From 0c06126d60b95ee69abda1eb6b3aed3a45815593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 25 Apr 2018 18:25:47 +0800 Subject: [PATCH 04/12] fix converter --- src/compiler/scanner.ts | 10 +++++----- .../addOrRemoveBracesToArrowFunction.ts | 7 ++++--- src/services/utilities.ts | 6 +++--- .../refactorAddOrRemoveBracesToArrowFunction1.ts | 4 +++- .../refactorAddOrRemoveBracesToArrowFunction2.ts | 4 +++- ...refactorAddOrRemoveBracesToArrowFunction20.ts | 2 +- ...refactorAddOrRemoveBracesToArrowFunction21.ts | 9 ++++++--- ...refactorAddOrRemoveBracesToArrowFunction22.ts | 16 ++++++++++++++++ .../refactorAddOrRemoveBracesToArrowFunction3.ts | 4 +++- .../refactorAddOrRemoveBracesToArrowFunction7.ts | 6 ++++++ .../refactorAddOrRemoveBracesToArrowFunction9.ts | 4 +++- 11 files changed, 53 insertions(+), 19 deletions(-) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction7.ts diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 8963285dc7..2bcbed1283 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -626,13 +626,13 @@ namespace ts { * @returns If "reduce" is true, the accumulated value. If "reduce" is false, the first truthy * return value of the callback. */ - function iterateCommentRanges(reduce: boolean, text: string, pos: number, trailing: boolean, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T, memo: U | undefined) => U, state: T, initial?: U): U | undefined { + function iterateCommentRanges(reduce: boolean, text: string, pos: number, trailing: boolean, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T, memo: U | undefined) => U, state: T, initial?: U, inline?: boolean): U | undefined { let pendingPos!: number; let pendingEnd!: number; let pendingKind!: CommentKind; let pendingHasTrailingNewLine!: boolean; let hasPendingCommentRange = false; - let collecting = trailing || pos === 0; + let collecting = inline || trailing || pos === 0; let accumulator = initial; scan: while (pos >= 0 && pos < text.length) { const ch = text.charCodeAt(pos); @@ -725,9 +725,9 @@ namespace ts { } export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean) => U): U | undefined; - export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state: T): U | undefined; - export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state?: T): U | undefined { - return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state); + export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state: T, inline?: boolean): U | undefined; + export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state?: T, inline?: boolean): U | undefined { + return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state, /* initial */ undefined, inline); } export function forEachTrailingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean) => U): U | undefined; diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index e6fed3ebaf..647bbbc5db 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -46,12 +46,14 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { let body: ConciseBody; if (actionName === addBracesActionName) { const returnStatement = createReturn(expression); - body = createBlock([returnStatement]); - copyComments(expression, returnStatement, file, SyntaxKind.SingleLineCommentTrivia, true); + body = createBlock([returnStatement], /* multiLine */ true); + suppressLeadingAndTrailingTrivia(expression); + copyComments(expression, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, true, true); } else if (actionName === removeBracesActionName) { const returnStatement = expression.parent; body = needsParentheses(expression) ? createParen(expression) : expression; + suppressLeadingAndTrailingTrivia(returnStatement); copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, false); } else { @@ -99,7 +101,6 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { else if (func.body.statements.length === 1) { const firstStatement = first(func.body.statements); if (isReturnStatement(firstStatement)) { - return { func, addBraces: false, diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 987398f36f..acb995bf5b 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1643,7 +1643,7 @@ namespace ts { return lastPos; } - export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, explicitKind?: CommentKind, explicitHtnl?: boolean) { + export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, explicitKind?: CommentKind, explicitHtnl?: boolean, inline?: boolean) { forEachLeadingCommentRange(sourceFile.text, sourceNode.pos, (pos, end, kind, htnl) => { if (kind === SyntaxKind.MultiLineCommentTrivia) { // Remove leading /* @@ -1655,7 +1655,7 @@ namespace ts { // Remove leading // pos += 2; } - addSyntheticLeadingComment(targetNode, explicitKind || kind, sourceFile.text.slice(pos, end), explicitHtnl !== undefined ? explicitHtnl : htnl); - }); + addSyntheticLeadingComment(targetNode, explicitKind || kind, sourceFile.text.slice(pos, end), explicitHtnl !== undefined ? explicitHtnl : htnl); + }, undefined, inline) } } diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts index 1d8d52e201..9671981dfc 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts @@ -7,5 +7,7 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", - newContent: `const foo = a => { return a + 1; };`, + newContent: `const foo = a => { + return a + 1; +};`, }); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts index d875018c39..2cce0dc599 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts @@ -7,5 +7,7 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", - newContent: `const foo = a => { return ({ a: 1 }); };`, + newContent: `const foo = a => { + return ({ a: 1 }); +};`, }); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts index 9dde36a8f2..f84e66898f 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction20.ts @@ -10,5 +10,5 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Remove braces from arrow function", actionDescription: "Remove braces from arrow function", - newContent: `const foo = a => /* return comment */ a;`, + newContent: `const foo = a => /* return comment*/ a;`, }); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts index 97ca16bcb2..4ba578ded8 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts @@ -5,7 +5,10 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", - actionName: "Remove braces from arrow function", - actionDescription: "Remove braces from arrow function", - newContent: `const foo = a => { /* expression comment */ return a + 1; }`, + actionName: "Add braces to arrow function", + actionDescription: "Add braces to arrow function", + newContent: `const foo = a => { + /* expression comment */ + return a + 1; +}`, }); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts new file mode 100644 index 0000000000..9d179bcb93 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts @@ -0,0 +1,16 @@ +/// + +//// const foo = /*a*/a/*b*/ => +//// /* expression comment */ +//// a + 1 + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Add braces to arrow function", + actionDescription: "Add braces to arrow function", + newContent: `const foo = a => { + /* expression comment */ + return a + 1; +}`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts index ead5867a17..23710deb00 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction3.ts @@ -7,5 +7,7 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", - newContent: `const foo = a => { return 1; };`, + newContent: `const foo = a => { + return 1; +};`, }); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction7.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction7.ts new file mode 100644 index 0000000000..7a8690be4e --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction7.ts @@ -0,0 +1,6 @@ +/// + +//// const foo = /*a*/a/*b*/ => { }; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Add or remove braces in an arrow function"); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts index 610e19cff4..a7ba12a599 100644 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction9.ts @@ -7,5 +7,7 @@ edit.applyRefactor({ refactorName: "Add or remove braces in an arrow function", actionName: "Add braces to arrow function", actionDescription: "Add braces to arrow function", - newContent: `const foo = a => { return (1, 2, 3); };`, + newContent: `const foo = a => { + return (1, 2, 3); +};`, }); From 1a59eb3949e275d9298a1fe53514252e4f7d5c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Thu, 26 Apr 2018 10:11:59 +0800 Subject: [PATCH 05/12] update body only --- .../addOrRemoveBracesToArrowFunction.ts | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index 647bbbc5db..cb6658f140 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -41,49 +41,34 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { if (!info) return undefined; const { expression, func } = info; - const changeTracker = textChanges.ChangeTracker.fromContext(context); let body: ConciseBody; if (actionName === addBracesActionName) { const returnStatement = createReturn(expression); body = createBlock([returnStatement], /* multiLine */ true); - suppressLeadingAndTrailingTrivia(expression); + suppressLeadingAndTrailingTrivia(body); copyComments(expression, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, true, true); } else if (actionName === removeBracesActionName) { const returnStatement = expression.parent; body = needsParentheses(expression) ? createParen(expression) : expression; - suppressLeadingAndTrailingTrivia(returnStatement); + suppressLeadingAndTrailingTrivia(body); copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, false); } else { Debug.fail('invalid action'); } - updateBody(changeTracker, file, func, body); - - return { - renameFilename: undefined, - renameLocation: undefined, - edits: changeTracker.getChanges() - }; + const edits = textChanges.ChangeTracker.with(context, t => updateBody(t, file, func, body)); + return { renameFilename: undefined, renameLocation: undefined, edits }; } function needsParentheses(expression: Expression) { - if (isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.CommaToken) return true; - if (isObjectLiteralExpression(expression)) return true; - return false; + return isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.CommaToken || isObjectLiteralExpression(expression); } function updateBody(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, body: ConciseBody) { - const arrowFunction = updateArrowFunction( - container, - container.modifiers, - container.typeParameters, - container.parameters, - container.type, - body); - changeTracker.replaceNode(file, container, arrowFunction); + changeTracker.replaceNode(file, container.body, body); } function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined { From b6669c93c0799a0d7ee252cbf4ea650c00c01a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 23 May 2018 10:27:10 +0800 Subject: [PATCH 06/12] revert wrong inline parameter --- src/compiler/scanner.ts | 10 +++++----- src/services/utilities.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 2bcbed1283..9ed6b89ccc 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -626,13 +626,13 @@ namespace ts { * @returns If "reduce" is true, the accumulated value. If "reduce" is false, the first truthy * return value of the callback. */ - function iterateCommentRanges(reduce: boolean, text: string, pos: number, trailing: boolean, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T, memo: U | undefined) => U, state: T, initial?: U, inline?: boolean): U | undefined { + function iterateCommentRanges(reduce: boolean, text: string, pos: number, trailing: boolean, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T, memo: U | undefined) => U, state: T, initial?: U): U | undefined { let pendingPos!: number; let pendingEnd!: number; let pendingKind!: CommentKind; let pendingHasTrailingNewLine!: boolean; let hasPendingCommentRange = false; - let collecting = inline || trailing || pos === 0; + let collecting = trailing || pos === 0; let accumulator = initial; scan: while (pos >= 0 && pos < text.length) { const ch = text.charCodeAt(pos); @@ -725,9 +725,9 @@ namespace ts { } export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean) => U): U | undefined; - export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state: T, inline?: boolean): U | undefined; - export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state?: T, inline?: boolean): U | undefined { - return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state, /* initial */ undefined, inline); + export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state: T): U | undefined; + export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state?: T): U | undefined { + return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state, /* initial */ undefined); } export function forEachTrailingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean) => U): U | undefined; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index acb995bf5b..ca65a41fd2 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1656,6 +1656,6 @@ namespace ts { pos += 2; } addSyntheticLeadingComment(targetNode, explicitKind || kind, sourceFile.text.slice(pos, end), explicitHtnl !== undefined ? explicitHtnl : htnl); - }, undefined, inline) + }) } } From de75f14d2b8630f9a9d6ccc52238a8a91bf870bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 23 May 2018 10:49:57 +0800 Subject: [PATCH 07/12] fix void return statement --- .../addOrRemoveBracesToArrowFunction.ts | 20 ++++++++++--------- src/services/utilities.ts | 4 ++-- ...actorAddOrRemoveBracesToArrowFunction23.ts | 13 ++++++++++++ 3 files changed, 26 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index cb6658f140..cf74786395 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -10,7 +10,8 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { interface Info { func: ArrowFunction; - expression: Expression; + expression: Expression | undefined; + returnStatement?: ReturnStatement; addBraces: boolean; } @@ -40,23 +41,23 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { const info = getConvertibleArrowFunctionAtPosition(file, startPosition); if (!info) return undefined; - const { expression, func } = info; + const { expression, returnStatement, func } = info; let body: ConciseBody; if (actionName === addBracesActionName) { const returnStatement = createReturn(expression); body = createBlock([returnStatement], /* multiLine */ true); suppressLeadingAndTrailingTrivia(body); - copyComments(expression, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, true, true); + copyComments(expression!, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, /* explicitHtnl */ true); } - else if (actionName === removeBracesActionName) { - const returnStatement = expression.parent; - body = needsParentheses(expression) ? createParen(expression) : expression; + else if (actionName === removeBracesActionName && returnStatement) { + const actualExpression = expression || createVoidZero(); + body = needsParentheses(actualExpression) ? createParen(actualExpression) : actualExpression; suppressLeadingAndTrailingTrivia(body); - copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, false); + copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, /* explicitHtnl */ false); } else { - Debug.fail('invalid action'); + Debug.fail("invalid action"); } const edits = textChanges.ChangeTracker.with(context, t => updateBody(t, file, func, body)); @@ -89,7 +90,8 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return { func, addBraces: false, - expression: firstStatement.expression + expression: firstStatement.expression, + returnStatement: firstStatement }; } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index ca65a41fd2..535f35eb48 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1643,7 +1643,7 @@ namespace ts { return lastPos; } - export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, explicitKind?: CommentKind, explicitHtnl?: boolean, inline?: boolean) { + export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, explicitKind?: CommentKind, explicitHtnl?: boolean) { forEachLeadingCommentRange(sourceFile.text, sourceNode.pos, (pos, end, kind, htnl) => { if (kind === SyntaxKind.MultiLineCommentTrivia) { // Remove leading /* @@ -1656,6 +1656,6 @@ namespace ts { pos += 2; } addSyntheticLeadingComment(targetNode, explicitKind || kind, sourceFile.text.slice(pos, end), explicitHtnl !== undefined ? explicitHtnl : htnl); - }) + }); } } diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts new file mode 100644 index 0000000000..062ae3f674 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts @@ -0,0 +1,13 @@ +/// + +//// const foo = /*a*/a/*b*/ => { +//// return; +//// }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => void 0`, +}); From 5497b42558c839cf66ca8dafb79438c0e19d5b1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Thu, 24 May 2018 18:00:42 +0800 Subject: [PATCH 08/12] remove some case --- ...refactorAddOrRemoveBracesToArrowFunction22.ts | 16 ---------------- ...refactorAddOrRemoveBracesToArrowFunction23.ts | 13 ------------- 2 files changed, 29 deletions(-) delete mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts delete mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts deleted file mode 100644 index 9d179bcb93..0000000000 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts +++ /dev/null @@ -1,16 +0,0 @@ -/// - -//// const foo = /*a*/a/*b*/ => -//// /* expression comment */ -//// a + 1 - -goTo.select("a", "b"); -edit.applyRefactor({ - refactorName: "Add or remove braces in an arrow function", - actionName: "Add braces to arrow function", - actionDescription: "Add braces to arrow function", - newContent: `const foo = a => { - /* expression comment */ - return a + 1; -}`, -}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts deleted file mode 100644 index 062ae3f674..0000000000 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts +++ /dev/null @@ -1,13 +0,0 @@ -/// - -//// const foo = /*a*/a/*b*/ => { -//// return; -//// }; - -goTo.select("a", "b"); -edit.applyRefactor({ - refactorName: "Add or remove braces in an arrow function", - actionName: "Remove braces from arrow function", - actionDescription: "Remove braces from arrow function", - newContent: `const foo = a => void 0`, -}); From 3d9a6ab068de2d94c7e35e5c107dce231d3cba89 Mon Sep 17 00:00:00 2001 From: kingwl Date: Thu, 24 May 2018 22:28:30 +0800 Subject: [PATCH 09/12] remove failed test --- .../refactorAddOrRemoveBracesToArrowFunction21.ts | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts deleted file mode 100644 index 4ba578ded8..0000000000 --- a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts +++ /dev/null @@ -1,14 +0,0 @@ -/// - -//// const foo = /*a*/a/*b*/ => /* expression comment */ a + 1 - -goTo.select("a", "b"); -edit.applyRefactor({ - refactorName: "Add or remove braces in an arrow function", - actionName: "Add braces to arrow function", - actionDescription: "Add braces to arrow function", - newContent: `const foo = a => { - /* expression comment */ - return a + 1; -}`, -}); From 590476bf060d3089e4deb6afc13193eeaa1dcb58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Fri, 25 May 2018 10:25:52 +0800 Subject: [PATCH 10/12] add more test and fix others --- src/compiler/scanner.ts | 2 +- .../addOrRemoveBracesToArrowFunction.ts | 12 +++------ src/services/utilities.ts | 4 +-- ...actorAddOrRemoveBracesToArrowFunction21.ts | 11 ++++++++ ...actorAddOrRemoveBracesToArrowFunction22.ts | 27 +++++++++++++++++++ ...actorAddOrRemoveBracesToArrowFunction23.ts | 18 +++++++++++++ 6 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts create mode 100644 tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 9c4dfbe967..41b65c6786 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -727,7 +727,7 @@ namespace ts { export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean) => U): U | undefined; export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state: T): U | undefined; export function forEachLeadingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean, state: T) => U, state?: T): U | undefined { - return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state, /* initial */ undefined); + return iterateCommentRanges(/*reduce*/ false, text, pos, /*trailing*/ false, cb, state); } export function forEachTrailingCommentRange(text: string, pos: number, cb: (pos: number, end: number, kind: CommentKind, hasTrailingNewLine: boolean) => U): U | undefined; diff --git a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts index cf74786395..e982412ed7 100644 --- a/src/services/refactors/addOrRemoveBracesToArrowFunction.ts +++ b/src/services/refactors/addOrRemoveBracesToArrowFunction.ts @@ -48,19 +48,19 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { const returnStatement = createReturn(expression); body = createBlock([returnStatement], /* multiLine */ true); suppressLeadingAndTrailingTrivia(body); - copyComments(expression!, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, /* explicitHtnl */ true); + copyComments(expression!, returnStatement, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ true); } else if (actionName === removeBracesActionName && returnStatement) { const actualExpression = expression || createVoidZero(); body = needsParentheses(actualExpression) ? createParen(actualExpression) : actualExpression; suppressLeadingAndTrailingTrivia(body); - copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, /* explicitHtnl */ false); + copyComments(returnStatement, body, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ false); } else { Debug.fail("invalid action"); } - const edits = textChanges.ChangeTracker.with(context, t => updateBody(t, file, func, body)); + const edits = textChanges.ChangeTracker.with(context, t => t.replaceNode(file, func.body, body)); return { renameFilename: undefined, renameLocation: undefined, edits }; } @@ -68,14 +68,10 @@ namespace ts.refactor.addOrRemoveBracesToArrowFunction { return isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.CommaToken || isObjectLiteralExpression(expression); } - function updateBody(changeTracker: textChanges.ChangeTracker, file: SourceFile, container: ArrowFunction, body: ConciseBody) { - changeTracker.replaceNode(file, container.body, body); - } - function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined { const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); const func = getContainingFunction(node); - if (!func || !isArrowFunction(func)) return undefined; + if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) return undefined; if (isExpression(func.body)) { return { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 199ef0fea9..babe95e57a 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1650,7 +1650,7 @@ namespace ts { return lastPos; } - export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, explicitKind?: CommentKind, explicitHtnl?: boolean) { + export function copyComments(sourceNode: Node, targetNode: Node, sourceFile: SourceFile, commentKind?: CommentKind, hasTrailingNewLine?: boolean) { forEachLeadingCommentRange(sourceFile.text, sourceNode.pos, (pos, end, kind, htnl) => { if (kind === SyntaxKind.MultiLineCommentTrivia) { // Remove leading /* @@ -1662,7 +1662,7 @@ namespace ts { // Remove leading // pos += 2; } - addSyntheticLeadingComment(targetNode, explicitKind || kind, sourceFile.text.slice(pos, end), explicitHtnl !== undefined ? explicitHtnl : htnl); + addSyntheticLeadingComment(targetNode, commentKind || kind, sourceFile.text.slice(pos, end), hasTrailingNewLine !== undefined ? hasTrailingNewLine : htnl); }); } } diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts new file mode 100644 index 0000000000..133940551a --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction21.ts @@ -0,0 +1,11 @@ +/// + +//// const foo = /*a*/a/*b*/ => { return; }; + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Add or remove braces in an arrow function", + actionName: "Remove braces from arrow function", + actionDescription: "Remove braces from arrow function", + newContent: `const foo = a => void 0;`, +}); diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts new file mode 100644 index 0000000000..5c5f973824 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction22.ts @@ -0,0 +1,27 @@ +/// + +//// const /*a*/foo/*b*/ = /*c*/(/*d*//*e*/aa/*f*/aa, /*g*/b/*h*/) /*i*//*j*/ /*k*/=>/*l*/ /*m*/{/*n*/ /*o*/return/*p*/ 1; }; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("c", "d"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("e", "f"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("g", "h"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("i", "j"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("k", "l"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("m", "n"); +verify.not.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") + +goTo.select("o", "p"); +verify.not.refactorAvailable("Add or remove braces in an arrow function", "Remove braces from arrow function") diff --git a/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts new file mode 100644 index 0000000000..4c39ddbd30 --- /dev/null +++ b/tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction23.ts @@ -0,0 +1,18 @@ +/// + +//// const /*a*/foo/*b*/ = /*c*/()/*d*/ /*e*//*f*/ /*g*/=>/*h*/ /*i*/1/*j*/; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Add or remove braces in an arrow function", "Add braces to arrow function") + +goTo.select("c", "d"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Add braces to arrow function") + +goTo.select("e", "f"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Add braces to arrow function") + +goTo.select("g", "h"); +verify.refactorAvailable("Add or remove braces in an arrow function", "Add braces to arrow function") + +goTo.select("i", "j"); +verify.not.refactorAvailable("Add or remove braces in an arrow function", "Add braces to arrow function") From 7df81311ac27d179ecd71be51deab5b7ab4cdb17 Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 11 Jun 2018 11:20:34 -0700 Subject: [PATCH 11/12] goToDefinition: Don't add duplicate definitions for VariableDeclaration and ArrowFunction at `f = () => {}` (#24863) --- src/services/goToDefinition.ts | 11 ++++++++++- tests/cases/fourslash/goToDefinitionSignatureAlias.ts | 9 +++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index dfa50253da..d91dc63d92 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -32,7 +32,7 @@ namespace ts.GoToDefinition { const sigInfo = createDefinitionFromSignatureDeclaration(typeChecker, calledDeclaration); // For a function, if this is the original function definition, return just sigInfo. // If this is the original constructor definition, parent is the class. - if (typeChecker.getRootSymbols(symbol).some(s => calledDeclaration.symbol === s || calledDeclaration.symbol.parent === s) || + if (typeChecker.getRootSymbols(symbol).some(s => symbolMatchesSignature(s, calledDeclaration)) || // TODO: GH#23742 Following check shouldn't be necessary if 'require' is an alias symbol.declarations.some(d => isVariableDeclaration(d) && !!d.initializer && isRequireCall(d.initializer, /*checkArgumentIsStringLiteralLike*/ false))) { return [sigInfo]; @@ -93,6 +93,15 @@ namespace ts.GoToDefinition { return getDefinitionFromSymbol(typeChecker, symbol, node); } + /** + * True if we should not add definitions for both the signature symbol and the definition symbol. + * True for `const |f = |() => 0`, false for `function |f() {} const |g = f;`. + */ + function symbolMatchesSignature(s: Symbol, calledDeclaration: SignatureDeclaration) { + return s === calledDeclaration.symbol || s === calledDeclaration.symbol.parent || + isVariableDeclaration(calledDeclaration.parent) && s === calledDeclaration.parent.symbol; + } + export function getReferenceAtPosition(sourceFile: SourceFile, position: number, program: Program): { fileName: string, file: SourceFile } | undefined { const referencePath = findReferenceInPosition(sourceFile.referencedFiles, position); if (referencePath) { diff --git a/tests/cases/fourslash/goToDefinitionSignatureAlias.ts b/tests/cases/fourslash/goToDefinitionSignatureAlias.ts index 27ff0eb69d..3657ebfd0d 100644 --- a/tests/cases/fourslash/goToDefinitionSignatureAlias.ts +++ b/tests/cases/fourslash/goToDefinitionSignatureAlias.ts @@ -8,8 +8,17 @@ ////[|/*useG*/g|](); ////[|/*useH*/h|](); +////const i = /*i*/() => 0; +////const /*j*/j = i; + +////[|/*useI*/i|](); +////[|/*useJ*/j|](); + verify.goToDefinition({ useF: "f", useG: ["g", "f"], useH: ["h", "f"], + + useI: "i", + useJ: ["j", "i"], }); From ed20f7d9838bfd3cbe238c728f3da5b47059ef7c Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 11 Jun 2018 13:10:29 -0700 Subject: [PATCH 12/12] Simplify tryDeleteDeclaration (#24808) --- src/services/codefixes/fixUnusedIdentifier.ts | 43 ++++++------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/src/services/codefixes/fixUnusedIdentifier.ts b/src/services/codefixes/fixUnusedIdentifier.ts index 719e3a316f..ad3e2e36f5 100644 --- a/src/services/codefixes/fixUnusedIdentifier.ts +++ b/src/services/codefixes/fixUnusedIdentifier.ts @@ -148,20 +148,9 @@ namespace ts.codefix { return false; } - function tryDeleteDeclaration(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, deletedAncestors: NodeSet | undefined, checker: TypeChecker, isFixAll: boolean): void { - switch (token.kind) { - case SyntaxKind.Identifier: - tryDeleteIdentifier(changes, sourceFile, token, deletedAncestors, checker, isFixAll); - deleteAssignments(changes, sourceFile, token as Identifier, checker); - break; - case SyntaxKind.PropertyDeclaration: - case SyntaxKind.NamespaceImport: - if (deletedAncestors) deletedAncestors.add(token.parent); - changes.deleteNode(sourceFile, token.parent); - break; - default: - tryDeleteDefault(changes, sourceFile, token, deletedAncestors); - } + function tryDeleteDeclaration(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, deletedAncestors: NodeSet | undefined, checker: TypeChecker, isFixAll: boolean) { + tryDeleteDeclarationWorker(changes, sourceFile, token, deletedAncestors, checker, isFixAll); + if (isIdentifier(token)) deleteAssignments(changes, sourceFile, token, checker); } function deleteAssignments(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Identifier, checker: TypeChecker) { @@ -173,19 +162,8 @@ namespace ts.codefix { }); } - function tryDeleteDefault(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, deletedAncestors: NodeSet | undefined): void { - if (isDeclarationName(token)) { - if (deletedAncestors) deletedAncestors.add(token.parent); - changes.deleteNode(sourceFile, token.parent); - } - else if (isLiteralComputedPropertyDeclarationName(token)) { - if (deletedAncestors) deletedAncestors.add(token.parent.parent); - changes.deleteNode(sourceFile, token.parent.parent); - } - } - - function tryDeleteIdentifier(changes: textChanges.ChangeTracker, sourceFile: SourceFile, identifier: Identifier, deletedAncestors: NodeSet | undefined, checker: TypeChecker, isFixAll: boolean): void { - const parent = identifier.parent; + function tryDeleteDeclarationWorker(changes: textChanges.ChangeTracker, sourceFile: SourceFile, token: Node, deletedAncestors: NodeSet | undefined, checker: TypeChecker, isFixAll: boolean): void { + const parent = token.parent; switch (parent.kind) { case SyntaxKind.VariableDeclaration: tryDeleteVariableDeclaration(changes, sourceFile, parent, deletedAncestors); @@ -250,7 +228,7 @@ namespace ts.codefix { // handle case where 'import a = A;' case SyntaxKind.ImportEqualsDeclaration: - const importEquals = getAncestor(identifier, SyntaxKind.ImportEqualsDeclaration)!; + const importEquals = getAncestor(token, SyntaxKind.ImportEqualsDeclaration)!; changes.deleteNode(sourceFile, importEquals); break; @@ -290,7 +268,14 @@ namespace ts.codefix { break; default: - tryDeleteDefault(changes, sourceFile, identifier, deletedAncestors); + if (isDeclarationName(token)) { + if (deletedAncestors) deletedAncestors.add(token.parent); + changes.deleteNode(sourceFile, token.parent); + } + else if (isLiteralComputedPropertyDeclarationName(token)) { + if (deletedAncestors) deletedAncestors.add(token.parent.parent); + changes.deleteNode(sourceFile, token.parent.parent); + } break; } }