parser: fix "Scope mismatch while visiting" panic from dropped TypeScript declare statements - #31231
Conversation
The `declare` statement branch parses the following statement and then drops
it (returning a bare TypeScript no-op, or just the extracted bindings for
"export declare var" inside a namespace). Scopes recorded while parsing that
dropped statement were left in scopes_in_order, so the visit pass hit
"panic: Scope mismatch while visiting" as soon as a later statement pushed a
scope. The same leak existed for the statements inside "declare global { }".
Discard the recorded scopes in both places, the same way the decorator path
in the same branch already does.
|
Warning Review limit reached
Your plan currently allows 2 reviews/hour. Refill in 12 minutes and 7 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR fixes a scope-tracking bug in the TypeScript parser where ChangesScope-stack management in TypeScript declare statements
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Updated 7:17 PM PT - May 22nd, 2026
✅ @robobun, your commit 747fd3834bc01cda55c764b49f94864ecf61b520 passed in 🧪 To try this PR locally: bunx bun-pr 31231That installs a local version of the PR into your bun-31231 --bun |
There was a problem hiding this comment.
LGTM — targeted scope-discard for dropped declare bodies, mirrors the existing decorator discard pattern; nit addressed.
Extended reasoning...
Overview
Two-file change: src/js_parser/parse/parse_stmt.rs adds discard_scopes_up_to() calls in the TsStmtDeclare arm of parse_stmt_fallthrough_ts_keyword — once for the dropped declare global { ... } body and once (in a new else branch) for the general declare <stmt> path when no decorators are present. test/bundler/transpiler/scope-mismatch-panic.test.ts gains a 5-case test.concurrent.each regression suite covering each panic shape from the description.
Security risks
None. This is parser-internal scope-record bookkeeping; no user-controlled data flows into new sinks, no auth/crypto/permissions surface.
Level of scrutiny
The JS/TS parser is a hot, critical path, so I checked the change carefully rather than treating it as mechanical. The key correctness invariant holds: every code path after the recursive parse_stmt(opts) either drops the result entirely (returns S::TypeScript {}) or reduces it to bindings-only (extract_decls_for_binding → S::Local with no initializers), so no recorded scope from that parse is ever consumed by the visit pass — discarding them is strictly safe. I also confirmed via grep that the well-formed declare class / declare namespace / declare enum paths already self-clean (pop_and_discard_scope in parse/mod.rs:725, parse_typescript.rs:366, and the enum path skips the push at parse_typescript.rs:555), so the new outer discard is a no-op for those and only takes effect on the leaking fallthrough shapes (labeled stmts, blocks, arrow initializers). The decorator branch already discards to decs.scope_index (which is ≤ the new local index), so the else placement is correct.
Other factors
- The fix is the same idiom as the adjacent
opts.ts_decoratorsdiscard, so it reads as filling a gap rather than introducing a new mechanism. - PR description verified against
ts.test.ts(57 pass) andtranspiler.test.js(114 pass), and the bug-hunter pass found nothing. - My earlier
test.concurrent.eachnit was applied in 747fd38 and the inline thread is resolved. - The robobun macOS x64 build failure on 9467cf3 is a
scripts/build/ci.tsinfra failure, not a test regression from this diff; the follow-up commit only touched the test file. - No CODEOWNERS entry covers
src/js_parser/.
… a larger expression (#31239) ### Repro Found by parser fuzzing (46 bytes): ```ts bun -e 'new Bun.Transpiler({ loader: "ts" }).transformSync("declare = (...t) => R;e((a) => {(u=> uge);\r\n})")' panic: Internal error: attempted to call popScope() on the topmost scope ``` Minimized: ```ts declare = t => 0; () => () => 0 ``` The same desync is reachable through the other TS contextual statement keywords, including from valid TypeScript: ```ts abstract = () => {} class Foo { m() { return () => () => 0 } } // panic: Scope mismatch while visiting type = (t) => 0 Foo = number; () => () => 0 // panic: popScope() on the topmost scope namespace = (t) => 0 Foo { () => () => 0 } // panic: Scope mismatch while visiting ``` Reproduces on 1.3.x and current canary; it is a faithful port of the pre-existing Zig parser behavior rather than a Rust-port regression. ### Cause In `parse_stmt_fallthrough` (`src/js_parser/parse/parse_stmt.rs`), the TS contextual-keyword handling (`type`/`interface`/`namespace`/`module`/`abstract`/`global`/`declare`) ran whenever the statement *began* with that identifier token. esbuild additionally requires that the parsed expression is still exactly that bare identifier; the port lost that nesting. So for `declare = (...t) => R`, the whole assignment is parsed first (recording the arrow function's scopes in `scopes_in_order`), and then the `declare` branch throws the expression away and emits a TS no-op. The orphaned scope records no longer line up with the AST, so the visit pass enters a scope whose parent chain is shallower than expected — `pop_scope()` walks off the module scope in release builds (`popScope() on the topmost scope`), or the order sanity-check fires in debug builds (`Scope mismatch while visiting`). The statement was also silently dropped from the output (`declare = t => 0;` → ``), and `interface = t => 0` produced a bogus parse error. ### Fix Move the TS keyword handling inside the `ExprData::EIdentifier` check, matching esbuild's structure. If the keyword identifier was consumed as part of a larger expression, we now fall through to the normal `SExpr` path: - `declare = t => 0;`, `abstract = () => {}` etc. are preserved as expression statements (same output as esbuild/tsc), so no scopes are orphaned. - Invalid suffix forms like `type = (t) => 0 Foo` now produce the normal `Expected ";" but found "Foo"` parse error (same as esbuild) instead of panicking. - Real ambient declarations (`declare const x: number`, `declare function f(): void`, `interface Foo {}`, `namespace Foo {}`, …) still parse through the keyword path because the expression is the bare identifier there — no behavior change. Related open PRs in this area, both complementary rather than overlapping in coverage: - #30008 gates the `declare`/`interface` arms on the same bare-identifier condition and additionally refines ASI/decorator/lookahead handling for the *bare*-identifier forms (`declare()` wrapped by its caller, `declare\nfoo()`, `@dec declare()`, `export default interface\nFoo {}`). It does not gate `type`/`namespace`/`module`/`abstract`, so the panics above through those keywords remain without this change. Whichever lands second needs a trivial rebase of the caller hunk. - #31231 discards the scopes of statements that a *real* ambient `declare` parses and then drops (`declare foo: bar`, `declare global { ... }`), which this change does not address. ### Verification New tests in `test/bundler/transpiler/transpiler.test.js` (`Bun.Transpiler > TypeScript`): - `contextual keywords used as plain identifiers keep their statements` — asserts `declare = t => 0;`, `declare.foo = 1;`, `interface = t => 0;`, `abstract = () => {}\nclass Foo { ... }`, etc. survive transpilation, and that real ambient `declare` forms are still erased. - `scope tracking stays balanced when a contextual keyword starts a larger expression` — the original fuzz input plus the minimized `declare`/`abstract`/`type`/`namespace`/`module` variants; each one panics without the fix. ``` USE_SYSTEM_BUN=1 bun test test/bundler/transpiler/transpiler.test.js -t "contextual" # assertion failure + popScope panic (bug) bun bd test test/bundler/transpiler/transpiler.test.js # 116 pass, 0 fail bun bd test test/bundler/esbuild/ts.test.ts # 57 pass, 0 fail bun bd test test/bundler/transpiler/scope-mismatch-panic.test.ts # 3 pass, 0 fail ``` --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…ript `declare` statements (#31231) ### Repro Found by parser fuzzing. Minimized: ```ts declare module : es2015 class Foo {} ``` ``` bun -e 'new Bun.Transpiler({loader:"tsx"}).transformSync("declare module : es2015\nclass Foo {}\n")' panic: Scope mismatch while visiting ``` Reproduces on 1.3.14 and current canary, so it is a faithful port of the pre-existing parser behavior rather than a regression. The same panic is reachable through several other shapes of `declare`: ```ts declare foo: bar class Foo {} ``` ```ts declare const x = () => {}; class Foo {} ``` ```ts declare global { if (1) { let x = 1 } } class Foo {} ``` (esbuild 0.21.5 panics on the `declare global` variant too, with its equivalent "Expected scope ... found scope" internal error.) ### Cause `declare module : es2015` never reaches the namespace parser — after `declare`, the guard for `module`/`namespace` requires an identifier or string literal as the name, so `module : es2015` falls through and is parsed as a **labeled statement**, which pushes (and records) a `Label` scope. The `declare` branch in `parse_stmt.rs` then throws the parsed statement away and returns a bare TypeScript no-op, but the scopes recorded while parsing it stay in `scopes_in_order`. When the visit pass later pushes the scope for the next scope-creating statement (the `class`), the recorded order no longer lines up and the parser panics with "Scope mismatch while visiting". The same leak exists for: - any dropped `declare <stmt>` whose parse recorded scopes (labeled statements, blocks, `if`, arrow-function initializers on `declare const`, …) - the statements inside `declare global { ... }`, which are parsed and discarded without discarding their scopes - `export declare var/let/const` inside a namespace, which is reduced to just its bindings (initializer scopes are dropped) ### Fix In the `declare` branch (`src/js_parser/parse/parse_stmt.rs`), record `scopes_in_order.len()` before parsing the declared statement and call `discard_scopes_up_to()` afterwards — the same thing the decorator path in that branch already does. Same treatment for the `declare global { ... }` body. This only removes scope records for statements the parser drops anyway, so output for valid `declare` code is unchanged. ### Verification - Added 5 cases to `test/bundler/transpiler/scope-mismatch-panic.test.ts`; all 5 panic with "Scope mismatch while visiting" before the fix and pass with it. - The original 431-byte fuzz input transpiles cleanly with the fix. - `bun bd test test/bundler/esbuild/ts.test.ts` (57 pass) and `test/bundler/transpiler/transpiler.test.js` (114 pass) are clean, so existing `declare`/namespace behavior is unchanged. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
… a larger expression (#31239) ### Repro Found by parser fuzzing (46 bytes): ```ts bun -e 'new Bun.Transpiler({ loader: "ts" }).transformSync("declare = (...t) => R;e((a) => {(u=> uge);\r\n})")' panic: Internal error: attempted to call popScope() on the topmost scope ``` Minimized: ```ts declare = t => 0; () => () => 0 ``` The same desync is reachable through the other TS contextual statement keywords, including from valid TypeScript: ```ts abstract = () => {} class Foo { m() { return () => () => 0 } } // panic: Scope mismatch while visiting type = (t) => 0 Foo = number; () => () => 0 // panic: popScope() on the topmost scope namespace = (t) => 0 Foo { () => () => 0 } // panic: Scope mismatch while visiting ``` Reproduces on 1.3.x and current canary; it is a faithful port of the pre-existing Zig parser behavior rather than a Rust-port regression. ### Cause In `parse_stmt_fallthrough` (`src/js_parser/parse/parse_stmt.rs`), the TS contextual-keyword handling (`type`/`interface`/`namespace`/`module`/`abstract`/`global`/`declare`) ran whenever the statement *began* with that identifier token. esbuild additionally requires that the parsed expression is still exactly that bare identifier; the port lost that nesting. So for `declare = (...t) => R`, the whole assignment is parsed first (recording the arrow function's scopes in `scopes_in_order`), and then the `declare` branch throws the expression away and emits a TS no-op. The orphaned scope records no longer line up with the AST, so the visit pass enters a scope whose parent chain is shallower than expected — `pop_scope()` walks off the module scope in release builds (`popScope() on the topmost scope`), or the order sanity-check fires in debug builds (`Scope mismatch while visiting`). The statement was also silently dropped from the output (`declare = t => 0;` → ``), and `interface = t => 0` produced a bogus parse error. ### Fix Move the TS keyword handling inside the `ExprData::EIdentifier` check, matching esbuild's structure. If the keyword identifier was consumed as part of a larger expression, we now fall through to the normal `SExpr` path: - `declare = t => 0;`, `abstract = () => {}` etc. are preserved as expression statements (same output as esbuild/tsc), so no scopes are orphaned. - Invalid suffix forms like `type = (t) => 0 Foo` now produce the normal `Expected ";" but found "Foo"` parse error (same as esbuild) instead of panicking. - Real ambient declarations (`declare const x: number`, `declare function f(): void`, `interface Foo {}`, `namespace Foo {}`, …) still parse through the keyword path because the expression is the bare identifier there — no behavior change. Related open PRs in this area, both complementary rather than overlapping in coverage: - #30008 gates the `declare`/`interface` arms on the same bare-identifier condition and additionally refines ASI/decorator/lookahead handling for the *bare*-identifier forms (`declare()` wrapped by its caller, `declare\nfoo()`, `@dec declare()`, `export default interface\nFoo {}`). It does not gate `type`/`namespace`/`module`/`abstract`, so the panics above through those keywords remain without this change. Whichever lands second needs a trivial rebase of the caller hunk. - #31231 discards the scopes of statements that a *real* ambient `declare` parses and then drops (`declare foo: bar`, `declare global { ... }`), which this change does not address. ### Verification New tests in `test/bundler/transpiler/transpiler.test.js` (`Bun.Transpiler > TypeScript`): - `contextual keywords used as plain identifiers keep their statements` — asserts `declare = t => 0;`, `declare.foo = 1;`, `interface = t => 0;`, `abstract = () => {}\nclass Foo { ... }`, etc. survive transpilation, and that real ambient `declare` forms are still erased. - `scope tracking stays balanced when a contextual keyword starts a larger expression` — the original fuzz input plus the minimized `declare`/`abstract`/`type`/`namespace`/`module` variants; each one panics without the fix. ``` USE_SYSTEM_BUN=1 bun test test/bundler/transpiler/transpiler.test.js -t "contextual" # assertion failure + popScope panic (bug) bun bd test test/bundler/transpiler/transpiler.test.js # 116 pass, 0 fail bun bd test test/bundler/esbuild/ts.test.ts # 57 pass, 0 fail bun bd test test/bundler/transpiler/scope-mismatch-panic.test.ts # 3 pass, 0 fail ``` --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…dropped class members (#31340) ### Repro Found by parser fuzzing on `1.4.0-canary.1`. Minimized (55 bytes): ```ts class C { @((td) => { })oo(): oo; h(@(() => {})ny) {}} ``` ``` bun -e 'new Bun.Transpiler({loader:"tsx", target:"bun", minifyWhitespace:true}).transformSync(atob("Y2xhc3MgQyB7DUAoKHRkKSA9PiB7IH0pb28oKTogb287CiBoKEAoKCkgPT4ge30pbnkpIHt9fQ=="))' panic: Scope mismatch while visiting ``` The same panic is reachable whenever a scope-creating decorator — or computed key — sits on a class member that the parser drops, followed by any later scope of a different kind: ```ts class C { @((td) => { })oo(): oo; h() { { let x; } } } abstract class C { @((td) => { })abstract oo(): void; h() { { let x; } } } class C { @((td) => { })declare oo(): void; h() { { let x; } } } class C { @((td) => { })[key: string]: any; h() { { let x; } } } class C { @((td) => { })oo(): oo; } function f() { { let x; } } class C { [((x) => x)('foo')](): void; h() { { let x; } } } ``` The Zig parser has the same structure, so this is a faithfully-ported long-standing bug rather than a port regression — same family as #31231 (dropped `declare` statements). ### Cause In `parse_class`, decorators for each property are parsed (in the class-body scope) *before* `parse_property` decides whether the member is kept, and a computed key is parsed before the member's own function scope is pushed. When the member turns out to be dropped — a TypeScript overload signature (method with no body), an abstract/`declare` method, or an index signature — `parse_property` returns `None` and only discards the scopes it pushed itself (the method's `FunctionArgs` scope). Scopes recorded while parsing the member's decorators or computed key (the arrow functions above) stay in `scopes_in_order` even though those expressions are dropped from the AST and never visited. The visit pass then consumes those orphaned entries for the *next* scope it pushes. In release builds only the scope kind is checked, so the panic fires once a scope of a different kind lines up against the leftover entry — here the arrow in `h`'s parameter decorator (`FunctionArgs` vs the orphaned arrow-body `FunctionBody` entry). Debug builds also check the location, so any later scope trips it. ### Fix `src/js_parser/parse/mod.rs` (`parse_class`): record `scopes_in_order.len()` before parsing a property's decorators, and when `parse_property` drops the member, call `discard_scopes_up_to()` so everything recorded for the dropped member (decorators and computed key) is discarded with it. This is the same pattern the statement-level decorator path already uses for `@decorator declare class Foo {}` (`DeferredTsDecorators::scope_index`). Output for kept members is unchanged; the discard only runs on members the parser already drops. ### Verification - Added 7 cases to `test/bundler/transpiler/scope-mismatch-panic.test.ts` (overload signature, abstract method, declare method, index signature, computed key, plus the original fuzz input). All 7 panic with "Scope mismatch while visiting" before the fix and pass with it; the 8 existing cases in that file still pass. - The original fuzz input now transpiles cleanly (`class C{h(ny){}}` plus decorator metadata preamble). - No regressions: `decorators.test.ts`, `decorator-metadata.test.ts`, `es-decorators.test.ts`, `es-decorators-esbuild.test.ts` (206 pass), `test/bundler/esbuild/ts.test.ts` + `test/bundler/transpiler/transpiler.test.js` (203 pass, 0 fail). - `cargo check -p bun_js_parser` and `cargo clippy -p bun_js_parser` are clean.
…emplates (#31693) ### Repro ```ts // macro.ts export function mac(...args: any[]) { return "x"; } // index.ts import { mac } from './macro.ts' with { type: 'macro' }; mac`a${() => { let q = 1; }}b`; function g() { { let y = 1; } } ``` ``` $ bun index.ts panic: Scope mismatch while visiting ``` Any tagged-template macro invocation whose interpolations contain a scope-creating expression (arrow, function, class) panics instead of reporting the intended `template literal macro invocations are not supported` error. The dead-code variant (`false && mac`…`` `), the macros-disabled path, and the node_modules path crash the same way. Panics on release builds (kind mismatch) and debug builds (loc mismatch); same structure existed in the Zig-era `visitExpr.zig`, so this predates the Rust port. Crash signature matches Sentry [BUN-3BYK](https://bun-p9.sentry.io/issues/7504990169/) (`Scope mismatch while visiting`, also seen in Zig-era releases). ### Cause The parser records every scope pushed during the parse pass in `scopes_in_order`; the visit pass replays them in the same order and panics on divergence. In `e_template` (`src/js_parser/visit/visit_expr.rs`), when the tag resolves to a macro ref, **every** dispatch outcome returns before the `for part in e_.parts_mut()` visit loop: - `is_control_flow_dead` → replaced with `undefined` - `no_macros` → error + `undefined` - `node_modules` → error + `undefined` - `macro_context.call(...)` failure → plain `return` (and template invocations currently always fail with `template literal macro invocations are not supported`, `src/js_parser_jsc/Macro.rs`) The scopes recorded for arrows/functions inside the interpolations are never consumed, so the next scope the visit pass pushes reads a stale entry and trips the `Scope mismatch while visiting` check. Same bug family as #31231 / #31340 / #31533 (constructs dropped without consuming/discarding their recorded scopes). ### Fix Visit the template parts right after visiting the tag, **before** the macro dispatch — the same ordering `e_call` uses (arguments are visited before its macro handling). All dispatch paths may then freely replace the expression: the parts' scope entries have already been consumed. The fall-through case no longer re-visits parts. Also syncs `Cargo.lock` with `bun_bin`'s manifest (`bstr` was added to `Cargo.toml` in 90f334a without the lock update; any local cargo invocation regenerates this line). ### Verification - 3 new tests in `test/bundler/transpiler/scope-mismatch-panic.test.ts` (live tag, dead-flow, namespace-member tag). All three panic without the fix and pass with it. - `test/bundler/transpiler/{scope-mismatch-panic,macro-test,transpiler,template-literal}.test.ts` and `test/bundler/bundler_string.test.ts` all pass. Note: #30545 (tagged-template macro support, feature) inserts a parts visit before the macro *call*, which would cover the live path but not the dead-flow / macros-disabled / node_modules returns. This fix is independent and minimal; #30545 rebases on top by dropping its duplicate visit loop (its fold-flag wrapper can stay).
Repro
Found by parser fuzzing. Minimized:
Reproduces on 1.3.14 and current canary, so it is a faithful port of the pre-existing parser behavior rather than a regression.
The same panic is reachable through several other shapes of
declare:(esbuild 0.21.5 panics on the
declare globalvariant too, with its equivalent "Expected scope ... found scope" internal error.)Cause
declare module : es2015never reaches the namespace parser — afterdeclare, the guard formodule/namespacerequires an identifier or string literal as the name, somodule : es2015falls through and is parsed as a labeled statement, which pushes (and records) aLabelscope.The
declarebranch inparse_stmt.rsthen throws the parsed statement away and returns a bare TypeScript no-op, but the scopes recorded while parsing it stay inscopes_in_order. When the visit pass later pushes the scope for the next scope-creating statement (theclass), the recorded order no longer lines up and the parser panics with "Scope mismatch while visiting".The same leak exists for:
declare <stmt>whose parse recorded scopes (labeled statements, blocks,if, arrow-function initializers ondeclare const, …)declare global { ... }, which are parsed and discarded without discarding their scopesexport declare var/let/constinside a namespace, which is reduced to just its bindings (initializer scopes are dropped)Fix
In the
declarebranch (src/js_parser/parse/parse_stmt.rs), recordscopes_in_order.len()before parsing the declared statement and calldiscard_scopes_up_to()afterwards — the same thing the decorator path in that branch already does. Same treatment for thedeclare global { ... }body.This only removes scope records for statements the parser drops anyway, so output for valid
declarecode is unchanged.Verification
test/bundler/transpiler/scope-mismatch-panic.test.ts; all 5 panic with "Scope mismatch while visiting" before the fix and pass with it.bun bd test test/bundler/esbuild/ts.test.ts(57 pass) andtest/bundler/transpiler/transpiler.test.js(114 pass) are clean, so existingdeclare/namespace behavior is unchanged.