add support to convert lambda to function and vice-versa #28250
Conversation
lambda to fn
|
|
|
|
||
| function getFunctionInfo(file: SourceFile, startPosition: number): FunctionInfo | undefined { | ||
| const token = getTokenAtPosition(file, startPosition); | ||
| let maybeFunc; |
There was a problem hiding this comment.
This could just be two different variables, one for each time it's assigned to.
| let maybeFunc; | ||
|
|
||
| maybeFunc = getArrowFunctionFromVariableDeclaration(token.parent); | ||
| if (!!maybeFunc) return { selectedVariableDeclaration: true, func: maybeFunc }; |
There was a problem hiding this comment.
No need to have !! for the condition of an if, it automatically booleanifies everything.
| return getEditInfoForConvertToArrowFunction(context, func); | ||
|
|
||
| default: | ||
| Debug.fail("invalid action"); |
There was a problem hiding this comment.
You can just return Debug.fail("Invalid action"); since it returns never.
|
|
||
| function getEditInfoForConvertToArrowFunction(context: RefactorContext, func: FunctionExpression | ArrowFunction): RefactorEditInfo | undefined { | ||
| const { file } = context; | ||
| if (!isFunctionExpression(func)) return undefined; |
There was a problem hiding this comment.
The parameter type should just be FunctionExpression then?
| } | ||
|
|
||
| function canBeConvertedToExpression(body: Block, head: Statement): head is ReturnStatement | ExpressionStatement { | ||
| return body.statements.length === 1 && ((isReturnStatement(head) && !!head.expression) || isExpressionStatement(head)); |
There was a problem hiding this comment.
Not sure about the last condition there. () => foo() has a different type from () => { foo(); }.
There was a problem hiding this comment.
you're right.
to clarify
let epsilon = () => "epsilon";
let foo = () => epsilon() // foo: () => string
let bar = () => { epsilon() } // bar: () => void
| body = func.body; | ||
| } | ||
|
|
||
| const newNode = createArrowFunction(func.modifiers, func.typeParameters, func.parameters, func.type, /* equalsGreaterThanToken */ undefined, body); |
There was a problem hiding this comment.
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 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 }; |
There was a problem hiding this comment.
It may be better for these functions to just return the edits, and some outer function to wrap them in RefactorEditInfo.
| @@ -0,0 +1,13 @@ | |||
| /// <reference path='fourslash.ts' /> | |||
|
|
|||
| //// let foo = /*x*/(/*y*/) => 1 + 1; | |||
There was a problem hiding this comment.
There should be tests where the function expression isn't preceded by a variable declaration.
ghost
mentioned this pull request
because this behaves differently in arrow than in function
|
Hi @Andy-MS I've solved that using forEachChild to detect whether a body contains a Are the any auxiliary function which can provide a better way to solve this? |
|
@bigaru Make sure you're not testing any nested functions or classes, which would come with their own meaning of |
|
@D0nGiovanni if you are still interested in this PR I would like to get it in once the merge conflicts are resolved. Thanks! |
|
@jessetrinity Alright! I should have it done by the end of the weekend. |
|
When @D0nGiovanni told me, I was so excited that I resolved the conflicts. Thanks @jessetrinity for your effort! |
|
@D0nGiovanni @bigaru thank you for the contribution to TypeScript! |
6 participants
Fixes #23299