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

Refactor yield await classification #12230

Merged
merged 14 commits into from Oct 26, 2020

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Oct 21, 2020

Q                       A
Fixed Issues? Fixes parsing error allowed in test262
Patch: Bug Fix? Yes
Tests Added + Pass? Yes
License MIT

Currently @babel/parser uses parser state to track the position and possible declaration error of yield and await in ambiguous patterns: arrow head ((x)) and async arrow head async(x). Historically we have done several fixing attempts (#6877, #7727, #9405, #10469, #11148, etc.), however this approach is error-prone and can not handle edge cases especially if these patterns are nested: For example, many, if not most, test cases I added in this PR are failing on main but fixed by this PR.

We introduce an ExpressionScope to handle declaration errors in ambiguous patterns. See here for the design of ExpressionScope. The idea is to record such errors and only thrown at the time when such pattern is indeed an arrow function or an async arrow function. The stack based scope play nicely with nested structures: so errors will not be thrown too early or incorrectly reset when later a new structure is parsed.

@JLHwung JLHwung added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: parser labels Oct 21, 2020
@babel-bot
Copy link
Collaborator

babel-bot commented Oct 21, 2020

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/30997/

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 21, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 46ba92f:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@JLHwung JLHwung force-pushed the refactor-yield-await-tracking branch from 15773bb to ae9acf0 Compare October 21, 2020 20:50
@JLHwung JLHwung marked this pull request as ready for review October 21, 2020 21:20
// verify if the same logic is needed for yield.
if (oldYieldPos !== -1) this.state.yieldPos = oldYieldPos;

// Await is trickier than yield. When parsing a possible arrow function
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nicolo-ribaudo I really love reading these informative comments.

@@ -2,7 +2,7 @@
"type": "File",
"start":0,"end":35,"loc":{"start":{"line":1,"column":0},"end":{"line":3,"column":1}},
"errors": [
"SyntaxError: Can not use 'yield' as identifier inside a generator (2:3)",
"SyntaxError: yield is not allowed in generator parameters (2:3)",
Copy link
Contributor Author

@JLHwung JLHwung Oct 21, 2020

Choose a reason for hiding this comment

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

The error message seems a bit misleading as it is not a generator, although it is indeed in function parameters. V8 has changed this error message to "Yield expression not allowed in formal parameter" (code), I suggest we align with V8 and update the error in following PRs.

@@ -2,7 +2,7 @@
"type": "File",
"start":0,"end":36,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":36}},
"errors": [
"SyntaxError: Can not use 'await' as identifier inside an async function (1:20)"
"SyntaxError: Can not use 'await' as identifier inside an async function (1:14)"
Copy link
Contributor Author

@JLHwung JLHwung Oct 21, 2020

Choose a reason for hiding this comment

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

The position of await should be 14. An unexpected bugfix. 😄

@JLHwung JLHwung force-pushed the refactor-yield-await-tracking branch from b0f87cf to 8c92bd9 Compare October 24, 2020 01:13
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

This is awesome 🎉

@JLHwung JLHwung merged commit 2782a54 into babel:main Oct 26, 2020
@JLHwung JLHwung deleted the refactor-yield-await-tracking branch October 26, 2020 15:42
@JLHwung JLHwung mentioned this pull request Oct 26, 2020
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 26, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: parser PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants