js_printer: parenthesize missing-import void 0 on LHS of ** - #36134
js_printer: parenthesize missing-import void 0 on LHS of **#36134robobun wants to merge 1 commit into
Conversation
A namespace-import property with no matching export is rewritten to EImportIdentifier with import_item_status = Missing, which the printer replaces with undefined. Under --minify-syntax that emits 'void 0', and on the left of ** it was printed at Level::Exponentiation, below print_undefined's Level::Prefix wrap threshold, producing the SyntaxError 'void 0 ** 2'. Bump left_level to Call for EImportIdentifier when minify_syntax is on, mirroring the existing EBoolean arm. The non-Missing cases print an identifier or a member access, neither of which consults the incoming level, so the bump is a no-op there.
|
Reproduced with CI build 83526: the bundler test added here passes on every lane. The only red is |
WalkthroughThe JavaScript printer now applies call-level precedence to missing import identifiers on the left side of minified exponentiation. A bundler regression test verifies generated parentheses, warnings, and runtime output. ChangesExponentiation import identifier handling
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — targeted 6-line printer fix following the existing EBoolean arm pattern.
What was reviewed:
- Confirmed
print_undefinedwraps only whenlevel >= Prefix, so bumpingleft_leveltoCallunderminify_syntaxcorrectly forces(void 0). - Checked the
EImportIdentifierprint path (src/js_printer/lib.rs:3932): non-missing branches emit an identifier or member access and never consultlevel, so the unconditional bump is a no-op there — matching the test'snot.toMatch(/\(\w+\) \*\* 2/)assertion. - Test covers LHS (wrapped), RHS (unwrapped, right-associative), and an existing export (unwrapped), plus runs the bundle and asserts the warning.
Extended reasoning...
Overview
This PR adds a single EImportIdentifier arm to the BinPow left-operand match in binary_check_and_prepare (src/js_printer/lib.rs). Under --minify-syntax, a namespace-import property with no matching export is printed as void 0 via print_undefined, which only parenthesizes when the incoming level is >= Prefix. Since ** is right-associative, the left operand's level was staying at Exponentiation (< Prefix), producing void 0 ** 2 — a SyntaxError. The fix bumps left_level to Call, mirroring the existing EBoolean arm two lines above (booleans print as !0/!1 under the same option and need the same treatment).
Security risks
None. This is a code-printer precedence adjustment for an edge case (missing import used as ** LHS). No parsing of untrusted input, no I/O, no allocation.
Level of scrutiny
Low. The change is 6 lines that copy the pattern of the adjacent EBoolean/EBranchBoolean arm exactly, gated on the same minify_syntax flag. I verified in the EImportIdentifier print handler at line 3932 that the non-missing branches (namespace alias → member access, or plain identifier) do not read the incoming level, so the unconditional bump cannot introduce spurious parentheses — the test's negative assertions (not.toContain("(2 ** void 0)"), not.toMatch(/\(\w+\) \*\* 2/)) confirm this. Without minify_syntax the missing import prints as the identifier undefined, which is already a valid ** LHS, so the minify_syntax gate is correct.
Other factors
The new itBundled test is well-constructed per the repo's bundler-test conventions: it runs the output bundle (proving no SyntaxError), asserts exact output shape for the wrapped LHS and unwrapped RHS/existing-export, and checks the "will always be undefined" warning. It sits alongside the existing NamespaceImportMissingES6 test in the right file. The PR description explains the mechanism precisely and references the sibling PR #36133 for the enum-inlining path. No outstanding reviewer comments.
|
@robobun is this exactly what esbuild does? |
|
No. esbuild 0.28.1 has the same bug: esbuild's This PR adds the missing arm in the same shape as the existing |
2 participants
Repro
The linker emits a "will always be undefined" warning but still produces a bundle, and that bundle is a
SyntaxError(UnaryExpressionis not a valid left operand of**).Cause
A namespace-import property with no matching export is rewritten to
EImportIdentifierwithimport_item_status == Missing. The printer'sEImportIdentifierhandler callsprint_undefined(loc, level), which under--minify-syntaxemits unwrappedvoid 0whenlevel < Prefix. The BinPow left-operand match inbinary_check_and_preparehas noEImportIdentifierarm, soleft_levelstays atLevel::Exponentiation, and the output isvoid 0 ** 2.Fix
Add an
EImportIdentifierarm to the BinPow left-operand match, bumpingleft_leveltoCallwhenminify_syntaxis enabled. This mirrors the existingEBooleanarm (booleans print as!0/!1under the same option). When the import is not missing the printer emits an identifier or a member access, neither of which consults the incoming level, so the bump is a no-op there; and withoutminify_syntaxthe printer emits the identifierundefined, which is already a valid left operand.Sibling of #36133, which handles the enum-inlining path on the same operand.
Verification
New
importstar/NamespaceImportMissingES6ExponentiationLHSintest/bundler/esbuild/importstar.test.tsfails against the unfixed build (bundle containsvoid 0 ** 2and does not run) and passes with the fix. It also asserts no extra parentheses are added for the right operand or for an existing export.[stamp-90s] gate passed · iteration 0 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 1 passed · 0 rejected · iteration 0
evidence per changed file
self-review · no surviving concerns
28 concerns were raised and did not survive verification.