Skip to content

render: emit lossless forms or error — never a document that re-parses differently - #4

Open
chappihappymeal wants to merge 1 commit into
ktav-lang:mainfrom
chappihappymeal:render-lossless
Open

render: emit lossless forms or error — never a document that re-parses differently#4
chappihappymeal wants to merge 1 commit into
ktav-lang:mainfrom
chappihappymeal:render-lossless

Conversation

@chappihappymeal

Copy link
Copy Markdown
Contributor

Closes #3.

Every case from the issue now either round-trips byte-for-byte or returns an error, on all three writer surfaces (emit_canonical, serde to_string, render / to_string_force_strings). The parser is untouched.

What changed

  • Edge-whitespace / control-byte strings → verbatim block (§ 5.9.5(b), § 5.9.7). string_needs_multiline in render/helpers.rs routes them to the multi-line form on all surfaces. The check is Unicode-aware (char::is_whitespace), matching the parser's str::trim() — NBSP/NEL/U+3000 edges were the sneakiest variant of the bug.
  • (-prefixed bodies → raw marker (§ 5.9.5(d)). The canonical writer had its own needs_raw_marker copy that missed the ( prefix and emitted unparseable documents; it now delegates to the shared helper.
  • Marker-shaped bare items → raw marker. New item_needs_raw_marker covers the collisions a pair body doesn't have: leading ## (comment line), leading :: (the raw marker itself), sole ] / } (closers).
  • Unrepresentable keys → error (CR precedent, § 5.9.7). push_escaped_key_segment now validates before writing: empty keys, edge whitespace (Unicode-aware, § 4), embedded line breaks, the bytes ( ) { } [ ] , (§ 3.7 defines no key escape for them on a pair line), and a leading ## produce a descriptive error naming the reason and the spec section. The serde path previously wrote map keys raw ({"a.b": 1} re-parsed as nesting); all its key sites now share this helper.
  • One shared multi-line form chooser (§ 5.6.1). The stripped/verbatim selection had drifted across five copies; choose_multiline_form replaces them. Pretty renderers still prefer the indented stripped form when it is unconditionally safe and verbatim otherwise; the canonical writer still prefers verbatim. New: a lossless stripped fallback (an unindented line pins the parser's common-indent dedent to zero) rescues bodies with a )) line, and bodies no form can hold are an error everywhere — the canonical writer used to emit them broken, and its stripped emitter no longer trim_starts content lines.

Deliberate behaviour change

values_with_leading_trailing_whitespace_are_trimmed (from the 0.1.0 initial commit) asserted that serializing " padded " comes back as "padded" — "documented" trim behaviour. Spec 0.6 § 5.9.5(b) forbids the plain form for such bodies and § 5.9.7 mandates verbatim, so the test now asserts the round-trip. This is the only previously-passing expectation the PR flips.

Two smaller form changes for previously round-tripping values, both spec-mandated: strings with interior control bytes now emit as verbatim blocks (§ 5.9.7), and multi-line bodies containing both a sole-) and a sole-)) line now error on every surface instead of canonical emitting a broken document.

Verification

  • tests/render_lossless.rs: 41 new tests, written red-first, covering every class on all three surfaces (round-trip asserts for representable values, error asserts for unrepresentable ones, guard tests for tricky-but-working keys and bodies).
  • Full suite green (including the spec conformance fixtures and the existing exact-output ser/render tests); cargo clippy --all-targets -- -D warnings and cargo fmt --check clean.
  • Differential check against v0.6.2: a corpus exercising scalars, nested compounds, dotted/escaped keys, both multi-line forms, :: literals and empty values renders byte-identically on old and new across all three surfaces — no output change for anything that already round-tripped.

…s differently

All three writer surfaces (emit_canonical, the serde text serializer,
and render / to_string_force_strings) could silently emit documents
that parse back to a different Value — or fail to parse at all:

- One-line strings with leading/trailing whitespace were emitted
  inline, so the parser's body trim ate the padding ("hunter2 " came
  back as "hunter2"; "   " came back empty). § 5.9.5(b) forbids the
  plain form for such bodies and § 5.9.7 mandates the verbatim block,
  which holds them byte-for-byte. The parser trims with str::trim(),
  so the new check covers the full Unicode White_Space set (NBSP,
  NEL, U+3000, …), not just ASCII space/tab. Strings with interior
  control bytes now take the verbatim form § 5.9.7 mandates as well.

- Strings starting with `(` (e.g. "(disabled)") made the canonical
  writer emit documents the parser rejects; they now take the raw
  marker, per § 5.9.5(d).

- Bare array items shaped like markers — a leading `##` (comment
  line), a leading `::` (the raw marker itself), a sole `]` / `}`
  (closer) — were silently dropped, clipped, or made the document
  unparseable. The item context now has its own raw-marker check.

- Keys with no written representation — empty, edge whitespace
  (Unicode-aware, § 4), embedded line breaks, the bytes
  ( ) { } [ ] , (§ 3.7 defines no key escape for them on a pair
  line), a leading ## — were emitted anyway, flipping the root kind,
  vanishing into comments, or splitting lines. The writers now
  return an error instead, following the CR precedent of § 5.9.7.
  The serde path additionally did not escape map keys at all
  ("a.b" silently became nesting); it now shares the render-side
  escaping and validation.

- Multi-line form selection had diverged across five copies. The
  canonical copy emitted verbatim for bodies containing both a
  sole-`)` and a sole-`))` line (unparseable pair, silently split
  item), and its stripped emitter trim_start()ed every content
  line. Selection now lives in one shared chooser (§ 5.6.1):
  stripped when unconditionally safe, verbatim otherwise, stripped
  with an unindented anchor line as the lossless fallback, and an
  error when no form can hold the body.

One test flips deliberately: values_with_leading_trailing_whitespace_
are_trimmed (from the 0.1.0 initial commit) asserted serializer
behaviour that spec 0.6 § 5.9.5(b) / § 5.9.7 later outlawed; it now
asserts the round-trip.

The parser is untouched, and output is unchanged for every previously
round-trippable value — verified by a differential run against v0.6.2
across all three surfaces and by the spec conformance fixtures.
tests/render_lossless.rs covers every case above on all three
surfaces (41 tests, written red-first).
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.

[0.6] Writer surfaces silently emit documents that re-parse to different data — or don't parse at all (§ 5.9.5(b), § 5.9.7, § 8.3)

1 participant