From 2b1dc3b9fbf655dc7bf5076617cbe47e72b0ba0c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 18 May 2026 03:49:21 +0000 Subject: [PATCH 1/8] js_parser: tolerate duplicate scope locations after error recovery Parsing ` P<'a, TYPESCRIPT, SCAN_O if cfg!(debug_assertions) { // Enforce that scope locations are strictly increasing to help catch bugs - // where the pushed scopes are mismatched between the first and second passes - if !self.scopes_in_order.is_empty() { + // where the pushed scopes are mismatched between the first and second passes. + // Skip this check after an error has been logged: error recovery can legitimately + // cause `next()` to return a token at a position that was already consumed (e.g. a + // zero-length token produced after `TSyntaxError`), which makes the parser push + // multiple scopes at the same source offset. Release builds just report the error + // and recover, so the debug assertion should not be stricter than release. + if !self.log().has_errors() && !self.scopes_in_order.is_empty() { let mut last_i = self.scopes_in_order.len() - 1; while self.scopes_in_order[last_i].is_none() && last_i > 0 { last_i -= 1; diff --git a/test/regression/issue/jsx-template-string-crash.test.ts b/test/regression/issue/jsx-template-string-crash.test.ts index f4dea9413e6..2a9d778f7e6 100644 --- a/test/regression/issue/jsx-template-string-crash.test.ts +++ b/test/regression/issue/jsx-template-string-crash.test.ts @@ -28,3 +28,44 @@ test("JSX lexer should not crash with slice bounds issues", async () => { `); expect(normalizeBunSnapshot(stdout.toString())).toMatchInlineSnapshot(`""`); }); + +test("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds", async () => { + // Previously, parsing `/[eval]:1:32 + + 1 | export function x(){return/[eval]:1:34" + `); + expect(stdout).toBe(""); + expect(exitCode).toBe(1); +}); From 3c9a3874e5020029f45e863c64657a5ed1ff41de Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 03:51:38 +0000 Subject: [PATCH 2/8] [autofix.ci] apply automated fixes --- test/regression/issue/jsx-template-string-crash.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/regression/issue/jsx-template-string-crash.test.ts b/test/regression/issue/jsx-template-string-crash.test.ts index 2a9d778f7e6..406befe3e9f 100644 --- a/test/regression/issue/jsx-template-string-crash.test.ts +++ b/test/regression/issue/jsx-template-string-crash.test.ts @@ -49,11 +49,7 @@ test("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds stdout: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([ - proc.stdout.text(), - proc.stderr.text(), - proc.exited, - ]); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(normalizeBunSnapshot(stderr.toString().replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(` "1 | export function x(){return Date: Mon, 18 May 2026 04:13:00 +0000 Subject: [PATCH 3/8] js_parser: compare against orig_error_count, mark new test concurrent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback (#30969): - `has_errors()` checks the absolute error count on the shared `Log`, which may already carry errors from earlier files in the same parse session. Capture the baseline at `P::init` (`orig_error_count`) and compare `log.errors > self.orig_error_count` instead — mirrors the `orig_error_count` pattern at `parse_entry.rs:762`/`:876` that already gates the visit-pass bailout. Without this, an unrelated error from an earlier file would silently disable the debug-only scope-location invariant for every subsequent file in the session. - Mark the new #30959 regression test as `test.concurrent` per `test/CLAUDE.md` — it spawns an async subprocess and shares no state with the pre-existing test in the file. --- src/js_parser/p.rs | 30 +++++++++++++++---- .../issue/jsx-template-string-crash.test.ts | 2 +- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 4f284db9da7..f92e7dff74e 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -457,6 +457,14 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> { // symbols to handle declaring a hoisted "var" symbol in a nested scope and // binding a name to it in a parent or sibling scope. pub scopes_in_order: ScopeOrderList<'a>, + // Error count on the shared `Log` at the time this `P` was constructed. + // The log may already carry errors from earlier files in the same parse + // session, so the parse-pass "did this file log anything?" question has + // to be phrased relative to this baseline — matches the + // `orig_error_count` pattern at `parse_entry.rs:762` that gates the + // visit-pass bailout. Currently read only by the debug-only + // strictly-increasing scope-location check in `push_scope_for_parse_pass`. + pub orig_error_count: u32, // Shared slice: the visit pass only ever *reads* `ScopeOrder` (which is // `Copy`) and advances/reslices the cursor. A `&'a mut [_]` here forced // raw-ptr round-trips at the enum-preprocess save/restore sites and @@ -3642,12 +3650,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O if cfg!(debug_assertions) { // Enforce that scope locations are strictly increasing to help catch bugs // where the pushed scopes are mismatched between the first and second passes. - // Skip this check after an error has been logged: error recovery can legitimately - // cause `next()` to return a token at a position that was already consumed (e.g. a - // zero-length token produced after `TSyntaxError`), which makes the parser push - // multiple scopes at the same source offset. Release builds just report the error - // and recover, so the debug assertion should not be stricter than release. - if !self.log().has_errors() && !self.scopes_in_order.is_empty() { + // Skip this check after *this file* has logged an error: error recovery can + // legitimately cause `next()` to return a token at a position that was + // already consumed (e.g. a zero-length token produced after `TSyntaxError`), + // which makes the parser push multiple scopes at the same source offset. + // Release builds just report the error and recover; the debug assertion + // should not be stricter than release. Compare against `orig_error_count` + // so an unrelated error from an earlier file in the same session doesn't + // silently disable this invariant for a file that parses cleanly — mirrors + // the visit-pass bailout at `parse_entry.rs:876`. + let file_has_errors = self.log().errors > self.orig_error_count; + if !file_has_errors && !self.scopes_in_order.is_empty() { let mut last_i = self.scopes_in_order.len() - 1; while self.scopes_in_order[last_i].is_none() && last_i > 0 { last_i -= 1; @@ -9195,6 +9208,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O current_scope: scope, module_scope: scope, scopes_in_order: scope_order, + // SAFETY: `log` is a `NonNull` whose pointee outlives `'a` + // (enforced by `Parser::init`); reading the `errors` scalar here + // mirrors `parse_entry.rs:762` which reads it through the lexer + // alias at the same point in the pipeline. + orig_error_count: unsafe { log.as_ref() }.errors, needs_jsx_import: false, // Zig: `if (scan_only) false else void` — NeedsJSXType collapsed to `bool` lexer, diff --git a/test/regression/issue/jsx-template-string-crash.test.ts b/test/regression/issue/jsx-template-string-crash.test.ts index 406befe3e9f..926459dbdc2 100644 --- a/test/regression/issue/jsx-template-string-crash.test.ts +++ b/test/regression/issue/jsx-template-string-crash.test.ts @@ -29,7 +29,7 @@ test("JSX lexer should not crash with slice bounds issues", async () => { expect(normalizeBunSnapshot(stdout.toString())).toMatchInlineSnapshot(`""`); }); -test("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds", async () => { +test.concurrent("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds", async () => { // Previously, parsing ` Date: Mon, 18 May 2026 04:58:01 +0000 Subject: [PATCH 4/8] lexer: step past unknown byte on TSyntaxError in next_inside_jsx_element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Jarred pointed out (#30969) that the previous fix — skipping the debug assertion once the file logs an error — was hiding a real bug rather than fixing it. The root cause is in next_inside_jsx_element's TSyntaxError branch. On an unknown character it sets `end = current` but leaves `code_point` holding that same byte, so `current` never advances past it. When the caller recovers via `expect().next()`, the main lexer's next() re-dispatches on that still-in-code_point byte, using the updated `end` as the new `start`. step_with() then reads the *following* byte (the second '(' in `` regression snapshot in jsx-template-string-crash.test.ts shifts from `Unexpected >` at col 37 to `Unterminated string literal` at col 35 — a more accurate diagnostic (the second backtick is what the recovery lexer treats as a template opener, which never terminates) rather than a behavioural regression. Reverts the `orig_error_count` workaround on P. --- src/js_parser/lexer.rs | 14 ++++++++++ src/js_parser/p.rs | 27 ++----------------- .../issue/jsx-template-string-crash.test.ts | 6 ++--- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/js_parser/lexer.rs b/src/js_parser/lexer.rs index 7dedbc2526a..c6e7518e5bc 100644 --- a/src/js_parser/lexer.rs +++ b/src/js_parser/lexer.rs @@ -3048,6 +3048,20 @@ lexer_impl_header! { self.end = self.current; self.token = T::TSyntaxError; + // Advance `code_point`/`current` past the bad byte so that a + // subsequent recovery `next()` (e.g. via `expect(...)` inside + // `parse_jsx_prop_value_identifier`) dispatches on the *following* + // byte instead of re-dispatching on the still-in-`code_point` bad + // byte. Without this step the recovery `next()` synthesises a + // zero-length token at the offset of the next byte, and the byte + // after that then gets tokenised a second time at the same + // `start` — the parser pushes two `FunctionArgs` scopes at that + // offset in `parse_paren_expr` and trips the strict-monotonicity + // debug assertion in `push_scope_for_parse_pass` (see #30959). + // `end` was already advanced above, so the step below only moves + // `current`/`code_point` forward and leaves the error range + // `[start, end)` intact. + self.step(); } } diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index f92e7dff74e..aed559be598 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -457,14 +457,6 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> { // symbols to handle declaring a hoisted "var" symbol in a nested scope and // binding a name to it in a parent or sibling scope. pub scopes_in_order: ScopeOrderList<'a>, - // Error count on the shared `Log` at the time this `P` was constructed. - // The log may already carry errors from earlier files in the same parse - // session, so the parse-pass "did this file log anything?" question has - // to be phrased relative to this baseline — matches the - // `orig_error_count` pattern at `parse_entry.rs:762` that gates the - // visit-pass bailout. Currently read only by the debug-only - // strictly-increasing scope-location check in `push_scope_for_parse_pass`. - pub orig_error_count: u32, // Shared slice: the visit pass only ever *reads* `ScopeOrder` (which is // `Copy`) and advances/reslices the cursor. A `&'a mut [_]` here forced // raw-ptr round-trips at the enum-preprocess save/restore sites and @@ -3649,18 +3641,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O if cfg!(debug_assertions) { // Enforce that scope locations are strictly increasing to help catch bugs - // where the pushed scopes are mismatched between the first and second passes. - // Skip this check after *this file* has logged an error: error recovery can - // legitimately cause `next()` to return a token at a position that was - // already consumed (e.g. a zero-length token produced after `TSyntaxError`), - // which makes the parser push multiple scopes at the same source offset. - // Release builds just report the error and recover; the debug assertion - // should not be stricter than release. Compare against `orig_error_count` - // so an unrelated error from an earlier file in the same session doesn't - // silently disable this invariant for a file that parses cleanly — mirrors - // the visit-pass bailout at `parse_entry.rs:876`. - let file_has_errors = self.log().errors > self.orig_error_count; - if !file_has_errors && !self.scopes_in_order.is_empty() { + // where the pushed scopes are mismatched between the first and second passes + if !self.scopes_in_order.is_empty() { let mut last_i = self.scopes_in_order.len() - 1; while self.scopes_in_order[last_i].is_none() && last_i > 0 { last_i -= 1; @@ -9208,11 +9190,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O current_scope: scope, module_scope: scope, scopes_in_order: scope_order, - // SAFETY: `log` is a `NonNull` whose pointee outlives `'a` - // (enforced by `Parser::init`); reading the `errors` scalar here - // mirrors `parse_entry.rs:762` which reads it through the lexer - // alias at the same point in the pipeline. - orig_error_count: unsafe { log.as_ref() }.errors, needs_jsx_import: false, // Zig: `if (scan_only) false else void` — NeedsJSXType collapsed to `bool` lexer, diff --git a/test/regression/issue/jsx-template-string-crash.test.ts b/test/regression/issue/jsx-template-string-crash.test.ts index 926459dbdc2..e5a22b15c43 100644 --- a/test/regression/issue/jsx-template-string-crash.test.ts +++ b/test/regression/issue/jsx-template-string-crash.test.ts @@ -22,9 +22,9 @@ test("JSX lexer should not crash with slice bounds issues", async () => { at /[eval]:1:34 1 | export function x(){return
} - ^ - error: Unexpected > - at /[eval]:1:37" + ^ + error: Unterminated string literal + at /[eval]:1:35" `); expect(normalizeBunSnapshot(stdout.toString())).toMatchInlineSnapshot(`""`); }); From 72597fcb95033d9668f2c0678f96e60ca2b4ef76 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 18 May 2026 04:59:54 +0000 Subject: [PATCH 5/8] test: update #30959 comment to match the lexer-layer fix --- .../issue/jsx-template-string-crash.test.ts | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/test/regression/issue/jsx-template-string-crash.test.ts b/test/regression/issue/jsx-template-string-crash.test.ts index e5a22b15c43..5f719e0824e 100644 --- a/test/regression/issue/jsx-template-string-crash.test.ts +++ b/test/regression/issue/jsx-template-string-crash.test.ts @@ -32,16 +32,21 @@ test("JSX lexer should not crash with slice bounds issues", async () => { test.concurrent("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds", async () => { // Previously, parsing ` Date: Mon, 18 May 2026 05:12:34 +0000 Subject: [PATCH 6/8] test: drop redundant stderr.toString() in #30959 regression test stderr is already a string from proc.stderr.text(). Per coderabbit. --- test/regression/issue/jsx-template-string-crash.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/regression/issue/jsx-template-string-crash.test.ts b/test/regression/issue/jsx-template-string-crash.test.ts index 5f719e0824e..a04b036aef1 100644 --- a/test/regression/issue/jsx-template-string-crash.test.ts +++ b/test/regression/issue/jsx-template-string-crash.test.ts @@ -56,7 +56,7 @@ test.concurrent("#30959 JSX attribute with invalid '(' value parses cleanly in d const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(normalizeBunSnapshot(stderr.toString().replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(` + expect(normalizeBunSnapshot(stderr.replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(` "1 | export function x(){return Date: Mon, 18 May 2026 05:38:29 +0000 Subject: [PATCH 7/8] ci: retrigger From e49af9ce3f0d07044d959d929fec5ba80fd67fee Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 18 May 2026 06:46:01 +0000 Subject: [PATCH 8/8] lexer: step past unknown byte on TSyntaxError in main next() too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors d1a40fcbe8 at the parallel site in the main `next()` fallthrough (lexer.rs:2234-2235). Per Jarred on PR #30969 — the same pattern lived at both sites and the Rust port diverged from the Zig sibling at one of them, leaving the main lexer with `current == end` after the error instead of the `current > end` invariant every other branch maintains. The concrete duplicate-scope panic the JSX site triggered isn't currently reachable from the main lexer (a byte that falls through to this arm is invalid in main-lexer context too, so recovery stays in `TSyntaxError` rather than manufacturing a real token), but keeping the invariant consistent across both dispatch tables means future recovery paths don't have to reason about one arm that leaves the lexer half-advanced. --- src/js_parser/lexer.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/js_parser/lexer.rs b/src/js_parser/lexer.rs index c6e7518e5bc..47439b8a528 100644 --- a/src/js_parser/lexer.rs +++ b/src/js_parser/lexer.rs @@ -2233,6 +2233,20 @@ lexer_impl_header! { self.end = self.current; self.token = T::TSyntaxError; + // Mirror the `next_inside_jsx_element` fix (#30959): advance + // `code_point`/`current` past the bad byte so a subsequent + // recovery `next()` dispatches on the *following* byte rather + // than re-dispatching on the still-in-`code_point` bad byte. + // In the main lexer the byte that falls through to this arm + // is invalid in main-lexer context too, so re-dispatch + // currently stays in `TSyntaxError` and the duplicate-scope + // panic isn't reachable — but keeping the `current > end` + // invariant consistent across both dispatch tables means + // future recovery code doesn't have to reason about one arm + // that leaves the lexer with `current == end`. `end` was + // already advanced above, so the error range `[start, end)` + // is unchanged. + self.step_with(contents); } }