fix(es/parser): Keep recoverable errors from speculative TypeScript parses

[David Sherret (dsherret)]

Description:

Opening this as a draft because there are two design decisions in here I am not happy with and would like your call on before it goes any further. Those are up front, below the root cause.

Root cause

try_parse_ts sets Context::IgnoreError for the duration of a speculative parse and only restores it after the closure returns. Suppressing errors is correct on the failure path — the parse is thrown away, so its errors should go too. It is wrong on the success path: the parse is kept, but every recoverable error emitted while producing it has already been dropped on the floor.

The concrete symptom:

export namespace Foo {

yields zero diagnostics, while the non-export form

namespace Foo {

correctly yields Expected '}', got '<eof>'. The export form routes through try_parse_ts; the plain form does not. export module Foo { is affected the same way.

This fix makes errors roll back with the checkpoint rather than being suppressed:

• lexer/state.rs — LexerCheckpoint gains errors_len / module_errors_len, and checkpoint_load truncates both. This mirrors the existing CapturingCheckpoint pattern in lexer/capturing.rs, which already truncates its captured-token buffer on rollback. Both error buffers are append-only until take_errors, so truncating to a saved length is a sound rollback.
• parser/typescript.rs — try_parse_ts and try_parse_ts_bool no longer set Context::IgnoreError. ts_look_ahead keeps the suppression, since it always reloads the checkpoint and the errors would be discarded regardless; suppressing there just avoids the wasted work.

Motivating downstream bug

dprint-plugin-typescript panics in gen_membered_body on this input, because its syntax-error gate has nothing to filter — swc reports no error at all, so the formatter proceeds on a truncated AST: dprint/dprint-plugin-typescript#802

---

Open question 1: the Flow carve-out (this is a smell)

should_ignore_error_while_backtracking() currently returns self.syntax().flow(). TypeScript gets the corrected behavior on every speculative path; Flow keeps the old blanket suppression. A syntax-conditional error policy is not something I would normally want to add.

It is there because un-suppressing for Flow breaks exactly 7 hermes parity fixtures:

JSX/jsx_type_args_implicit.js
comment_interning/call.js
types/annotations/underscore_is_allowed_trailing_commas.js
types/annotations/underscore_is_implicit_anywhere_in_list.js
types/annotations/underscore_is_implicit_in_calls.js
types/annotations/underscore_is_implicit_in_constructor_calls.js
types/annotations/underscore_is_implicit_in_methods.js

All 7 are the same issue: _ is in is_flow_reserved_type_name, and parse_ts_type_ref reports TS1003 for it in every type-argument list. In Flow, _ is legal as an implicit type argument in call and new positions (test<_>()), but illegal in an annotation — and the corpus fixture types/annotations/underscore_is_reserved_elsewhere.js (var x: Generic<_> = 3;) explicitly expects the error. So it cannot simply be allowed everywhere; the two positions genuinely need to be distinguished.

Distinguishing them wants a new context flag, and Context is a bitflags u32 with all 32 bits already allocated (bit 31 is DisallowFlowAnonFnType). Widening to u64 in a hot, frequently-copied parser struct felt like a decision for you rather than for me, so I left the carve-out in and am raising it here instead of hiding it.

Happy to split this into two PRs if you prefer, and I think that may well be the better shape:

• (a) the checkpoint rollback mechanism alone — errors_len/module_errors_len on LexerCheckpoint. With IgnoreError still set by try_parse_ts, this is a pure mechanism change with no observable behavior change at all.
• (b) the un-suppression as a follow-up, once Flow's _ handling can distinguish call position from annotation position.

That would let (a) land uncontroversially and keep the Flow question isolated in (b). Say the word and I will restructure.

Open question 2: the Context::InType gate in parser/ident.rs

parse_ident no longer emits InvalidIdentInStrict when Context::InType is set. On its face this is an unrelated behavior change bundled into an error-plumbing fix, and I want to flag it rather than let it slide through.

In fairness to it, two things are true:

1. It is a real bug fix on its own. A parameter name in a function type is not a binding, so the strict-mode reserved-word restriction does not apply. On main today, type A = (package: string) => void; wrongly reports InvalidIdentInStrict, and so does interface I { m(package: string): void } — while the exported form export type A = (package: string) => void; reports nothing, because the speculative path swallows it. So main is currently inconsistent with itself here, depending on whether export is present.

2. It is not actually separable from this change. Un-suppressing errors without it regresses Parser error for callback signature with "package" as parameter name #7186: with the ident.rs hunk reverted and the rest of this PR in place, spec_tests__typescript__issue_7186__1__input_ts and spec_tests__typescript__issue_7186__2__input_tsx both fail. Those are the fixtures added by fix(es/parser): Allow using package as a parameter name in TS interface #7438, and they only pass on main today because export interface routes through try_parse_ts and the error is suppressed. Removing the suppression exposes it.

So it is load-bearing rather than opportunistic — but the gate is written broadly (all of InType), and if you would rather see it as its own PR with its own fixtures, landing first, I am glad to split it out; this PR would then rebase on top of it.

Verified that it does not over-suppress real bindings — these all still report InvalidIdentInStrict after the change: function f(package) {}, function f(package: string) {}, var package = 1;, class C { constructor(private package: string) {} }.

---

Test results (as actually measured, on Windows):

cargo test -p swc_ecma_parser --no-fail-fast (default features):

	total passed	total failed	tests/typescript.rs target
main (2f3e88d)	5488	2	4957 passed / 1 failed
this branch	5489	2	4958 passed / 1 failed

cargo test -p swc_ecma_parser --features verify,flow --no-fail-fast:

	total passed	total failed	tests/typescript.rs target
main (2f3e88d)	5621	2	4958 passed / 0 failed
this branch	5622	2	4959 passed / 0 failed

The +1 in each case is the new regression fixture. No test fails on this branch that does not also fail on main at the same commit — I ran the baseline explicitly rather than assuming it.

The failures, all pre-existing and identical on main:

• test262::identity (both configs) and test262::error (verify,flow) — I do not have the test262-parser submodule checked out locally, so the test262 suite could not run at all; it fails on a missing directory. Unverified by me either way.
• errors_tests__typescript_errors__type_only_import_specifier__invalid_type_only__input_ts (default features only) — import { type import } from 'mod' should fail but parses. Present identically on main; behavior is unchanged by this PR.

New regression fixture tests/typescript-errors/export-namespace-unclosed/: confirmed it fails on main ("should fail, but parsed as ...") and passes with this change.

No existing expected-output files were modified — git diff upstream/main --stat is 6 files, and both fixture files are additions.

Not measured: I scoped builds to -p swc_ecma_parser. Downstream crates (swc, swc_ecma_transforms, …) were not built or tested, so any fixture churn there is unknown. Since this PR makes the parser surface more diagnostics on TS speculative paths, downstream suites that assert on error output are the most plausible place for fallout, and CI will be a better judge than my local run.

AI assistance disclosure: this change was developed with AI assistance. I have reviewed the diff and the test results, and independently re-ran both the baseline and the branch rather than taking the reported numbers on trust.

BREAKING CHANGE:

None in API terms. It is a behavior change: TypeScript sources that route through try_parse_ts and contain recoverable errors will now surface diagnostics that were previously silently dropped. That is the point of the fix, but consumers gating on "did the parser report anything" may see newly-reported errors on input they previously got silence for.

Related issue (if exists):

• Downstream report: Panic "Expected to find a close brace token" formatting an unclosed exported namespace/module dprint/dprint-plugin-typescript#802
• Touches the area of Parser error for callback signature with "package" as parameter name #7186 / fix(es/parser): Allow using package as a parameter name in TS interface #7438 (see open question 2)

[changeset-bot]

🦋 Changeset detected

Latest commit: 9eba6f1

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[David Sherret (dsherret)]

Disclosure: All of this PR is AI generated and the above. I opened as a draft PR because I have yet to review, but feel free to use it as a starting point and take it over.

[codspeed-hq]

Merging this PR will improve performance by 2.41%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 199 untouched benchmarks
⏩ 61 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Simulation	es/lints/libs/three	50.3 ms	49.1 ms	+2.41%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing dsherret:fix/try-parse-ts-error-rollback (9eba6f1) with main (3802924)}

Footnotes

1. 61 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[Donny/강동윤 (kdy1)]

The overall direction makes sense to me: diagnostics from a successful speculative parse should be preserved, while diagnostics from a discarded parse should be rolled back.

However, making every LexerCheckpoint restore truncate the shared error buffers may be too broad in the presence of nested speculative parses. The current swc fixture failure for generic arrow functions seems to demonstrate this: existing TS2369 diagnostics are lost for cases such as <T>(public x: T, y: T) => {}.

Could we scope diagnostic rollback to the speculative operation that owns those diagnostics, or otherwise make the commit/rollback semantics explicit for nested checkpoints?