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.
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! |
Fixes #23299