Skip to content
14 changes: 14 additions & 0 deletions src/js_parser/lexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Comment thread
robobun marked this conversation as resolved.
}
}

Expand Down
43 changes: 40 additions & 3 deletions test/regression/issue/jsx-template-string-crash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,46 @@
at <cwd>/[eval]:1:34

1 | export function x(){return<div a=\`\`/>}
^
error: Unexpected >
at <cwd>/[eval]:1:37"
^
error: Unterminated string literal
at <cwd>/[eval]:1:35"
`);
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

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

View check run for this annotation

Claude / Claude Code Review

Stale test comment describes reverted fix approach

This comment (and the PR title/description) describes the reverted `orig_error_count` approach from 2b1dc3b9/bc00e5d1 — "the debug assertion is now relaxed once an error has been logged" is no longer true. d1a40fcb reverted the p.rs guard; the shipped fix is `self.step()` in `next_inside_jsx_element` (lexer.rs:3064), which prevents the duplicate scope offset from arising at all. The assertion in `push_scope_for_parse_pass` is *not* relaxed and would still fire on a duplicate offset. Worth reword
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