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 to convert lambda to function and vice-versa #28250

Merged
merged 38 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
8cb019d
add skeleton
bigaru Oct 17, 2018
6cfbee7
add getAvailableActions
bigaru Oct 17, 2018
f558f2f
add working getEditsForAction
bigaru Oct 17, 2018
4c80de9
add multi vardecl
bigaru Oct 18, 2018
367f47e
fix multi decl bug
bigaru Oct 18, 2018
e697856
change refactor name
bigaru Oct 18, 2018
cc07d68
add tests for ToAnon, ToArrow and available arrow
bigaru Oct 18, 2018
05c44f1
add tests for ToNamed and available anon
bigaru Oct 18, 2018
d771865
add tests for ReturnType and available Arrow as FnParam
bigaru Oct 18, 2018
62b9fcc
fix bug modifiers by toNamed
bigaru Oct 18, 2018
c25726e
add tests for modifiers
bigaru Oct 18, 2018
7c78cd5
fix for tslint error
bigaru Oct 18, 2018
152e362
adapt one test case
bigaru Oct 19, 2018
6922f6c
refactor getInfo getAvailableActions
bigaru Oct 19, 2018
39c3928
refactor small progress
bigaru Oct 19, 2018
a9cb623
extract creation of block
bigaru Oct 19, 2018
6bd26cd
extract creation of funcDeclaration
bigaru Oct 19, 2018
3e7dcad
make guideline compliant
bigaru Oct 19, 2018
649b53c
apply feedback from pr
bigaru Oct 22, 2018
5951318
add testcase and apply feedback from pr
bigaru Oct 22, 2018
d97e073
apply feedback from pr
bigaru Oct 22, 2018
0fa2fad
add newline
bigaru Oct 23, 2018
1c74d0e
rename testcases
bigaru Oct 23, 2018
8bc7f43
Make conditions more expressive
D0nGiovanni Oct 25, 2018
6b80047
Merge pull request #19 from D0nGiovanni/d-lambda-to-fn
bigaru Oct 31, 2018
739e1e9
fix for unnecessary duplication of comment
bigaru Nov 7, 2018
dc81a7f
apply feedback from pr
bigaru Nov 7, 2018
5dfc71c
Merge remote-tracking branch 'upstream/master' into m-lambda-to-fn
bigaru Nov 7, 2018
9a466f4
update getAvailableActions
bigaru Nov 7, 2018
dfb86ac
check if functionExpression name is used
bigaru Nov 8, 2018
e1dc52f
add more testcases
bigaru Nov 9, 2018
52e94d2
Merge remote-tracking branch 'upstream/master' into m-lambda-to-fn
bigaru Jan 22, 2019
dbd5859
do not provide refactoring when it contains this
bigaru Jan 22, 2019
5ec1201
exclude nested functions and classes at containingThis check
bigaru Jan 23, 2019
e6dc8f6
Merge remote-tracking branch 'upstream/master' into m-lambda-to-fn
bigaru May 27, 2020
fcb8e87
fix linting error
bigaru May 27, 2020
52dad73
Merge branch 'master' into m-lambda-to-fn
jessetrinity Jun 1, 2020
fb3a84c
fix line endings
Jun 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4720,5 +4720,21 @@
"Generate types for all packages without types": {
"category": "Message",
"code": 95068
},
"Convert arrow function or function expression": {
"category": "Message",
"code": 95069
},
"Convert to anonymous function": {
"category": "Message",
"code": 95070
},
"Convert to named function": {
"category": "Message",
"code": 95071
},
"Convert to arrow function": {
"category": "Message",
"code": 95072
}
}
192 changes: 192 additions & 0 deletions src/services/refactors/convertArrowFunctionOrFunctionExpression.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
/* @internal */
namespace ts.refactor.convertArrowFunctionOrFunctionExpression {
const refactorName = "Convert arrow function or function expression";
const refactorDescription = getLocaleSpecificMessage(Diagnostics.Convert_arrow_function_or_function_expression);

const toAnonymousFunctionActionName = "Convert to anonymous function";
const toNamedFunctionActionName = "Convert to named function";
const toArrowFunctionActionName = "Convert to arrow function";

const toAnonymousFunctionActionDescription = getLocaleSpecificMessage(Diagnostics.Convert_to_anonymous_function);
const toNamedFunctionActionDescription = getLocaleSpecificMessage(Diagnostics.Convert_to_named_function);
const toArrowFunctionActionDescription = getLocaleSpecificMessage(Diagnostics.Convert_to_arrow_function);

registerRefactor(refactorName, { getEditsForAction, getAvailableActions });

interface FunctionInfo {
readonly selectedVariableDeclaration: boolean;
readonly func: FunctionExpression | ArrowFunction;
}

interface VariableInfo {
readonly variableDeclaration: VariableDeclaration;
readonly variableDeclarationList: VariableDeclarationList;
readonly statement: VariableStatement;
readonly name: Identifier;
}

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

if (!info) return undefined;
const { selectedVariableDeclaration, func } = info;
const possibleActions: RefactorActionInfo[] = [];

if (selectedVariableDeclaration || (isArrowFunction(func) && isVariableDeclaration(func.parent))) {
possibleActions.push({
name: toNamedFunctionActionName,
description: toNamedFunctionActionDescription
});
}

if (!selectedVariableDeclaration && isArrowFunction(func)) {
possibleActions.push({
name: toAnonymousFunctionActionName,
description: toAnonymousFunctionActionDescription
});
}

if (isFunctionExpression(func)) {
possibleActions.push({
name: toArrowFunctionActionName,
description: toArrowFunctionActionDescription
});
}

return [{
name: refactorName,
description: refactorDescription,
actions: possibleActions
}];
}

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

if (!info) return undefined;
const { func } = info;

switch (actionName) {
case toAnonymousFunctionActionName:
return getEditInfoForConvertToAnonymousFunction(context, func);

case toNamedFunctionActionName:
return getEditInfoForConvertToNamedFunction(context, func);

case toArrowFunctionActionName:
return getEditInfoForConvertToArrowFunction(context, func);

default:
Debug.fail("invalid action");
Copy link

Choose a reason for hiding this comment

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

You can just return Debug.fail("Invalid action"); since it returns never.

break;
}

return undefined;
}

function getFunctionInfo(file: SourceFile, startPosition: number): FunctionInfo | undefined {
const token = getTokenAtPosition(file, startPosition);
let maybeFunc;
Copy link

Choose a reason for hiding this comment

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

This could just be two different variables, one for each time it's assigned to.


maybeFunc = getArrowFunctionFromVariableDeclaration(token.parent);
if (!!maybeFunc) return { selectedVariableDeclaration: true, func: maybeFunc };
Copy link

Choose a reason for hiding this comment

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

No need to have !! for the condition of an if, it automatically booleanifies everything.


maybeFunc = getContainingFunction(token);
if (!!maybeFunc && (isFunctionExpression(maybeFunc) || isArrowFunction(maybeFunc)) && !rangeContainsRange(maybeFunc.body, token)) {
return { selectedVariableDeclaration: false, func: maybeFunc };
}

return undefined;
}

function isSingleVariableDeclaration(parent: Node): parent is VariableDeclarationList {
return isVariableDeclaration(parent) || (isVariableDeclarationList(parent) && parent.declarations.length === 1);
}

function getArrowFunctionFromVariableDeclaration(parent: Node): ArrowFunction | undefined {
if (!isSingleVariableDeclaration(parent)) return undefined;
const variableDeclaration = isVariableDeclaration(parent) ? parent : parent.declarations[0];

const initializer = variableDeclaration.initializer;
if (!initializer || !isArrowFunction(initializer)) return undefined;
return initializer;
}

function convertToBlock(body: ConciseBody): Block {
if (isExpression(body)) {
return createBlock([createReturn(body)], /* multiLine */ true);
}
else {
return body;
}
}

function getVariableInfo(func: FunctionExpression | ArrowFunction): VariableInfo | undefined {
const variableDeclaration = func.parent;
if (!isVariableDeclaration(variableDeclaration) || !isVariableDeclarationInVariableStatement(variableDeclaration)) return undefined;

const variableDeclarationList = variableDeclaration.parent;
const statement = variableDeclarationList.parent;
if (!isVariableDeclarationList(variableDeclarationList) || !isVariableStatement(statement) || !isIdentifier(variableDeclaration.name)) return undefined;

return { variableDeclaration, variableDeclarationList, statement, name: variableDeclaration.name };
}

function getEditInfoForConvertToAnonymousFunction(context: RefactorContext, func: FunctionExpression | ArrowFunction): RefactorEditInfo {
const { file } = context;
const body = convertToBlock(func.body);
const newNode = createFunctionExpression(func.modifiers, func.asteriskToken, /* name */ undefined, func.typeParameters, func.parameters, func.type, body);
const edits = textChanges.ChangeTracker.with(context, t => t.replaceNode(file, func, newNode));
return { renameFilename: undefined, renameLocation: undefined, edits };
Copy link

Choose a reason for hiding this comment

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

It may be better for these functions to just return the edits, and some outer function to wrap them in RefactorEditInfo.

}

function getEditInfoForConvertToNamedFunction(context: RefactorContext, func: FunctionExpression | ArrowFunction): RefactorEditInfo | undefined {
const { file } = context;
const body = convertToBlock(func.body);
const variableInfo = getVariableInfo(func);
if (!variableInfo) return undefined;

const { variableDeclaration, variableDeclarationList, statement, name } = variableInfo;
const newNode = createFunctionDeclaration(func.decorators, statement.modifiers, func.asteriskToken, name, func.typeParameters, func.parameters, func.type, body);
let edits: FileTextChanges[];

if (variableDeclarationList.declarations.length === 1) {
edits = textChanges.ChangeTracker.with(context, t => t.replaceNode(file, statement, newNode));
}
else {
edits = textChanges.ChangeTracker.with(context, t => {
t.delete(file, variableDeclaration);
t.insertNodeAfter(file, statement, newNode);
});
}
return { renameFilename: undefined, renameLocation: undefined, edits };
}

function getEditInfoForConvertToArrowFunction(context: RefactorContext, func: FunctionExpression | ArrowFunction): RefactorEditInfo | undefined {
const { file } = context;
if (!isFunctionExpression(func)) return undefined;
Copy link

@ghost ghost Nov 1, 2018

Choose a reason for hiding this comment

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

The parameter type should just be FunctionExpression then?


const statements = func.body.statements;
const head = statements[0];
let body: ConciseBody;

if (canBeConvertedToExpression(func.body, head)) {
body = head.expression!;
suppressLeadingAndTrailingTrivia(body);
copyComments(head, body, file, SyntaxKind.MultiLineCommentTrivia, /* hasTrailingNewLine */ false);
}
else {
body = func.body;
}

const newNode = createArrowFunction(func.modifiers, func.typeParameters, func.parameters, func.type, /* equalsGreaterThanToken */ undefined, body);
Copy link

Choose a reason for hiding this comment

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

Suprised this works without the token -- seems like it would be better to createToken(SyntaxKind.EqualsGreaterThanToken) even if the emitter doesn't currently require it.

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

function canBeConvertedToExpression(body: Block, head: Statement): head is ReturnStatement | ExpressionStatement {
return body.statements.length === 1 && ((isReturnStatement(head) && !!head.expression) || isExpressionStatement(head));
Copy link

Choose a reason for hiding this comment

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

Not sure about the last condition there. () => foo() has a different type from () => { foo(); }.

Copy link
Contributor

Choose a reason for hiding this comment

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

you're right.
to clarify

let epsilon = () => "epsilon";
let foo = () => epsilon()       // foo: () => string
let bar = () => { epsilon() }   // bar: () => void

}
}
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"refactors/generateGetAccessorAndSetAccessor.ts",
"refactors/moveToNewFile.ts",
"refactors/addOrRemoveBracesToArrowFunction.ts",
"refactors/convertArrowFunctionOrFunctionExpression.ts",
"services.ts",
"breakpoints.ts",
"transform.ts",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// <reference path='fourslash.ts' />

//// /*z*/c/*y*/onst /*x*/f/*w*/oo = /*v*/f/*u*/unction() /*t*/{/*s*/ /*r*/r/*q*/eturn 42;};

goTo.select("z", "y");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("x", "w");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("v", "u");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("t", "s");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("r", "q");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// <reference path='fourslash.ts' />

//// function foo(a){}
//// /*z*/f/*y*/oo/*x*/(/*w*//*v*/f/*u*/unction/*t*/(/*s*//*r*/b/*q*/,c){/*p*/r/*o*/eturn 42;})

goTo.select("z", "y");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("x", "w");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("v", "u");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("t", "s");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("r", "q");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("p", "o");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/// <reference path='fourslash.ts' />

//// /*z*/c/*y*/onst /*x*/f/*w*/oo = /*v*/(/*u*//*t*/a/*s*/, b) /*r*/=/*q*/> /*p*/4/*o*/2;

goTo.select("z", "y");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("x", "w");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("v", "u");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("t", "s");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("r", "q");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("p", "o");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/// <reference path='fourslash.ts' />

//// function foo(a){}
//// /*z*/f/*y*/oo/*x*/(/*w*//*v*/(/*u*//*t*/a/*s*/, b) => /*r*/a/*q*/ + b)


goTo.select("z", "y");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("x", "w");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("v", "u");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("t", "s");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("r", "q");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/// <reference path='fourslash.ts' />

//// /*z*/l/*y*/et /*x*/f/*w*/oo, /*v*/b/*u*/ar = /*t*/(/*s*/) => 42;

goTo.select("z", "y");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("x", "w");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("v", "u");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");

goTo.select("t", "s");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to named function");
verify.refactorAvailable("Convert arrow function or function expression", "Convert to anonymous function");
verify.not.refactorAvailable("Convert arrow function or function expression", "Convert to arrow function");
Loading