Skip to content

Commit fa42133

Browse files
dmealingclaude
andcommitted
feat(metadata,ts): yaml-desugar tracks line/col positions [FR5b]
Adds a position-by-key map (Symbol.for("@metaobjectsdev/yamlPositionByKey"), non-enumerable so JSON.stringify and Object.keys ignore it) to every mapping object the YAML pipeline produces. The carrier rides along through the Rule-1/2/4/5 desugar transforms — Rule-2's scalar lift inherits the wrapper key's position on its synthesized `name` slot, Rule-4's `[]` suffix keeps the position under the un-suffixed canonical key, Rule-5's sigil-free attrs re-key the position under the `@`-prefixed canonical name. Anchor aliases resolve to the target value (matches yaml.toJS()). parser-yaml swaps from `yaml.parse()` to the position-preserving walker (parseYamlWithPositions, in core/yaml-positions-walker.ts). The browser bundle stays clean: walker lives next to parser-yaml.ts (server-only), and the pure-types accessor module (core/yaml-positions.ts) carries no `yaml` import — the browser-safety test still passes. Resolves the FR5b spec's per-node-vs-per-error-site brainstorm by implementing per-node (mirrors FR5a source-on-node). Resolves the desugar-synthesized-node brainstorm by inheriting the wrapper key's position on the synthesized `name` slot (Rule 2) and omitting position on truly empty-body nodes (Rule 3). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dd8ed02 commit fa42133

4 files changed

Lines changed: 262 additions & 12 deletions

File tree

server/typescript/packages/metadata/src/core/parser-yaml.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,31 @@
11
// Authoring YAML parser.
22
//
3-
// parseYaml is a front-end: yaml.parse → desugar → the shared buildTree
4-
// (parser-core.ts). The desugar applies the four authoring-sugar rules so the
5-
// resulting typed tree is identical to the one the equivalent canonical JSON
6-
// produces.
3+
// parseYaml is a front-end: parseYamlWithPositions → desugar → the shared
4+
// buildTree (parser-core.ts). The desugar applies the four authoring-sugar
5+
// rules so the resulting typed tree is identical to the one the equivalent
6+
// canonical JSON produces.
7+
//
8+
// FR5b: the parse phase now preserves YAML source positions through the
9+
// pipeline. parseYamlWithPositions attaches a Symbol-keyed position-by-key
10+
// map onto every mapping object; desugar shallow-copies that property; and
11+
// buildTree's `format: "yaml"` mode reads the position when stamping
12+
// `node.source.yamlPosition` (see parser-core.ts).
713

8-
import { parse as parseYamlText } from "yaml";
914
import { ParseError } from "../errors.js";
1015
import { buildTree } from "../parser-core.js";
1116
import type { ParseOptions, ParseResult } from "../parser-core.js";
1217
import type { ErrorSource } from "../source.js";
1318
import { desugar } from "./yaml-desugar.js";
19+
import { parseYamlWithPositions } from "./yaml-positions-walker.js";
1420

1521
/** FR5a / ADR-0009 — build a YAML-source envelope rooted at "$".
16-
* yamlPosition is FR5b territory and not populated here. */
22+
* yamlPosition on the envelope is left undefined here; envelopes for
23+
* THROWN parser errors carry positions only when the parse phase pushed
24+
* the path into the live parser-core builder (see populateNodeSource in
25+
* parser-core.ts). Errors raised before buildTree (e.g. yaml syntax
26+
* failures, an empty desugar result) lack a node — `yamlPosition` is
27+
* intentionally omitted, per the spec's "skip on desugar-synthesized
28+
* nodes" decision. */
1729
function yamlSource(sourceName: string | undefined): ErrorSource {
1830
return {
1931
format: "yaml",
@@ -31,7 +43,7 @@ export function parseYaml(content: string, opts: ParseOptions): ParseResult {
3143
// The loader's per-source try/catch collects the throw into LoadResult.errors.
3244
let parsed: unknown;
3345
try {
34-
parsed = parseYamlText(normalizedContent);
46+
parsed = parseYamlWithPositions(normalizedContent).value;
3547
} catch (err) {
3648
throw new ParseError(
3749
`Invalid YAML: ${(err as Error).message}`,
@@ -52,7 +64,10 @@ export function parseYaml(content: string, opts: ParseOptions): ParseResult {
5264
);
5365
}
5466

55-
const result = buildTree(canonical, opts);
67+
// FR5b — buildTree needs to know the source-format discriminant so
68+
// populateNodeSource emits `format: "yaml"` envelopes (with the
69+
// optional yamlPosition) instead of the default `format: "json"`.
70+
const result = buildTree(canonical, { ...opts, sourceFormat: "yaml" });
5671

5772
// Merge collected desugar errors ahead of buildTree's own collected errors.
5873
// Each CollectedError carries its own stable code when set (e.g.

server/typescript/packages/metadata/src/core/yaml-desugar.ts

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ import {
3838
RESERVED_KEY_IS_ARRAY,
3939
TYPE_SUBTYPE_SEPARATOR,
4040
} from "../shared/structural.js";
41+
import {
42+
getPositionMap,
43+
setPositionMap,
44+
type PositionMap,
45+
} from "./yaml-positions.js";
4146
import {
4247
ATTR_SUBTYPE_STRING,
4348
ATTR_SUBTYPE_CLASS,
@@ -99,6 +104,11 @@ function desugarNode(
99104
const rawKey = entries[0]!;
100105
const rawBody = (input as Record<string, unknown>)[rawKey];
101106

107+
// FR5b — capture the wrapper-level position-by-key map BEFORE re-keying.
108+
// The author's raw key (with `[]` suffix and possibly omitted subType) is
109+
// the lookup key; the desugar's canonical key is what we emit.
110+
const wrapperPositions = getPositionMap(input);
111+
102112
// Rule 4: a trailing "[]" on the key → isArray.
103113
let key = rawKey;
104114
let isArray = false;
@@ -111,7 +121,10 @@ function desugarNode(
111121
const canonicalKey = resolveKey(key, registry, errors, path);
112122

113123
// Rule 2: a scalar body → { name: <scalar> }.
114-
const body = desugarBody(rawBody, registry, canonicalKey, errors, path);
124+
// FR5b — propagate the wrapper-key's position into the synthesized body
125+
// when the input body was a scalar (no body-side positions to inherit).
126+
const wrapperKeyPos = wrapperPositions?.[rawKey];
127+
const body = desugarBody(rawBody, registry, canonicalKey, errors, path, wrapperKeyPos);
115128

116129
// Rule 4 (cont.): stamp isArray onto the canonical body.
117130
if (isArray) body[RESERVED_KEY_IS_ARRAY] = true;
@@ -131,7 +144,16 @@ function desugarNode(
131144
}
132145
// A non-array `children` value is left untouched — buildTree reports it.
133146

134-
return { [canonicalKey]: body };
147+
// FR5b — emit a wrapper-level position-by-key map for the canonical wrapper
148+
// so buildTree's per-child iteration can read the position via the same
149+
// lookup it uses for JSON input. The single key transformation is
150+
// rawKey → canonicalKey (Rule 1 fuses the subType, Rule 4 strips `[]`).
151+
const outWrapper: Record<string, unknown> = { [canonicalKey]: body };
152+
if (wrapperKeyPos !== undefined) {
153+
setPositionMap(outWrapper, { [canonicalKey]: wrapperKeyPos });
154+
}
155+
156+
return outWrapper;
135157
}
136158

137159
// Rule 1 — resolve a possibly-bare key to a fused `type.subType` token.
@@ -169,16 +191,30 @@ function desugarBody(
169191
canonicalKey: string,
170192
errors: CollectedError[],
171193
path: string,
194+
/** FR5b — position of the WRAPPER key (the `field.string:` line). Used to
195+
* back-fill `yamlPosition` on synthesized bodies (Rule 2's scalar lift) +
196+
* empty bodies; for mapping bodies we use the body's own position-by-key
197+
* map. */
198+
wrapperKeyPos: { line: number; col: number } | undefined,
172199
): Record<string, unknown> {
173200
if (
174201
typeof rawBody === "string" ||
175202
typeof rawBody === "number" ||
176203
typeof rawBody === "boolean"
177204
) {
178-
return { [RESERVED_KEY_NAME]: rawBody };
205+
// FR5b — the synthesized `{ name: rawBody }` has no YAML-side
206+
// counterpart; we attribute the `name` slot to the wrapper-key's
207+
// position (the only YAML position that meaningfully belongs to this
208+
// synthesis).
209+
const out: Record<string, unknown> = { [RESERVED_KEY_NAME]: rawBody };
210+
if (wrapperKeyPos !== undefined) {
211+
setPositionMap(out, { [RESERVED_KEY_NAME]: wrapperKeyPos });
212+
}
213+
return out;
179214
}
180215
if (rawBody === null || rawBody === undefined) {
181216
// An empty body (`field.string:` with nothing after) → an empty node.
217+
// No body keys to position; the wrapper carries the node's position.
182218
return {};
183219
}
184220
if (Array.isArray(rawBody)) {
@@ -192,20 +228,38 @@ function desugarBody(
192228
// Rule D2 (type-coercion guard).
193229
const src = rawBody as Record<string, unknown>;
194230
const out: Record<string, unknown> = {};
231+
// FR5b — translate the body's position-by-key map across the sigil-free
232+
// rewrite. A bare `filterable` key in the source maps to `@filterable` in
233+
// the canonical body; the YAML position belongs to BOTH names (the YAML
234+
// author only wrote one). We re-key the position map to match the canonical
235+
// body's keys so buildTree's per-attr inspection (FR5b follow-ups, e.g.
236+
// ERR_BAD_ATTR_VALUE) can find the position via the canonical key.
237+
const srcPositions = getPositionMap(src);
238+
const outPositions: PositionMap = {};
239+
let hasOutPositions = false;
195240
const schemaIndex = attrSchemaIndex(registry, canonicalKey);
196241
for (const key of Object.keys(src)) {
242+
let outKey: string;
197243
if (RESERVED_KEYS.has(key) || key.startsWith(ATTR_PREFIX)) {
198244
out[key] = src[key];
245+
outKey = key;
199246
// D2 also applies to author-written @-keys (the awkward form).
200247
const attrName = key.startsWith(ATTR_PREFIX) ? key.slice(ATTR_PREFIX.length) : "";
201248
if (attrName !== "" && !RESERVED_KEYS.has(attrName)) {
202249
checkCoercion(attrName, src[key], schemaIndex, errors, path);
203250
}
204251
} else {
205-
out[`${ATTR_PREFIX}${key}`] = src[key];
252+
outKey = `${ATTR_PREFIX}${key}`;
253+
out[outKey] = src[key];
206254
checkCoercion(key, src[key], schemaIndex, errors, path);
207255
}
256+
const pos = srcPositions?.[key];
257+
if (pos !== undefined) {
258+
outPositions[outKey] = pos;
259+
hasOutPositions = true;
260+
}
208261
}
262+
if (hasOutPositions) setPositionMap(out, outPositions);
209263
return out;
210264
}
211265

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// FR5b — YAML AST → JS walker that preserves source positions.
2+
//
3+
// This module is the only place inside @metaobjectsdev/metadata that
4+
// imports the `yaml` package. It lives in core/ alongside parser-yaml.ts,
5+
// and is reached only via that parser — never via src/index.ts. The
6+
// browser-safety test guards this invariant.
7+
8+
import {
9+
parseDocument,
10+
isAlias,
11+
isMap,
12+
isScalar,
13+
isSeq,
14+
LineCounter,
15+
type Document,
16+
} from "yaml";
17+
18+
import {
19+
setPositionMap,
20+
type PositionMap,
21+
} from "./yaml-positions.js";
22+
23+
/** Result of parsing YAML text with positions retained. */
24+
export interface YamlParseResult {
25+
/** The JS object (same shape as `yaml.parse(text)` returns), with
26+
* position-by-key maps attached to every mapping. */
27+
value: unknown;
28+
/** The yaml library's LineCounter — exposed for callers that need to map
29+
* additional ranges (e.g. surfacing errors raised by the YAML library
30+
* itself). */
31+
lineCounter: LineCounter;
32+
}
33+
34+
/** Parse YAML text and return a JS object with positions attached.
35+
*
36+
* Mirrors the contract of `yaml.parse(text)` for the shapes the metaobjects
37+
* authoring grammar uses (mappings, sequences, scalars). Aliases and tags
38+
* are deferred via the underlying parseDocument call — i.e. they resolve as
39+
* the library normally would.
40+
*
41+
* Throws on YAML syntax errors (same behavior as `yaml.parse`). */
42+
export function parseYamlWithPositions(text: string): YamlParseResult {
43+
const lineCounter = new LineCounter();
44+
const doc = parseDocument(text, { lineCounter });
45+
// Surface YAML syntax errors as a throw, matching `yaml.parse` behavior.
46+
// (parseDocument collects them rather than throwing.)
47+
if (doc.errors.length > 0) {
48+
throw doc.errors[0]!;
49+
}
50+
const value = yamlNodeToJs(doc.contents, lineCounter, doc);
51+
return { value, lineCounter };
52+
}
53+
54+
// Walk a yaml AST node into a JS structure. For each YAMLMap, attach a
55+
// position-by-key map onto the resulting JS object — the position of each
56+
// key is the (line, col) of the KEY token in the YAML source.
57+
function yamlNodeToJs(
58+
node: unknown,
59+
lineCounter: LineCounter,
60+
doc: Document,
61+
): unknown {
62+
if (node === null || node === undefined) return null;
63+
if (isScalar(node)) {
64+
// Honour the library's default scalar typing (numbers / booleans /
65+
// strings / null all come through Scalar.value).
66+
return node.value;
67+
}
68+
if (isAlias(node)) {
69+
// Resolve an anchor alias (e.g. `*col` after `&col sku_code`) to its
70+
// target value — same behaviour as the library's toJS().
71+
const target = node.resolve(doc);
72+
return yamlNodeToJs(target, lineCounter, doc);
73+
}
74+
if (isMap(node)) {
75+
const out: Record<string, unknown> = {};
76+
const positions: PositionMap = {};
77+
let hasAnyPosition = false;
78+
for (const pair of node.items) {
79+
// Only string-keyed entries are valid in metaobjects authoring; ignore
80+
// exotic keys (numeric / complex) — they'd already break the desugar.
81+
if (!isScalar(pair.key)) continue;
82+
const keyText = String(pair.key.value);
83+
const valueJs = yamlNodeToJs(pair.value, lineCounter, doc);
84+
out[keyText] = valueJs;
85+
const keyRange = pair.key.range;
86+
if (keyRange !== null && keyRange !== undefined) {
87+
const pos = lineCounter.linePos(keyRange[0]);
88+
positions[keyText] = { line: pos.line, col: pos.col };
89+
hasAnyPosition = true;
90+
}
91+
}
92+
if (hasAnyPosition) setPositionMap(out, positions);
93+
return out;
94+
}
95+
if (isSeq(node)) {
96+
return node.items.map((item) => yamlNodeToJs(item, lineCounter, doc));
97+
}
98+
// Tags / unsupported — fall back to null. The metaobjects authoring
99+
// grammar does not use them.
100+
return null;
101+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// FR5b — YAML authoring source-position carrier (per ADR-0009).
2+
//
3+
// This module is split into two layers so the browser bundle (which
4+
// imports from src/index.ts) stays free of the Node-only `yaml` package:
5+
//
6+
// - yaml-positions.ts (this file) — pure types + Symbol + accessors. NO
7+
// `yaml` import. Imported by parser-core.ts so the source-on-node
8+
// stamper can read positions when stamping `format: "yaml"` envelopes.
9+
// - yaml-positions-walker.ts — depends on `yaml`. Imported only by
10+
// parser-yaml.ts (and parser-yaml itself only ships server-side).
11+
//
12+
// Source-map carrier (per the FR5b spec's "open question" §2): a
13+
// Symbol-keyed, non-enumerable property on the wrapper-mapping object. The
14+
// symbol is the well-known cross-port key
15+
// `Symbol.for("@metaobjectsdev/yamlPositionByKey")`, so any plugin that
16+
// touches the canonical JS can read positions if it knows to look. The
17+
// map's keys are the wrapper's own keys (e.g. "object.entity" for a
18+
// wrapper `{ "object.entity": { ... } }` or "name" / "package" /
19+
// "children" for the body keys of a node).
20+
//
21+
// Rationale for "symbol-keyed property" over a parallel sourcemap / wrapper
22+
// type:
23+
// - Invisible to JSON.stringify and Object.keys (non-enumerable).
24+
// - No parallel data structure to keep in sync — the position rides with
25+
// the node it describes.
26+
// - No wrapper type — desugar still operates on plain JS objects, so the
27+
// existing Rule 1–5 logic does not need a rewrite.
28+
//
29+
// On desugar-synthesized nodes (Rule 2's scalar-body lift): the synthesized
30+
// body `{ name: rawScalar }` inherits the wrapper key's position from the
31+
// parent's position map. On any other synthesis (Rule 4's isArray stamping,
32+
// for example), the position survives because we shallow-copy via the
33+
// existing desugar path.
34+
35+
/** Cross-port well-known symbol key for the position-by-key map. */
36+
export const YAML_POSITION_BY_KEY = Symbol.for(
37+
"@metaobjectsdev/yamlPositionByKey",
38+
);
39+
40+
/** A YAML source position — 1-indexed line and column. */
41+
export interface YamlPosition {
42+
readonly line: number;
43+
readonly col: number;
44+
}
45+
46+
/** The position-by-key map attached to a mapping object. */
47+
export type PositionMap = Record<string, YamlPosition>;
48+
49+
/** Read the position-by-key map from a JS object, if present.
50+
* Returns undefined for primitives, arrays, null, and untagged objects. */
51+
export function getPositionMap(obj: unknown): PositionMap | undefined {
52+
if (obj === null || typeof obj !== "object" || Array.isArray(obj)) {
53+
return undefined;
54+
}
55+
return (obj as { [YAML_POSITION_BY_KEY]?: PositionMap })[YAML_POSITION_BY_KEY];
56+
}
57+
58+
/** Read the position for a specific key on a mapping object. */
59+
export function getYamlPosition(
60+
obj: unknown,
61+
key: string,
62+
): YamlPosition | undefined {
63+
const map = getPositionMap(obj);
64+
return map?.[key];
65+
}
66+
67+
/** Attach (or replace) the position-by-key map on a mapping object. The map
68+
* property is non-enumerable so JSON.stringify and `for (const k in obj)`
69+
* loops do not see it. */
70+
export function setPositionMap(
71+
obj: Record<string, unknown>,
72+
positions: PositionMap,
73+
): void {
74+
Object.defineProperty(obj, YAML_POSITION_BY_KEY, {
75+
value: positions,
76+
enumerable: false,
77+
writable: true,
78+
configurable: true,
79+
});
80+
}

0 commit comments

Comments
 (0)