Yaml position - #6
Merged
Merged
Conversation
A consumer laying YL's tokens out by position — colouring YAML in a TUI — had
to sort each line's spans by column and discard tokens reporting line 0. Both
workarounds were needed for YL and neither for the JSON lexer.
Cause: block collections have no "{" / "[" / "}" / "]" characters, so goccy has
no token to point a container delimiter at. Confirmed against the AST:
block: MappingNode IsFlowStyle=false Start=":"@L1C5 End=<nil>
SequenceNode IsFlowStyle=false Start="-"@L2C3 End=<nil>
flow: SequenceNode IsFlowStyle=true Start="["@l1c7 End="]"@L1C12
Start is the first entry's SEPARATOR and End is nothing. Used literally, that
put the opening delimiter after the key it precedes,
info:
title: Petstore
L2 C8 delimiter /info <- the colon of `title`, a line below
L2 C3 key /info/title <- column goes 8 -> 3
so the stream contradicted its own emission order, and left every closing
delimiter at line 0 column 0 — not a position at all, since lines are 1-based.
This affected every block container at every depth, including the ROOT mapping,
which is why it showed up even in documents whose inner collections are flow.
The delimiters now take the SPAN of what they enclose: the opener reports the
first token inside, the closer the last. The span is read back off the emitted
stream rather than computed from the AST, so it composes with everything the
walk does to the children — merge-key resolution, alias expansion, tags, nested
containers — none of which the node's own tokens know about. Flow collections
keep their real delimiter positions untouched.
Two consequences worth stating:
- Offset moves with Line/Column (same source position), so closing delimiters
now carry a real offset instead of 0.
- Block delimiters share their neighbour's position rather than sitting strictly
before/after it: there is no character of their own to point at. Consumers
must order by non-decreasing position, not strictly increasing.
Also fixes an implicit null reporting no position at all: an empty, comment-only
or header-only document, and a "key:" with nothing after it. It now inherits the
preceding token's position, or the start of input when it IS the whole stream.
Found by running the invariants over the vendored test suite, not by hand — six
documents the worked examples missed.
Positions had NO test coverage before this, which is how it shipped. Now pinned:
exact spans for block and flow, no zero positions, non-decreasing order over 199
suite documents, and Offset inside the input. The conformance baseline is
unchanged (204/85/63) — that comparison covers token kind and value, not
position, so it could never have caught this.
Aliases still report the ANCHOR DEFINITION site and so can still go backwards.
That is by design (see walkAlias) and now has a test saying so, so it stays a
decision rather than becoming an accident.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
The prose said "two DK95 tab cases"; the third case is actually VJP3/1, and DK95/7 is a multi-document case (which conformanceXFail already had right — only the write-up was wrong). Also names what 4MUZ/2 and VJP3/1 have in common, since it turns out to be one defect rather than two: inside a FLOW mapping a line break between the key and its ":" is legal YAML 1.2, but goccy applies the block-context rule that they share a line. Verified directly against parser.ParseBytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…ocument is a boundary Two corrections to the record, both prompted by re-proposing things that were already settled. Separators: DESIGN.md already said YL never emits ":" / "," and that this is not an option. What it did not say is WHY, or that the tokens are available — so the question keeps coming back as if it were blocked on goccy. It is not: goccy exposes the ":" of every pair (MappingValueNode.Start) and the "-" of every block item (SequenceEntryNode.Start). The reason is that a separator mode is error-prone for no gain — consumers index by the value tokens, YAML's separators do not map onto JSON's, and a block "-" would need either a new delimiter kind in the shared token package (a JSON-side change for a YAML-only need) or a Comma token whose String() reports the wrong character. Recorded so the next reader does not rediscover the availability and mistake it for an argument. Multi-document: the 14 cases were counted among "54 divergences", which reads as 54 things needing attention. YL's model is structurally single-document, so those are an acknowledged boundary; they stay in the xfail table only so their verdict cannot drift unnoticed. The number worth acting on is 40. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
A JSON object key is a string, so YL only accepts a key denoting a scalar. But
YAML lets a key carry node properties and indirection that JSON has no place for
and that do not stop it being a string:
? explicit key : v MappingKeyNode
!!str key : v TagNode
&anchor key : v AnchorNode
*alias : v AliasNode
and combinations of those ("? !!str &a foo"). All of them were rejected as
"complex keys", and they are the single largest group of conformance
divergences: 23 of the YAML Test Suite's documents turn on it, and the suite's
own JSON equivalent shows the resolved string as the key in every one.
resolveKey now peels each wrapper until a scalar is reached. An anchor written on
a KEY is registered as it is peeled, so a later "*a" resolves to it, and alias
resolution shares the existing cycle guard so a key aliasing its own definition
terminates instead of recursing. resolveMapping's merge-key precedence goes
through the same resolver, so a wrapped key inside a merge-bearing mapping
dedupes correctly too.
A key that really denotes a sequence or mapping stays rejected. Worth recording:
written out directly ("? [a, b] : v") goccy refuses the document before we see
it, so ErrComplexKey is now reachable only through indirection — an alias naming
a collection. The tests pin both paths.
accept + match 204/272 -> 226/272
comparable slice 79% -> 85%
which is above every implementation in the middle band of matrix.yaml.info's
JSON comparison (PyYAML 75%, psych 76%, go-yaml 76%, ruamel 78%).
22 of the 23 flipped. The 23rd, RR7F, turned out not to be ours at all: it is a
defect in the FIXTURE. Its event stream and canonical dump both order the keys
a, d; only its json field writes d first. The suite compares loaded data as
unordered maps, so nothing there ever checked its JSON text against its own tree.
We keep key order — a JSON token stream is ordered, and so is our model — so we
match the event stream. Recorded as such rather than worked around, and worth
reporting to yaml-test-suite.
The recorded-behaviour golden caught two out-of-scope documents flipping reject
-> accept as a side effect (6M2F, DFF7 — both valid YAML with explicit/alias keys
and no JSON equivalent). Both are improvements; blessed deliberately rather than
silently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
…efect yaml/yaml-test-suite#179. That repository appears largely unmaintained, so the xfail entry is recorded as permanent rather than pending — nothing is waiting on it. If the fixture is ever corrected the entry reports itself as an unexpected pass, which is the signal to remove it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
CI fuzz case 509dec0845c6303e: "\t0" lexed as the STRING "0" under
YL and the NUMBER 0 under L, tripping the JSON-subset differential.
YAML 1.2 allows a document to be prefixed by a byte order mark and it is not
content, but goccy does not strip it. The consequence is not a dirty value, it
is a different parse — the mark becomes the first character of the first token:
"<BOM>{}" -> the SCALAR "<BOM>{}", not an empty mapping
"<BOM>[1]" -> the SCALAR "<BOM>[1]", not a sequence
"<BOM>a: 1" -> the key "<BOM>a"
So it has to go before the parser sees it; trimming it off the tokens afterwards
would not recover the structure. YL now strips it in build().
The divergence is new but only half of it was: L started consuming a leading BOM
in the UTF-8 round, which is what made the differential fire. YL mangling a
BOM-prefixed document predates that and was simply never compared against
anything.
Positions are put back on the caller's coordinates, since the parser now sees an
input three bytes shorter than the caller's: offsets gain the mark's width, and
line 1 columns gain one character (a BOM is one character, and it is all on line
one). Pinned by comparing every token of a BOM'd document against the same
document without one.
Tests: the parse is unchanged by a leading BOM across eight document shapes; the
stream matches L's for the documents both accept (the fuzzer's invariant, pinned
directly); U+FEFF anywhere else is ordinary content, including the second of two
leading marks. The four shapes are seeded into FuzzYL. 2M fuzz executions clean.
Conformance is unchanged at 226/85/63 — the suite's BOM fixtures are structure
tests that goccy already rejected or accepted the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6 +/- ##
=========================================
Coverage ? 63.88%
=========================================
Files ? 90
Lines ? 9681
Branches ? 0
=========================================
Hits ? 6185
Misses ? 2994
Partials ? 502 ☔ View full report in Codecov by Harness. |
YL.Offset reported goccy's Position.Offset unchanged, which is not a byte offset at all. Consumers were subtracting 1 to make it usable; that workaround is only correct for pure-ASCII documents with no comments. goccy's scanner holds the source as []rune and advances its offset once per rune, starting at 1, so Position.Offset is a 1-based RUNE index. It also loses one more per preceding comment line (goccy #856). The two errors have opposite signs, so on an ASCII document with exactly one comment they cancel and the raw value looks correct -- which is why subtracting 1 seemed to work. Line and Column are correct under both defects, so the offset is now DERIVED from them against a line-start index built once per parse, and Position.Offset is not consumed anywhere. src[Offset()] is the token's first byte, for any document: comments, accented text, CJK, emoji, and a leading BOM. Note this deliberately does not match L.Offset despite the shared name: L reports bytes consumed, a cursor pointing PAST the token it returned, while YL reports the token's own start. Both are now documented as distinct, in the API and the README. Costs one O(n) scan of the source per parse and two small slices. Benchmarked over an OpenAPI-shaped document at 2K and 38K: no measurable change (geomean +0.6%, p>0.5 at n=8), since a pure-ASCII line -- almost every line -- resolves in O(1). Also corrects a claim left stale by the earlier block-position work, in both the API docs and the README: block-style closers no longer report 0/0/0. They report the span of what they enclose, so order positions by non-decreasing rather than strictly increasing. BREAKING CHANGE: YL.Offset() values shift. Callers compensating with -1 must drop it. Offsets in documents with comments or non-ASCII text change by more than 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist