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

Improved lookahead for arrow functions. #203

Merged
merged 5 commits into from Jul 25, 2014
Merged

Conversation

DanielRosenwasser
Copy link
Member

Fixes issue #34.

Specifically:

  • We no longer automatically assume "()" is a function expression. It must be followed by an arrow, colon, or curly brace.
  • If an arrow is missing following a signature, but a curly brace is present, we assume the user forgot the arrow and try to parse the body anyway.

Fixes issue #34.

Specifically:
    * We no longer automatically assume "()" is a function expression. It must be followed by an arrow, colon, or curly brace.
    * If an arrow is missing following a signature, but a curly brace is present, we assume the user forgot the arrow and try to parse the body anyway.
// We had "(" not followed by an identifier. This definitely doesn't
// look like a lambda. Note: we could be a little more lenient and allow
// "(public" or "(private". These would not ever actually be allowed,
// but we could provide a good error message instead of bailing out.
if (!isIdentifier()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did tihs comment move outside of the block that it is relevant to?

Copy link
Member Author

Choose a reason for hiding this comment

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

Felt like it would be more consistent since all the other comment blocks reside outside the checks. I'm feeling against it now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, with different phrasing, I think it might be fine.

Specifically where we have a full signature followed by an open curly brace.
var sig = parseSignature(SyntaxKind.CallSignature, SyntaxKind.ColonToken);
parseExpected(SyntaxKind.EqualsGreaterThanToken);
return sig;
if (token === SyntaxKind.EqualsGreaterThanToken || token === SyntaxKind.OpenBraceToken) {
Copy link
Contributor

Choose a reason for hiding this comment

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

looks good. could you add a comment here why having a colon isn't enogh. i.e. the ambiguity with:

a ? (b) : ...
thanks!

!!! ';' expected.
~
!!! Cannot find name 'x'.
!!! '=>' expected.
Copy link
Member Author

Choose a reason for hiding this comment

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

DanielRosenwasser added a commit that referenced this pull request Jul 25, 2014
Improved lookahead for arrow functions.
@DanielRosenwasser DanielRosenwasser merged commit c625cd9 into master Jul 25, 2014
@DanielRosenwasser DanielRosenwasser deleted the arrowLookAhead branch July 25, 2014 01:41
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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.

None yet

3 participants