-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Properly disallow yield
in bodyless arrows
#61976
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
base: main
Are you sure you want to change the base?
Conversation
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
1 similar comment
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures that yield
expressions are disallowed in concise (bodyless) arrow function bodies as required by the ES6 spec and adds a corresponding test case.
- Disable the
yield
parsing context when parsing arrow expression bodies - Add a conformance test and update baselines to validate that
yield
in a bodyless arrow yields an error - Update
.types
,.symbols
,.js
, and.errors.txt
baselines to reflect the new error
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/compiler/parser.ts | Disable yield context around arrow body parsing and restore afterward |
tests/cases/conformance/es6/yieldExpressions/YieldExpression20_es6.ts | New test case verifying yield is disallowed in a concise arrow |
tests/baselines/reference/YieldExpression20_es6.types | Updated type-checking baseline for the new test |
tests/baselines/reference/YieldExpression20_es6.symbols | Updated symbol baseline for the new test |
tests/baselines/reference/YieldExpression20_es6.js | Updated transpiled JS baseline for the new test |
tests/baselines/reference/YieldExpression20_es6.errors.txt | Updated error-message baseline for the new test |
Comments suppressed due to low confidence (2)
src/compiler/parser.ts:5562
- [nitpick] Add a brief comment explaining that we temporarily disable the yield parsing context here to enforce the ES6 rule that concise arrow bodies are not generator bodies.
const savedYieldContext = inYieldContext();
src/compiler/parser.ts:5563
- [nitpick] Consider using a
doOutsideOfContext(NodeFlags.YieldContext, ...)
helper (similar to await) to encapsulate context toggling, improving consistency and readability.
setYieldContext(false);
Pretty sure this is correct but @typescript-bot test it |
Hey @jakebailey, the results of running the DT tests are ready. Everything looks the same! |
@jakebailey Here are the results of running the user tests with tsc comparing Everything looks good! |
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@jakebailey Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
No description provided.