Skip to content
27 changes: 25 additions & 2 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -3641,8 +3649,18 @@ 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
if !self.scopes_in_order.is_empty() {
// 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() {
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;
Expand Down Expand Up @@ -9190,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<Log>` 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,

Expand Down
37 changes: 37 additions & 0 deletions test/regression/issue/jsx-template-string-crash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,40 @@ test("JSX lexer should not crash with slice bounds issues", async () => {
`);
expect(normalizeBunSnapshot(stdout.toString())).toMatchInlineSnapshot(`""`);
});

test.concurrent("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds", async () => {
// Previously, parsing `<r L=((` in a debug build panicked with
// `Scope location must be greater than previous 6 must be greater than 6`
// in push_scope_for_parse_pass. The recovery path after
// `expect(TOpenBrace)` against a TSyntaxError lexer token caused two
// FunctionArgs scopes to be pushed at the same source offset.
//
// Release builds already reported the two parse errors and recovered;
// the debug assertion is now relaxed once an error has been logged so
// debug matches release. We assert the expected release-mode error
Comment thread
robobun marked this conversation as resolved.
Outdated
// messages instead of the absence of a panic, so the test fails cleanly
// both when the panic returns (exit code / hang) and when the error
// output regresses.
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", "export function x(){return<r L=((}"],
env: bunEnv,
stderr: "pipe",
stdout: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(normalizeBunSnapshot(stderr.toString().replace(/(Bun v.*)$/gm, ""))).toMatchInlineSnapshot(`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
"1 | export function x(){return<r L=((}
^
error: Expected "{" but found "("
at <cwd>/[eval]:1:32

1 | export function x(){return<r L=((}
^
error: Unexpected }
at <cwd>/[eval]:1:34"
`);
expect(stdout).toBe("");
expect(exitCode).toBe(1);
});
Loading