Skip to content
9 changes: 7 additions & 2 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3641,8 +3641,13 @@

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() {

Check warning on line 3650 in src/js_parser/p.rs

View check run for this annotation

Claude / Claude Code Review

Assertion guard uses absolute error count instead of per-parse delta

nit: `!self.log().has_errors()` checks the absolute error count (`log.errors > 0`), but the parser is designed to be invoked with a `Log` that already carries errors from prior parses — that's why `_parse` captures `orig_error_count` (parse_entry.rs:762) and gates the visit-pass bailout on `errors > orig_error_count` (parse_entry.rs:876). With a pre-existing error from an earlier file, this guard disables the assertion for the entire current parse even though it's clean and *does* proceed to the
Comment thread
robobun marked this conversation as resolved.
Outdated
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
41 changes: 41 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,44 @@
`);
expect(normalizeBunSnapshot(stdout.toString())).toMatchInlineSnapshot(`""`);
});

test("#30959 JSX attribute with invalid '(' value parses cleanly in debug builds", async () => {

Check warning on line 32 in test/regression/issue/jsx-template-string-crash.test.ts

View check run for this annotation

Claude / Claude Code Review

New process-spawning test should use test.concurrent per test/CLAUDE.md

nit: per `test/CLAUDE.md` ("When multiple tests in the same file spawn processes or write files, make them concurrent with `test.concurrent` or `describe.concurrent`"), this new test could be `test.concurrent` — it spawns an independent subprocess and shares no state with the other test. Full benefit also requires converting the existing `spawnSync` test above, but the new one should at least follow the convention.
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
// 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