Allow Runtime Errors for Function Call Assignment Targets
#17446
Conversation
liuxingbaoyu
commented
Jul 19, 2025
liuxingbaoyu
commented
| Q | A |
|---|---|
| Fixed Issues? | tc39/test262#4459 |
| Patch: Bug Fix? | |
| Major: Breaking Change? | |
| Minor: New Feature? | |
| Tests Added + Pass? | Yes |
| Documentation PR Link | |
| Any Dependency Changes? | |
| License | MIT |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/59960 |
| language/expressions/assignmenttargettype/direct-callexpression-in-logical-assignment.js(default) | ||
| language/expressions/assignmenttargettype/parenthesized-callexpression-in-logical-assignment.js(default) | ||
| language/expressions/logical-assignment/lgcl-and-assignment-operator-non-simple-lhs.js(default) | ||
| language/expressions/logical-assignment/lgcl-nullish-assignment-operator-non-simple-lhs.js(default) | ||
| language/expressions/logical-assignment/lgcl-or-assignment-operator-non-simple-lhs.js(default) |
There was a problem hiding this comment.
It seems impossible to distinguish them with the information available.
I noticed that the engine262 tests at tc39/test262#4459 also fail.
Do we need to ask in test262?
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, the cases involving the logical assignment operators remain syntax errors.
(See tc39/ecma262#2193 (comment) for the origin story there.)
liuxingbaoyu
changed the title
Runtime Errors for Function Call Assignment TargetsRuntime Errors for Function Call Assignment Targets
|
commit: |
liuxingbaoyu
force-pushed
the
fix-test262-pr4459
branch
from
5a3ca78 to
6435d5d
Compare
liuxingbaoyu
added
PR: Spec Compliance 👓
JLHwung
left a comment
JLHwung
left a comment
There was a problem hiding this comment.
Nice work, left a few comments on tests & docs.
| type, | ||
| disallowCallExpression || | ||
| (expression.type === "CallExpression" && | ||
| expression.callee.type === "Import"), |
There was a problem hiding this comment.
We should also exclude Super here, could you also a test for the super() case?
The Import check should be Babel 7 only since we will generate ImportExpression in Babel 8.
There was a problem hiding this comment.
It suddenly occurred to me that Babel 8 still supports createImportExpressions as false.
There was a problem hiding this comment.
OK, if that's the case let's keep the import check as-is.
| */ | ||
| isValidLVal( | ||
| type: string, | ||
| disallowCallExpression: boolean, |
There was a problem hiding this comment.
Could you also update the JSDoc comments above?
| checkClashes: Set<string> | false = false, | ||
| strictModeChanged: boolean = false, | ||
| hasParenthesizedAncestor: boolean = false, | ||
| disallowCallExpression: boolean = false, |
| async() &&= 1; | ||
|
|
||
| import("")++; |
There was a problem hiding this comment.
Since these examples are invalid in both annex-B and non-annex-B mode, could you extract these cases to invalid-assignment-target-type? The current one can also be renamed to valid-*.
| async()++; | ||
| ++async(); | ||
| for (async() in [1]) { } | ||
| for (async() of [1]) {} |
There was a problem hiding this comment.
Could you also add a negative case for parenthesized expressions, such as (f()) = 1. Since the AssignmentTargetType will pass through the parentheses.
| async()++; | ||
| ++async(); | ||
| for (async() in [1]) { } | ||
| for (async() of [1]) {} |
There was a problem hiding this comment.
Could you also add a test case for call expression nested within array pattern / object pattern? The AssignmentTargetType does not pass through the array / object pattern, so examples like [f()] = [] should be rejected regardless of the annex B mode.
|
Thanks for the amazing review! |
JLHwung
left a comment
JLHwung
left a comment
There was a problem hiding this comment.
The new changes look good to me, thank you!
github-actions
bot
added
the
outdated
5 participants
