Skip to content

feat(parser): report spec parse errors via recoverable_errors#94

Merged
leaysgur merged 2 commits into
mainfrom
report-spec-parse-errors
Jul 8, 2026
Merged

feat(parser): report spec parse errors via recoverable_errors#94
leaysgur merged 2 commits into
mainfrom
report-spec-parse-errors

Conversation

@Boshen

@Boshen Boshen commented Jul 4, 2026

Copy link
Copy Markdown
Member

Closes #86.

Problem

Since the css-syntax conformance work (#61), the parser recovers from EOF/newline-unclosed constructs to a valid AST but records nothing in recoverable_errors(). The recovery is spec-conformant, yet the spec labels each of these "this is a parse error", so downstream consumers (oxc_formatter_css) cannot tell recovered-from-broken input apart from valid CSS — and end up formatting it into corrupted, non-idempotent output.

Change

Record these spec parse errors. Purely additive — the AST and recovery behavior are unchanged, so css-syntax conformance is unaffected (same mechanism as the existing TopLevelDeclaration recoverable error).

New ErrorKinds and where they're recorded (two sites cover every case):

Kind Case Site
EofInBlock {}-block closed at EOF (style-rule and at-rule blocks) SimpleBlock::parse
UnterminatedString (reused) string closed by EOF — spec's canonically-closable <string-token> parse_declaration_value_tokens
BadString string closed by a newline — <bad-string-token> parse_declaration_value_tokens
UnclosedParen (/[ group closed at EOF in a raw value parse_declaration_value_tokens

Every other unclosed-construct context (@import "…, a[href="…, @media (…) already hard-errors, so these are the only silent-recovery paths.

BadUrl from the issue's "e.g." list is not applicable: this parser models url(…) as a flexible function rather than emitting spec bad-url-tokens, so there was never a silent bad-url recovery to convert.

Verification

  • All parser tests pass; cargo fmt + clippy clean.
  • wpt css-syntax conformance unchanged (5 clean / 2 expected) — spec conformance genuinely unaffected.
  • Conformance flips 10 plain-.css fixtures in the dialect suites (sass-spec, less.js) from cleanrecovered; each is a genuine malformation (e.g. a sass output.css with a stray output: ";, and a truncated less.js actual.css ending mid-base64). Snapshots regenerated.
  • Moved 3 now-legitimately-recovering fixtures out of tests/ast/ (which asserts no recoverable errors) into tests/recoverable/eof/, and added fixtures/snapshots covering every kind.

🤖 Generated with Claude Code

CSS Syntax recovers from EOF/newline-unclosed constructs to a valid AST,
but the spec labels each "this is a parse error". These recoveries were
silent (no `recoverable_errors()` entry), so downstream consumers could
not tell recovered-from-broken input apart from valid CSS.

Record them, purely additively (AST and recovery unchanged):

- `EofInBlock` — a `{}`-block closed at EOF (covers style-rule and
  at-rule blocks), recorded in `SimpleBlock::parse`.
- `UnterminatedString` (EOF) / `BadString` (newline) — an unterminated
  string kept as a preserved `<bad-string-token>`, recorded in
  `parse_declaration_value_tokens` and split by where the string stopped.
- `UnclosedParen` — a `(`/`[` group closed at EOF in a raw value.

wpt css-syntax conformance is unaffected; only genuinely-broken `.css`
fixtures in the dialect suites flip clean -> recovered.
@codspeed-hq

codspeed-hq Bot commented Jul 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing report-spec-parse-errors (ef585f0) with main (e4c405e)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@Boshen Boshen marked this pull request as ready for review July 4, 2026 10:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7f560b80b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/oxc_css_parser/src/parser/stmt.rs Outdated
@leaysgur leaysgur merged commit 5f48953 into main Jul 8, 2026
7 checks passed
@leaysgur leaysgur deleted the report-spec-parse-errors branch July 8, 2026 00:42
@oxc-guard oxc-guard Bot mentioned this pull request Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Report spec parse errors (unterminated string / unclosed paren / bad-string) via recoverable_errors

2 participants