Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for add or remove braces to arrow function #23423

Merged
merged 12 commits into from
Jun 11, 2018
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4284,5 +4284,17 @@
"Convert '{0}' to mapped object type": {
"category": "Message",
"code": 95055
},
"Add or remove braces in an arrow function": {
"category": "Message",
"code": 95056
},
"Add braces to arrow function": {
"category": "Message",
"code": 95057
},
"Remove braces from arrow function": {
"category": "Message",
"code": 95058
}
}
1 change: 1 addition & 0 deletions src/harness/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
"../services/refactors/addOrRemoveBracesToArrowFunction.ts",
"../services/sourcemaps.ts",
"../services/services.ts",
"../services/breakpoints.ts",
Expand Down
1 change: 1 addition & 0 deletions src/server/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
"../services/refactors/addOrRemoveBracesToArrowFunction.ts",
"../services/sourcemaps.ts",
"../services/services.ts",
"../services/breakpoints.ts",
Expand Down
1 change: 1 addition & 0 deletions src/server/tsconfig.library.json
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
"../services/refactors/extractSymbol.ts",
"../services/refactors/generateGetAccessorAndSetAccessor.ts",
"../services/refactors/moveToNewFile.ts",
"../services/refactors/addOrRemoveBracesToArrowFunction.ts",
"../services/sourcemaps.ts",
"../services/services.ts",
"../services/breakpoints.ts",
Expand Down
16 changes: 0 additions & 16 deletions src/services/codefixes/convertFunctionToEs6Class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Modifier> | undefined {
return filter(source.modifiers, modifier => modifier.kind === kind);
}
Expand Down
96 changes: 96 additions & 0 deletions src/services/refactors/addOrRemoveBracesToArrowFunction.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/* @internal */
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;
const removeBracesActionDescription = Diagnostics.Remove_braces_from_arrow_function.message;
registerRefactor(refactorName, { getEditsForAction, getAvailableActions });

interface Info {
func: ArrowFunction;
expression: Expression | undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why expression is required, but can be undefined, whereas returnStatement is optional. Should they follow the same pattern?

returnStatement?: ReturnStatement;
addBraces: boolean;
}

function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
const { file, startPosition } = context;
const info = getConvertibleArrowFunctionAtPosition(file, startPosition);
if (!info) return undefined;

return [{
name: refactorName,
description: refactorDescription,
actions: [
info.addBraces ?
{
name: addBracesActionName,
description: addBracesActionDescription
} : {
name: removeBracesActionName,
description: removeBracesActionDescription
}
]
}];
}

function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined {
const { file, startPosition } = context;
const info = getConvertibleArrowFunctionAtPosition(file, startPosition);
if (!info) return undefined;

const { expression, returnStatement, func } = info;

let body: ConciseBody;
if (actionName === addBracesActionName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter whether this agrees with info.addBraces?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened with this?

const returnStatement = createReturn(expression);
body = createBlock([returnStatement], /* multiLine */ true);
suppressLeadingAndTrailingTrivia(body);
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, /* hasTrailingNewLine */ false);
}
else {
Debug.fail("invalid action");
}

const edits = textChanges.ChangeTracker.with(context, t => t.replaceNode(file, func.body, body));
return { renameFilename: undefined, renameLocation: undefined, edits };
}

function needsParentheses(expression: Expression) {
return isBinaryExpression(expression) && expression.operatorToken.kind === SyntaxKind.CommaToken || isObjectLiteralExpression(expression);
}

function getConvertibleArrowFunctionAtPosition(file: SourceFile, startPosition: number): Info | undefined {
const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
const func = getContainingFunction(node);
if (!func || !isArrowFunction(func) || (!rangeContainsRange(func, node) || rangeContainsRange(func.body, node))) return undefined;

if (isExpression(func.body)) {
return {
func,
addBraces: true,
expression: func.body
};
}
else if (func.body.statements.length === 1) {
const firstStatement = first(func.body.statements);
if (isReturnStatement(firstStatement)) {
return {
func,
addBraces: false,
expression: firstStatement.expression,
returnStatement: firstStatement
};
}
}
return undefined;
}
}
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
"refactors/extractSymbol.ts",
"refactors/generateGetAccessorAndSetAccessor.ts",
"refactors/moveToNewFile.ts",
"refactors/addOrRemoveBracesToArrowFunction.ts",
"sourcemaps.ts",
"services.ts",
"breakpoints.ts",
Expand Down
16 changes: 16 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1649,4 +1649,20 @@ namespace ts {
Debug.assert(lastPos >= 0);
return lastPos;
}

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 /*
pos += 2;
// Remove trailing */
end -= 2;
}
else {
// Remove leading //
pos += 2;
}
addSyntheticLeadingComment(targetNode, commentKind || kind, sourceFile.text.slice(pos, end), hasTrailingNewLine !== undefined ? hasTrailingNewLine : htnl);
});
}
}
13 changes: 13 additions & 0 deletions tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path='fourslash.ts' />

//// const foo = /*a*/a/*b*/ => 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 => {
return a + 1;
};`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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);`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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);`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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";`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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 => ({});`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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}\`;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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\`;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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;`,
});
13 changes: 13 additions & 0 deletions tests/cases/fourslash/refactorAddOrRemoveBracesToArrowFunction2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path='fourslash.ts' />

//// const foo = /*a*/a/*b*/ => ({ 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 => {
return ({ a: 1 });
};`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

//// 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;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />

//// 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;`,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/// <reference path='fourslash.ts' />

//// 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")
Loading