Skip to content

feat!: support spec v4.4 - #88

Merged
shreyasbhat0 merged 25 commits into
mainfrom
feat/spec-4.2
Oct 12, 2026
Merged

shreyasbhat0 merged 25 commits into
mainfrom
feat/spec-4.2

Conversation

@johannschopplich

@johannschopplich johannschopplich commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Brings the crate to spec v4.4 – the submodule moves to the v4.4.0 tag and every fixture passes. The branch first went through v4.2.1 and v4.3.0, and each behavior change is its own commit, so the history reads as a changelog.

The biggest change is non-strict decoding. with_strict(false) now recovers from exactly the five conditions of §14.4 – a declared [N] is advisory, duplicate keys resolve last-write-wins, indentation may use tabs or uneven spaces, blank lines inside a header span are skipped, and a block whose first line starts too deep takes that depth. Everything else errors in both modes, including malformed headers, over-indented lines and row-width mismatches. Code that relied on non-strict mode to read sloppy input may now get an error. Documents a conforming encoder writes decode as before.

Other behavior changes users may notice:

  • Lines and headers: a line whose first unquoted [ comes before its first colon must be a valid header (strict mode no longer reads a[2:]{x} as a key), a hyphen marks a list item only at item depth and may be followed by several spaces, only SP and HTAB count as whitespace, a " opens a quoted span anywhere in a line, spaces between a quoted key and its colon are accepted, whitespace before a nested field group is an error, and a header whose length exceeds usize still parses.
  • JSON input is read with serde_json's float_roundtrip, so floats keep their exact value on the way to TOON.

The "Documented implementation-defined behavior" list in lib.rs no longer mentions tab indentation – §14.4 now defines it.

@johannschopplich
johannschopplich requested a review from a team as a code owner October 6, 2026 08:38
@shreyasbhat0

shreyasbhat0 commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

@johannschopplich

Two things I checked that are worth recording, since neither is obvious from the diff.

The encoder needed no change, and that's verifiable rather than assumed. v4.2 moved four encoder rules from SHOULD/MAY to MUST (§7.1 literal UTF-8 and lowercase hex, §7.2 must-not-over-quote, §7.3 keys must be unquoted when they match, §11.1 delimiter quoting widened to primitive list items and root primitives). Each one is pinned by an existing encode fixture and each passes, so the crate already conformed on all four.

float_roundtrip is load-bearing. I reverted just that feature flag and re-ran: exactly one case fails, encodes a large integer-valued number with its shortest round-trip digits — 123456789012345670000 instead of ...680000. Good to know it isn't a speculative flag.

I'd also suggest surfacing c3829ca more prominently, maybe in the PR body and the changelog entry. It reads as an indentation nicety but it's a silent data-loss fix: in non-strict mode an over-indented line used to terminate the enclosing scope, so

items[2]:
  - a
    b: 1
  - c

decoded as {"items":["a"]} on v4.1 and {"items":["a","c"]} here. Dropping c on input lenient mode was asked to accept is the kind of thing people want to see called out by name.

Two requests before merge, both tests rather than code:

  1. A local regression test for the Rust-specific decisions — the usize::MAX header, float_roundtrip, and the c3829ca truncation case. Right now all twelve commits are covered only by fixtures in the submodule, and the v4.2 changelog warns that redundant cases get removed and indices shift (533 now, down from 538). A spec bump could quietly delete the only coverage any of these has.
  2. One question on an unpinned behaviour change: an indented [] as the only line now errors in both modes, where v4.1 returned []. I think that's right — §5's new "first non-blank depth-0 line" qualifier stops that branch applying, and §6's "a line without any unquoted colon is neither a header nor a key context" makes [] a scalar line, which §5.2 errors on in any mode. But no fixture covers it and no commit message mentions it, so I'd like to know it was deliberate.

Two small things, non-blocking: skip_over_indented_line reports "over-indented" for a line that's actually under-indented relative to an adopted depth, and decode_object_fields adopts its depth through computed_depth.get_or_insert without the local assert_no_depth_jump that scope_content_depth has. Strict mode does reject the jump today, so this is about keeping it that way rather than a bug.

Also worth a line in the release notes: non-strict decoding of a wholly-indented document now yields {} rather than its contents.

Happy to approve once the tests land.

@johannschopplich johannschopplich changed the title feat: support spec v4.2.1 feat!: support spec v4.4 Oct 8, 2026
@johannschopplich

Copy link
Copy Markdown
Member Author

Thanks for the thorough pass, and sorry for the silence. The branch has since gone on to spec v4.4, which settles most of it:

  • The usize header and the float_roundtrip case are now pinned by tagged fixtures – keeps a header whose length exceeds integer range in non-strict mode and encodes a large integer-valued number with its shortest round-trip digits, both minSpecVersion 4.2 – so a fixture cleanup won't drop them, and local copies would only duplicate them.
  • The c3829ca truncation is gone for another reason: 4.4 makes an over-indented line an error in both modes, so skip_over_indented_line no longer exists and your example now errors instead of losing c.
  • The indented [] erroring is deliberate – §5 takes the root form from the first depth-0 line, and throws on an indented root array header pins the header case.
  • A wholly indented document now errors in non-strict mode too (throws on a whole document indented by one level in non-strict mode), so that release-notes line reads differently now.

The computed_depth.get_or_insert in decode_object_fields stays: adopting a jumped first line's depth is one of the five non-strict recoveries in 4.4, and strict mode still rejects the jump. The 4.3 and 4.4 commits sit on top of what you approved.

@johannschopplich

Copy link
Copy Markdown
Member Author

One more commit on top: test: drop tests that assert nothing removes the nine tests in tests/errors.rs that only println! their result, plus two print-only cases in test_invalid_array_header_syntax. They pass whatever the decoder returns – test_length_mismatch_non_strict_mode still lists expectations 4.4 no longer holds – so they cover nothing. The tests in that file that do assert stay.

`deny_unknown_fields` guarded `TestOptions` but not `TestCase`, so every
case-level field was dropped: `minSpecVersion` on 103 of 564 cases, `note`
on 114, and `specSection` on all of them. A fixture tagged for a spec this
crate has not implemented ran anyway, which is the situation the tag exists
to report.

Deny unknown fields on the case too, and fail with the version it needs when
a case postdates the targeted spec. `specSection` and `note` now appear in
the failure label, so a red case names the rule it broke.
@shreyasbhat0

shreyasbhat0 commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Re-ran this at d083f4e with the submodule at v4.4.0: 564/564 fixture cases, 0 skipped, clippy clean, tests green across no-default, layout, json_stream, layout,json_stream, cli and all-features. I also walked the five §14.4 recoveries — counts, duplicate keys, tab indentation and depth jumps all still recover, while brace fall-through, row-width mismatches, trailing content and over-indentation now error in both modes. Matches 4.4.

All three of your points hold up:

  • test: drop tests that assert nothing is exactly that. The 104 removed lines contain no assert, unwrap, expect or ? — test_length_mismatch_non_strict_mode even binds _expected and throws it away, so the expectations it listed were never checked. Removing them makes the suite more honest, not less covered.
  • The c3829ca example errors now instead of dropping c. Better outcome than the one I was asking for.
  • Dropping the tab-indentation note from lib.rs is right — §14.4 recovery Invitation: Official Rust implementation of TOON #3 makes it a MUST, so it isn't implementation-defined any more. I went looking for a regression there and found a correct cleanup.

On the fixture tags: I took you at your word that minSpecVersion pins those cases, then went to check and found the runner never reads it. deny_unknown_fields sits on TestOptions but not on TestCase, so every case-level field was being dropped — minSpecVersion on 103 of 564 cases, note on 114, specSection on all of them. grep minSpecVersion tests/spec_fixtures.rs returned nothing.

Rather than hand it back to you I pushed dea7b77 onto this branch — sorry for touching your PR without asking first. It's 65 lines in tests/spec_fixtures.rs and nothing else:

  • deny_unknown_fields on TestCase too, so the next case-level field fails loudly instead of vanishing. That was already the stated intent in the comment above TestOptions; it just stopped at options.
  • minSpecVersion is now checked against a TARGET_SPEC_VERSION const that mirrors the toon-spec line in lib.rs. A case needing a newer spec fails with requires spec v4.5, but this crate targets v4.4: the spec submodule has moved ahead of the implementation.
  • specSection and note are folded into the failure label, so a red case reads [decode] root-form (§8): throws on a whole document indented by one level. That also keeps them live rather than parked behind #[allow(dead_code)].

One judgement call worth your opinion: I made a too-new fixture fail rather than skip. For a downstream runner skipping is the point of the tag, but for the reference implementation I'd rather a submodule bump tell us the spec moved than quietly run fewer cases — especially with #75 adding a gitsubmodule Dependabot entry. Easy to flip to a skip if you disagree.

I tested both guards by mutation and reverted: injecting a futureField into a fixture gives unknown field 'futureField', expected one of 'name', 'input', 'expected', 'options', 'shouldError', 'minSpecVersion', 'specSection', 'note', and setting the target to 4.3 makes the 39 cases tagged 4.4 fail with the message above. 564/564 still pass unchanged, fmt and clippy clean, CI green on dea7b77.

Squashing and merging this now rather than holding it any longer. If you'd rather dea7b77 looked different — the fail-vs-skip call especially — say so and I'll send a follow-up PR.

One for the release notes: non-strict decoding is meaningfully stricter than 0.6.1, rejecting malformed headers, row-width mismatches, trailing content and over-indented lines it previously recovered from. Worth saying plainly, since the commit subjects won't carry it.

@shreyasbhat0
shreyasbhat0 merged commit 896540e into main Oct 12, 2026
3 checks passed
@shreyasbhat0
shreyasbhat0 deleted the feat/spec-4.2 branch October 12, 2026 07:09
@github-actions github-actions Bot mentioned this pull request Oct 12, 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.

2 participants