Skip to content

Fix parameter emit for synthetic function types#16341

Merged
rbuckton merged 2 commits into
masterfrom
fix15651
Jun 7, 2017
Merged

Fix parameter emit for synthetic function types#16341
rbuckton merged 2 commits into
masterfrom
fix15651

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton commented Jun 7, 2017

Currently, the emitter determines whether to emit parentheses around the parameter purely based on the number of parameters and whether there are any parsed tokens between the parent node and the first parameter. However, this logic is invalid if both the parent node and the parameter are both synthesized.

This change adds a few more additional tests to ensure its valid to emit the parameter list of an arrow function or function type.

Fixes #15651

@rbuckton rbuckton requested a review from a user June 7, 2017 22:32
@rbuckton rbuckton requested a review from sandersn June 7, 2017 22:32
Comment thread src/compiler/emitter.ts
function canEmitSimpleArrowHead(parentNode: FunctionTypeNode | ArrowFunction, parameters: NodeArray<ParameterDeclaration>) {
const parameter = singleOrUndefined(parameters);
return parameter
&& parameter.pos === parentNode.pos // may not have parsed tokens between parent and parameter
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this for async?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This existed previously and was for handling source parsed as (x) => x vs x => x.

@DanielRosenwasser
Copy link
Copy Markdown
Member

👍

@rbuckton rbuckton merged commit 4d5175b into master Jun 7, 2017
@rbuckton rbuckton deleted the fix15651 branch June 7, 2017 23:24
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants