parser: apply ASI when TS contextual keywords precede a newline - #34258
Merged
Claude / Claude Code Review
completed
Jul 15, 2026 in 27m 38s
Code review found 2 potential issues
Found 4 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/js_parser/parse/parse_stmt.rs:1807-1812 |
534bc36's is_typescript_declare guard is over-broad: rejects export declare\n and keyword\n inside ambient bodies |
| 🟡 Nit | src/js_parser/parse/parse_stmt.rs:1794-1798 |
534bc36's per-arm fix misses sibling arms TsStmtType/TsStmtNamespace/TsStmtModule |
Annotations
Check warning on line 1812 in src/js_parser/parse/parse_stmt.rs
claude / Claude Code Review
534bc365's is_typescript_declare guard is over-broad: rejects export declare\n and keyword\n inside ambient bodies
534bc365's new `opts.is_typescript_declare` guard here (and in `TsStmtDeclare` at line 1828) is over-broad: that flag is also set by `t_export`'s `SDeclare` arm before it re-parses the same `declare` token, and by `parse_type_script_namespace_stmt` / the `declare global {}` handler for every body statement. So `export declare\nclass Foo {}` and `declare namespace N { abstract\nclass Foo {} }` / `declare global { abstract\nclass Foo {} }` now error, while esbuild (and this PR's own tested `export
Check warning on line 1798 in src/js_parser/parse/parse_stmt.rs
claude / Claude Code Review
534bc365's per-arm fix misses sibling arms TsStmtType/TsStmtNamespace/TsStmtModule
🟡 nit: 534bc365 added the `is_typescript_declare` error fallthrough here (and to `TsStmtAbstract`/`TsStmtDeclare`) so `declare interface\n…` / `declare abstract\n…` reject instead of leaking live code, but the sibling `TsStmtType` (~1752) and `TsStmtNamespace`/`TsStmtModule` (~1764) arms — which already had `!has_newline_before` gates — were not given the same check. `declare type\nFoo = number` still emits a live `Foo = number;` assignment and `declare namespace\nFoo\n{ sideEffect() }` emits `F
Loading