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

Fix parsing of an "async of" edge case in for loop #1286

Merged
merged 3 commits into from
Apr 3, 2024

Conversation

adams85
Copy link
Contributor

@adams85 adams85 commented Mar 28, 2024

The following edge case of for loops should result in syntax error but is accepted currently:

async () => { for (async
of []); }

This PR makes an attempt at fixing this issue.

Also, suggests a minor improvement to parsing of for statements:

As far as I can tell based on the spec, we can "take a shortcut" in the case of for-await-of loops. Once we read "for await", the initializer of the for statement is no longer ambiguous (since we know that it can't be a for(;;) loop), that is, the initializer part will be a LeftHandSideExpression (unless it's a variable declaration). So, we don't need to parse a full expression using parseExpression, parseExprSubscripts is enough - if my understanding of acorn's inner workings is correct.

Also V8 seems to take this shortcut: just check the error reported for e.g. async() => { for await (x = 1 of []); }

@adams85 adams85 changed the title Fix/for of issue Fix parsing of an async of edge case in for loop Mar 28, 2024
@adams85 adams85 changed the title Fix parsing of an async of edge case in for loop Fix parsing of an async edge case in for loop Mar 28, 2024
@adams85 adams85 changed the title Fix parsing of an async edge case in for loop Fix parsing of an "async of" edge case in for loop Mar 28, 2024
@marijnh marijnh merged commit ed4a7a1 into acornjs:master Apr 3, 2024
1 check passed
@marijnh
Copy link
Member

marijnh commented Apr 3, 2024

Thanks for finding that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants