lexer: step past unknown byte on TSyntaxError in next_inside_jsx_element #30969
Claude / Claude Code Review
completed
May 18, 2026 in 11m 26s
Code review found 3 potential issues
Found 4 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/js_parser/p.rs:3650 |
Assertion guard uses absolute error count instead of per-parse delta |
| 🟡 Nit | test/regression/issue/jsx-template-string-crash.test.ts:32 |
New process-spawning test should use test.concurrent per test/CLAUDE.md |
Annotations
Check warning on line 3650 in src/js_parser/p.rs
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
Check warning on line 32 in test/regression/issue/jsx-template-string-crash.test.ts
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.
Loading