From 0ea83a6fff6c18bc12a6a071ccbfc2d8ee00fa4e Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 24 Jun 2026 15:35:59 +0300 Subject: [PATCH 1/2] Record node end position (EndLine/EndColumn) Node tracked only its start (Line/Column). Add EndLine/EndColumn, populated from the parser event's end_mark: for scalars/aliases at node construction, and for mappings/sequences from the matching MAPPING-END/SEQUENCE-END event so the span covers the whole block, not just its first line. This lets a consumer extract an entire collection (e.g. a whole OpenAPI operation block) from its node, which the change-origin work upstream builds on. TestNodeEndPosition covers scalar and nested-block spans, including that a block does not bleed into its sibling. TestNodeRoundtrip clears the new fields before its deep-equality check, since its expected literals predate them (end positions are covered by the new test). Co-Authored-By: Claude Opus 4.8 --- decode.go | 35 +++++++++ end_position_test.go | 166 +++++++++++++++++++++++++++++++++++++++++++ node_test.go | 18 +++++ yaml.go | 8 +++ 4 files changed, 227 insertions(+) create mode 100644 end_position_test.go diff --git a/decode.go b/decode.go index 4a6e0767..1dc4a1ea 100644 --- a/decode.go +++ b/decode.go @@ -187,6 +187,13 @@ func (p *parser) node(kind Kind, defaultTag, tag, value string) *Node { if !p.textless { n.Line = p.event.start_mark.line + 1 n.Column = p.event.start_mark.column + 1 + // end_mark is the position just past this event. For scalars and aliases + // it already spans the whole node. For mappings and sequences this is the + // MAPPING-START/SEQUENCE-START event, so it only marks the start for now; + // mapping()/sequence() overwrite it from the matching END event so the + // span covers the whole block. + n.EndLine = p.event.end_mark.line + 1 + n.EndColumn = p.event.end_mark.column + 1 n.HeadComment = string(p.event.head_comment) n.LineComment = string(p.event.line_comment) n.FootComment = string(p.event.foot_comment) @@ -264,6 +271,19 @@ func (p *parser) sequence() *Node { } n.LineComment = string(p.event.line_comment) n.FootComment = string(p.event.foot_comment) + // End at the last item's end so the span reaches the end of the actual + // content, consistent with scalars/aliases. The SEQUENCE-END token sits at + // the start of the following line after a block dedent, which would + // overshoot the element. Empty sequences fall back to that token's mark. + if !p.textless { + if len(n.Content) > 0 { + last := n.Content[len(n.Content)-1] + n.EndLine, n.EndColumn = last.EndLine, last.EndColumn + } else { + n.EndLine = p.event.end_mark.line + 1 + n.EndColumn = p.event.end_mark.column + 1 + } + } p.expect(yaml_SEQUENCE_END_EVENT) return n } @@ -304,6 +324,21 @@ func (p *parser) mapping() *Node { n.Content[len(n.Content)-2].FootComment = n.FootComment n.FootComment = "" } + // End at the last entry's value end so the span reaches the end of the + // actual content, consistent with scalars/aliases. The MAPPING-END token + // sits at the start of the following line after a block dedent, which would + // overshoot the element. Empty mappings fall back to that token's mark. + // (Origin __origin__ nodes are appended later, during decode, so the last + // element here is a real value.) + if !p.textless { + if len(n.Content) > 0 { + last := n.Content[len(n.Content)-1] + n.EndLine, n.EndColumn = last.EndLine, last.EndColumn + } else { + n.EndLine = p.event.end_mark.line + 1 + n.EndColumn = p.event.end_mark.column + 1 + } + } p.expect(yaml_MAPPING_END_EVENT) return n } diff --git a/end_position_test.go b/end_position_test.go new file mode 100644 index 00000000..71204283 --- /dev/null +++ b/end_position_test.go @@ -0,0 +1,166 @@ +package yaml_test + +import ( + yaml "github.com/oasdiff/yaml3" + . "gopkg.in/check.v1" +) + +// findKey returns the value node for the given key path in a decoded mapping. +func findKey(c *C, n *yaml.Node, path ...string) *yaml.Node { + cur := n + if cur.Kind == yaml.DocumentNode { + cur = cur.Content[0] + } + for _, key := range path { + c.Assert(cur.Kind, Equals, yaml.MappingNode, Commentf("node for key %q is not a mapping", key)) + var next *yaml.Node + for i := 0; i+1 < len(cur.Content); i += 2 { + if cur.Content[i].Value == key { + next = cur.Content[i+1] + break + } + } + c.Assert(next, NotNil, Commentf("key %q not found", key)) + cur = next + } + return cur +} + +// TestNodeEndPosition covers all four node kinds. The end position is recorded +// on two code paths: node() sets it for scalars and aliases (from the event's +// end_mark), while mapping() and sequence() derive it from their last child. +// Across all kinds the end means the same thing -- the position just past the +// last content character -- rather than the start of the following line, which +// is where the collection END token sits after a block dedent. +func (s *S) TestNodeEndPosition(c *C) { + // 1: mapping: + // 2: a: 1 + // 3: b: 2 + // 4: sequence: + // 5: - first + // 6: - second + // 7: base: &anchor + // 8: key: value + // 9: aliased: *anchor + // 10: plain: text + src := `mapping: + a: 1 + b: 2 +sequence: + - first + - second +base: &anchor + key: value +aliased: *anchor +plain: text +` + var doc yaml.Node + c.Assert(yaml.Unmarshal([]byte(src), &doc), IsNil) + + // Mapping (last-child path): starts at its first entry (line 2, col 3) and + // ends just past its last value `2` (line 3, col 7) -- the end of the actual + // content, not the start of the following line. + mapping := findKey(c, &doc, "mapping") + c.Assert(mapping.Kind, Equals, yaml.MappingNode) + c.Assert(mapping.Line, Equals, 2) + c.Assert(mapping.Column, Equals, 3) + c.Assert(mapping.EndLine, Equals, 3) + c.Assert(mapping.EndColumn, Equals, 7) + + // Sequence (last-child path): ends just past its last item `second` + // (line 6, col 11). + seq := findKey(c, &doc, "sequence") + c.Assert(seq.Kind, Equals, yaml.SequenceNode) + c.Assert(seq.Line, Equals, 5) + c.Assert(seq.Column, Equals, 3) + c.Assert(seq.EndLine, Equals, 6) + c.Assert(seq.EndColumn, Equals, 11) + + // Alias (node() path): a reference is a single token, so it starts and ends + // on the same line -- and its end is the alias token's (cols 10-17), not the + // span of the anchored mapping it points at. + alias := findKey(c, &doc, "aliased") + c.Assert(alias.Kind, Equals, yaml.AliasNode) + c.Assert(alias.Line, Equals, 9) + c.Assert(alias.Column, Equals, 10) + c.Assert(alias.EndLine, Equals, 9) + c.Assert(alias.EndColumn, Equals, 17) + + // Scalar (node() path): starts and ends on the same line (cols 8-12). + scalar := findKey(c, &doc, "plain") + c.Assert(scalar.Kind, Equals, yaml.ScalarNode) + c.Assert(scalar.Line, Equals, 10) + c.Assert(scalar.Column, Equals, 8) + c.Assert(scalar.EndLine, Equals, 10) + c.Assert(scalar.EndColumn, Equals, 12) +} + +// TestNodeEndPositionLastInDoc covers a collection that is the last element in +// the document: it ends at EOF rather than at a dedent to a following sibling. +// The end must still be its last child's end, not an overshoot to the line +// past the last content -- and it must hold with or without a trailing newline. +func (s *S) TestNodeEndPositionLastInDoc(c *C) { + // 1: top: 1 + // 2: block: + // 3: x: 10 + // 4: y: 20 + src := `top: 1 +block: + x: 10 + y: 20 +` + var doc yaml.Node + c.Assert(yaml.Unmarshal([]byte(src), &doc), IsNil) + + block := findKey(c, &doc, "block") + c.Assert(block.Kind, Equals, yaml.MappingNode) + c.Assert(block.EndLine, Equals, 4) // last content line, not the post-EOF line 5 + c.Assert(block.EndColumn, Equals, 8) // just past `20` + + // The document's root mapping ends where its last child does. + root := doc.Content[0] + c.Assert(root.EndLine, Equals, 4) + c.Assert(root.EndColumn, Equals, 8) + + // A trailing sequence ends at its last item, again without overshooting EOF. + var doc2 yaml.Node + c.Assert(yaml.Unmarshal([]byte("top: 1\nlist:\n - a\n - bb\n"), &doc2), IsNil) + list := findKey(c, &doc2, "list") + c.Assert(list.Kind, Equals, yaml.SequenceNode) + c.Assert(list.EndLine, Equals, 4) + c.Assert(list.EndColumn, Equals, 7) // just past `bb` + + // Robust to a missing trailing newline on the final element. + var doc3 yaml.Node + c.Assert(yaml.Unmarshal([]byte("top: 1\nblock:\n x: 10\n y: 20"), &doc3), IsNil) + block3 := findKey(c, &doc3, "block") + c.Assert(block3.EndLine, Equals, 4) + c.Assert(block3.EndColumn, Equals, 8) +} + +// TestNodeEndPositionEmpty covers the fallback for empty collections: with no +// last child to borrow an end from, mapping()/sequence() fall back to the +// END-event mark, which for a flow `{}`/`[]` is just past the closing delimiter. +func (s *S) TestNodeEndPositionEmpty(c *C) { + // 1: emptyMap: {} + // 2: emptySeq: [] + src := `emptyMap: {} +emptySeq: [] +` + var doc yaml.Node + c.Assert(yaml.Unmarshal([]byte(src), &doc), IsNil) + + em := findKey(c, &doc, "emptyMap") + c.Assert(em.Kind, Equals, yaml.MappingNode) + c.Assert(em.Content, HasLen, 0) + c.Assert(em.Line, Equals, 1) + c.Assert(em.EndLine, Equals, 1) + c.Assert(em.EndColumn, Equals, 13) // just past `}` (cols 11-12) + + es := findKey(c, &doc, "emptySeq") + c.Assert(es.Kind, Equals, yaml.SequenceNode) + c.Assert(es.Content, HasLen, 0) + c.Assert(es.Line, Equals, 2) + c.Assert(es.EndLine, Equals, 2) + c.Assert(es.EndColumn, Equals, 13) // just past `]` (cols 11-12) +} diff --git a/node_test.go b/node_test.go index b0f6bea3..4e768f4f 100644 --- a/node_test.go +++ b/node_test.go @@ -2555,6 +2555,19 @@ var nodeTests = []struct { }, } +// clearEndPos recursively zeroes EndLine/EndColumn across a decoded node tree. +// Used by TestNodeRoundtrip, whose expected literals predate those fields. +func clearEndPos(n *yaml.Node) { + if n == nil { + return + } + n.EndLine = 0 + n.EndColumn = 0 + for _, c := range n.Content { + clearEndPos(c) + } +} + func (s *S) TestNodeRoundtrip(c *C) { defer os.Setenv("TZ", os.Getenv("TZ")) os.Setenv("TZ", "UTC") @@ -2589,6 +2602,11 @@ func (s *S) TestNodeRoundtrip(c *C) { fprintComments(&buf, &node, " ") c.Logf(" obtained comments:\n%s", buf.Bytes()) } + // The expected node literals predate EndLine/EndColumn and don't set + // them. End positions are covered by TestNodeEndPosition; clear them + // here so this roundtrip check still verifies structure, content, + // start position, and comments against the upstream test data. + clearEndPos(&node) c.Assert(&node, DeepEquals, &item.node) } if encode { diff --git a/yaml.go b/yaml.go index 4f91be83..f8095e30 100644 --- a/yaml.go +++ b/yaml.go @@ -439,6 +439,14 @@ type Node struct { // These fields are not respected when encoding the node. Line int Column int + + // EndLine and EndColumn hold the position just past the end of the node in + // the decoded YAML text. For a mapping or sequence this spans the whole + // block (so a caller can extract the entire collection), not just its first + // line. Like Line and Column, these are 1-based and are not respected when + // encoding the node. + EndLine int + EndColumn int } // IsZero returns whether the node has all of its fields unset. From 0e5f33350ba4b871c9ce2b92862ea70e4a8f69d5 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 24 Jun 2026 17:22:05 +0300 Subject: [PATCH 2/2] Record block end in the __origin__ sequence buildOriginSeq now appends end_delta (line delta from key_line) and end_col for the whole mapping block, using the node EndLine/EndColumn added in the previous commit. This lets a consumer reconstruct an element's full span (e.g. an entire endpoint operation block) from its origin, not just the start. Appended after the sequences section so a consumer that stops there (today's kin-openapi originFromSeq) ignores the new fields -- backward compatible, no lockstep release required. TestOrigin_BlockEnd asserts the end reconstructs the block's last content line without bleeding into the next sibling; the existing golden origin tests are updated for the two new trailing entries. Co-Authored-By: Claude Opus 4.8 --- origin.go | 16 +++++++++++- origin_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/origin.go b/origin.go index 3f2f0a7e..2edec477 100644 --- a/origin.go +++ b/origin.go @@ -33,7 +33,7 @@ func addOriginInMap(key, n *Node, file string) *Node { // addOrigin injects a compact __origin__ sequence into the mapping node n. // -// Format: [file, key_name, key_line, key_col, nf, f1_name, f1_delta, f1_col, ..., ns, s1_name, s1_count, s1_l0_delta, s1_c0, ...] +// Format: [file, key_name, key_line, key_col, nf, f1_name, f1_delta, f1_col, ..., ns, s1_name, s1_count, s1_l0_delta, s1_c0, ..., end_delta, end_col] // // - file: source file path // - key_name: the YAML key whose value is this mapping @@ -42,6 +42,10 @@ func addOriginInMap(key, n *Node, file string) *Node { // - per field: name (string), line delta from key_line (int), column (int) // - ns: number of sequence fields that have item locations // - per sequence: name (string), item count (int), then count × (line delta, col) +// - end_delta, end_col: end of the whole mapping block — line delta from +// key_line and absolute column of the position just past its last content. +// Appended last so a consumer that stops after the sequences section +// simply ignores it (backward compatible). func addOrigin(key, n *Node, file string) *Node { if isOrigin(key) { return n @@ -108,6 +112,16 @@ func buildOriginSeq(key, n *Node, file string) []*Node { nodes = append(nodes, fieldNodes...) nodes = append(nodes, intNode(ns)) nodes = append(nodes, seqNodes...) + + // Block end: line delta from key_line and absolute end column of the whole + // mapping. Lets a consumer reconstruct the full block span + // [key_line, key_line+end_delta] -- e.g. an entire endpoint operation block. + endDelta, endCol := 0, 0 + if n.EndLine > 0 { + endDelta = n.EndLine - key.Line + endCol = n.EndColumn + } + nodes = append(nodes, intNode(endDelta), intNode(endCol)) return nodes } diff --git a/origin_test.go b/origin_test.go index 124fbde5..4a4d4f0b 100644 --- a/origin_test.go +++ b/origin_test.go @@ -68,6 +68,8 @@ __origin__: - 0 - 1 - 0 + - 3 + - 17 root: __origin__: - file.yaml @@ -82,6 +84,8 @@ root: - 2 - 5 - 0 + - 3 + - 17 hello: world object: __origin__: @@ -94,6 +98,8 @@ root: - 1 - 9 - 0 + - 1 + - 17 foo: bar ` @@ -132,6 +138,8 @@ __origin__: - 0 - 1 - 0 + - 5 + - 19 root: __origin__: - file.yaml @@ -143,6 +151,8 @@ root: - 1 - 5 - 0 + - 5 + - 19 continents: - __origin__: - file.yaml @@ -157,6 +167,8 @@ root: - 1 - 11 - 0 + - 1 + - 19 name: europe size: 10 - __origin__: @@ -172,6 +184,8 @@ root: - 1 - 11 - 0 + - 1 + - 19 name: america size: 20 ` @@ -214,6 +228,8 @@ __origin__: - 0 - 1 - 0 + - 5 + - 23 parent: __origin__: - spec.yaml @@ -228,6 +244,8 @@ parent: - 2 - 5 - 0 + - 5 + - 23 labels: __origin__: - spec.yaml @@ -245,6 +263,8 @@ parent: - 3 - 9 - 0 + - 3 + - 23 env: production region: us-east version: "2.0" @@ -289,6 +309,8 @@ __origin__: - 0 - 1 - 0 + - 5 + - 18 schema: __origin__: - spec.yaml @@ -314,6 +336,8 @@ schema: - integer - 5 - 11 + - 5 + - 18 description: a test type: - string @@ -471,6 +495,51 @@ alias: *schema c.Assert(ns > 0, Equals, true, Commentf("alias __origin__ must record sequence item locations")) } +// TestOrigin_BlockEnd verifies the trailing end_delta/end_col appended to each +// __origin__ sequence reconstruct the end of the whole block (the position just +// past its last content), which is how kin-openapi recovers an endpoint's span. +func (s *S) TestOrigin_BlockEnd(c *C) { + // 1: paths: + // 2: /pets: + // 3: get: + // 4: summary: list + // 5: x: "y" + // 6: /health: + input := `paths: + /pets: + get: + summary: list + x: "y" + /health: + get: + summary: ok +` + dec := yaml.NewDecoder(bytes.NewBufferString(input)) + dec.Origin(true, "spec.yaml") + var out any + err := dec.Decode(&out) + c.Assert(err, IsNil) + + get := out.(map[string]any)["paths"].(map[string]any)["/pets"].(map[string]any)["get"].(map[string]any) + seq, ok := get["__origin__"].([]any) + c.Assert(ok, Equals, true, Commentf("get block must carry __origin__")) + + keyLine := toAnyInt(seq[2]) // header: file, key_name, key_line, key_col, ... + c.Assert(keyLine, Equals, 3, Commentf("get key is on line 3")) + + // end_delta, end_col are the last two entries. + endDelta := toAnyInt(seq[len(seq)-2]) + endCol := toAnyInt(seq[len(seq)-1]) + endLine := keyLine + endDelta + // When a block is followed by a dedented sibling, the end lands on the + // block's last content line (x: "y" on line 5), inclusive. The key property + // for block extraction: it covers the whole get block and does not bleed + // into the /health sibling on line 6. + c.Assert(endLine, Equals, 5, Commentf("get block should end at its last content line (5), not bleed into the sibling; got %d", endLine)) + // End column is just past the last content (`x: "y"` ends at col 12, so 13). + c.Assert(endCol, Equals, 13, Commentf("end column should be just past the last content; got %d", endCol)) +} + func (s *S) TestOrigin_DuplicateKey(c *C) { input := ` root: