From 976bcc2a2ca50fba37b0b922df11d3e521b6ef83 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Tue, 10 Nov 2026 22:05:56 +0100 Subject: [PATCH] =?UTF-8?q?Answer=20stability=20round=201:=201=20?= =?UTF-8?q?=F0=9F=94=B4,=204=20=F0=9F=9F=A1=20(0=20misread);=20one=20walke?= =?UTF-8?q?r=20for=20both=20refusals,=20nodeRefusal=20split,=20two=20tests?= =?UTF-8?q?=20added,=20the=20decision=20and=20the=20changelog=20narrowed?= =?UTF-8?q?=20to=20what=20holds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 7 ++-- README.md | 2 +- docs/decisions.md | 10 +++--- src/adf/document.test.ts | 25 +++++++------- src/adf/document.ts | 73 ++++++++++++++++++++++------------------ 5 files changed, 64 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2f0348..d273287 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,10 @@ ## Unreleased -- `readAdfDocument` and every converter name the node a `not-an-adf-document` refusal concerns: - `path` leads to it, and the message says what is wrong, such as which attribute holds a value - JSON cannot. +- `readAdfDocument` and the three writers name the node `not-an-adf-document` refuses: `path` leads + to a node that is not an ADF node, and the message says what is wrong with it, such as which + attribute holds a value that is not JSON. A getter, an object holding itself, a throwing Proxy + and a bad array length are still refused at the empty path. - **Breaking:** `portableMarkdownToAdf` reads more of GFM's pipe table. A body row holding fewer cells than the header is padded with empty cells, where `0.3.1` returned `malformed-pipe-table`; a table whose padding would add more cells than the table holds is `malformed-pipe-table`. diff --git a/README.md b/README.md index d0bfc53..7d5e884 100644 --- a/README.md +++ b/README.md @@ -615,7 +615,7 @@ Which reader raises which code: [The calls](#the-calls). Codes every direction r | Code | Fires when | What you can do | | --- | --- | --- | -| `not-an-adf-document` | the value handed in is no ADF document — a missing or wrong `type`, a stray key, a node that is not a node, an object in the document holding itself, an accessor property, a Proxy that throws, an attribute value that is not a plain object, array or JSON primitive, such as a `Date` or `Map` | read the boundary you receive JSON at with `readAdfDocument`; the message names the branch that refused. Where your objects hold getters (Vue 2, MobX, a class model), hand in a snapshot, `JSON.parse(JSON.stringify(document))`. Build an attribute value as JSON holds it: `date.toISOString()`, `Object.fromEntries(map)`, `{ ...instance }` | +| `not-an-adf-document` | the value handed in is no ADF document — a missing or wrong `type`, a stray key, a node that is not a node, an object in the document holding itself, an accessor property, a Proxy that throws, an attribute value that is not a plain object, array or JSON primitive, such as a `Date` or `Map` | read the boundary you receive JSON at with `readAdfDocument`; for a node that is not a node, `path` leads to it, and the message names the branch that refused. Where your objects hold getters (Vue 2, MobX, a class model), hand in a snapshot, `JSON.parse(JSON.stringify(document))`. Build an attribute value as JSON holds it: `date.toISOString()`, `Object.fromEntries(map)`, `{ ...instance }` | | `unsupported-document-version` | the document's `version` is not 1 | keep the ADF and pass the document over, or show it read-only; the version is the site's, not yours to change | ### Either direction diff --git a/docs/decisions.md b/docs/decisions.md index e15aa94..62a0378 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -687,10 +687,12 @@ input reads `message`. must invert before it reads as a failure — and where the flavour's claim refuses ordinary prose it names the escape that unclaims the form claimed: `\!adf:` for a directive, block line and inline alike, `\|` for every pipe row. -- `not-an-adf-document` carries the document's own path throughout: seven of the guard's nine - branches read the document's own shape, and threading a path to the other two, which look anywhere - in the tree — a malformed node, or an object holding itself — wants the manual stack §Nothing - recurses unbounded forces. The message names the violation instead. +- `not-an-adf-document` names a node that is not an ADF node at that node's path, as + `unsupported-node-shape` does (2026-11-10, plan item 80). The checks of the document's own fields + and the copy `readAdfDocument` makes first, which refuses a getter, an object holding itself, a + throwing Proxy and a bad array length, stay at the empty path: the copy tracks no path, and + threading one through it wants the manual stack §Nothing recurses unbounded forces. The message + names the violation. ## Publish on a version bump diff --git a/src/adf/document.test.ts b/src/adf/document.test.ts index 2d3a816..db5ed2b 100644 --- a/src/adf/document.test.ts +++ b/src/adf/document.test.ts @@ -66,7 +66,7 @@ test('names the check anything that is not a doc node failed', () => { assert.equal(fault({ type: 'doc', version: Number.NaN }), 'an ADF document holds a version number: found NaN') }) -function unreadableNode(content: unknown[]): { message: string; path: unknown } { +function refusedNode(content: unknown[]): { message: string; path: unknown } { const read = readAdfDocument({ content, type: 'doc', version: 1 }) assert.equal(read.ok, false) if (read.ok) throw new Error('unreachable') @@ -75,26 +75,27 @@ function unreadableNode(content: unknown[]): { message: string; path: unknown } } test('names the path of the node that is no ADF node, and what is wrong with it', () => { - const date = unreadableNode([{ content: [{ text: 'x', type: 'text' }], type: 'paragraph' }, { content: [{ attrs: { when: new Date(0) }, type: 'panel' }], type: 'paragraph' }]) + const date = refusedNode([{ content: [{ text: 'x', type: 'text' }], type: 'paragraph' }, { content: [{ attrs: { when: new Date(0) }, type: 'panel' }], type: 'paragraph' }]) assert.deepEqual(date.path, ['content', 1, 'content', 0]) assert.match(date.message, /when attribute of panel/) - const topLevel = unreadableNode([{ type: 1 }]) + const topLevel = refusedNode([{ type: 1 }]) assert.deepEqual(topLevel.path, ['content', 0]) assert.match(topLevel.message, /type string: found 1/) - const extra = unreadableNode([{ type: 'paragraph' }, { node: 'x', type: 'paragraph' }]) + const extra = refusedNode([{ type: 'paragraph' }, { node: 'x', type: 'paragraph' }]) assert.deepEqual(extra.path, ['content', 1]) assert.match(extra.message, /key node/) - assert.match(unreadableNode([{ text: 1, type: 'text' }]).message, /text of text is a string: found 1/) - assert.match(unreadableNode([{ content: {}, type: 'paragraph' }]).message, /content of paragraph is an array: found an object/) - assert.match(unreadableNode([{ marks: [{ type: 1 }], text: 'x', type: 'text' }]).message, /mark of text/) - assert.match(unreadableNode([{ attrs: [], type: 'paragraph' }]).message, /attrs of paragraph is an object: found an array/) - assert.match(unreadableNode([{ attrs: new Map(), type: 'paragraph' }]).message, /attrs of paragraph is a plain object/) - assert.match(unreadableNode([5]).message, /node is an object: found 5/) + assert.match(refusedNode([{ text: 1, type: 'text' }]).message, /text of text is a string: found 1/) + assert.match(refusedNode([{ content: {}, type: 'paragraph' }]).message, /content of paragraph is an array: found an object/) + assert.match(refusedNode([{ marks: [{ type: 'em' }, { type: 1 }], text: 'x', type: 'text' }]).message, /mark at index 1 of text/) + assert.match(refusedNode([{ marks: {}, text: 'x', type: 'text' }]).message, /marks of text is an array: found an object/) + assert.match(refusedNode([{ attrs: [], type: 'paragraph' }]).message, /attrs of paragraph is an object: found an array/) + assert.match(refusedNode([{ attrs: new Map(), type: 'paragraph' }]).message, /attrs of paragraph is a plain object/) + assert.match(refusedNode([5]).message, /node is an object: found 5/) }) test('names the first unreadable node in document order', () => { - const first = unreadableNode([{ content: [{ type: 1 }], type: 'paragraph' }, { type: 2 }]) - assert.deepEqual(first.path, ['content', 0, 'content', 0]) + assert.deepEqual(refusedNode([{ content: [{ type: 1 }], type: 'paragraph' }, { type: 2 }]).path, ['content', 0, 'content', 0]) + assert.deepEqual(refusedNode([{ content: [{ type: 'text' }, { type: 1 }, { type: 2 }], type: 'paragraph' }]).path, ['content', 0, 'content', 1]) }) test('rejects a node whose shape ProseMirror JSON cannot hold', () => { diff --git a/src/adf/document.ts b/src/adf/document.ts index cc8f6d3..0af27ca 100644 --- a/src/adf/document.ts +++ b/src/adf/document.ts @@ -161,17 +161,26 @@ export function nodeMarks(node: { marks?: AdfMark[] }): readonly AdfMark[] { // Walks every node, carried or of an unknown type, so a shape is refused wherever it stands (docs/decisions.md §Schema refusals are made in the first step alone). export function refusedShape(content: readonly AdfNode[]): ConvertError | undefined { - type Step = { index: number; node: AdfNode; parent: Step | undefined } + const refused = firstRefusal(content, nodeContent, shapeRefusal) + return refused === undefined ? undefined : { code: 'unsupported-node-shape', message: refused.message, path: refused.path } +} + +// The first node in document order that `refusal` names, with the path to it; the walk keeps its own stack, so no depth overflows it. +function firstRefusal( + content: readonly Node[], + childrenOf: (node: Node) => readonly Node[], + refusal: (node: Node) => string | undefined, +): { message: string; path: ConvertErrorPath } | undefined { + type Step = { index: number; node: Node; parent: Step | undefined } const pending: Step[] = content.map((node, index) => ({ index, node, parent: undefined })).reverse() for (let step = pending.pop(); step !== undefined; step = pending.pop()) { - const message = shapeRefusal(step.node) + const message = refusal(step.node) if (message !== undefined) { const path: (number | string)[] = [] for (let up: Step | undefined = step; up !== undefined; up = up.parent) path.push(up.index, 'content') - path.reverse() - return { code: 'unsupported-node-shape', message, path } + return { message, path: path.reverse() } } - for (const child of nodeContent(step.node).map((node, index) => ({ index, node, parent: step })).reverse()) pending.push(child) + for (const child of childrenOf(step.node).map((node, index) => ({ index, node, parent: step })).reverse()) pending.push(child) } return undefined } @@ -193,49 +202,47 @@ function isNodeArray(value: readonly unknown[]): value is AdfNode[] { return unreadableNode(value) === undefined } -// The first node, in document order, that is no ADF node, with the path of the node that holds the fault. +// The first node in document order that is not an ADF node, and its path. function unreadableNode(content: readonly unknown[]): { fault: ConvertFault; path: ConvertErrorPath } | undefined { - type Step = { index: number; node: unknown; parent: Step | undefined } - const pending: Step[] = content.map((node, index) => ({ index, node, parent: undefined })).reverse() - for (let step = pending.pop(); step !== undefined; step = pending.pop()) { - const message = nodeFault(step.node) - if (message !== undefined) { - const path: (number | string)[] = [] - for (let up: Step | undefined = step; up !== undefined; up = up.parent) path.push(up.index, 'content') - return { fault: { code: 'not-an-adf-document', message }, path: path.reverse() } - } - if (!isRecord(step.node) || !Array.isArray(step.node['content'])) continue - const children: readonly unknown[] = step.node['content'] - for (const child of children.map((node, index) => ({ index, node, parent: step })).reverse()) pending.push(child) - } - return undefined + const refused = firstRefusal(content, unreadableChildren, nodeRefusal) + return refused === undefined ? undefined : { fault: { code: 'not-an-adf-document', message: refused.message }, path: refused.path } } -function nodeFault(node: unknown): string | undefined { +function unreadableChildren(node: unknown): readonly unknown[] { + const content: unknown = isRecord(node) ? node['content'] : undefined + return Array.isArray(content) ? content : [] +} + +function nodeRefusal(node: unknown): string | undefined { if (!isRecord(node)) return `an ADF node is an object: found ${describe(node)}` const extra = extraKey(node, nodeKeys) if (extra !== undefined) return `an ADF node holds attrs, content, marks, text and type alone: found the key ${extra}` const type: unknown = node['type'] if (typeof type !== 'string') return `an ADF node holds a type string: found ${'type' in node ? describe(type) : 'no type field'}` - if ('attrs' in node) { - const attrs: unknown = node['attrs'] - if (!isRecord(attrs)) return `the attrs of ${type} is an object: found ${describe(attrs)}` - if (!isJsonValue(attrs)) { - const key = Object.keys(attrs).find((name) => !isJsonValue(attrs[name])) - return key === undefined ? `the attrs of ${type} is a plain object: this one is not` : `the ${key} attribute of ${type} holds JSON alone: this one holds a value that is not JSON` - } - } + const attrs = 'attrs' in node ? attrsRefusal(node['attrs'], type) : undefined + if (attrs !== undefined) return attrs if ('marks' in node) { - const marks: unknown = node['marks'] - if (!Array.isArray(marks)) return `the marks of ${type} is an array: found ${describe(marks)}` - const unreadable = [...marks].some((mark) => !isAdfMark(mark)) - if (unreadable) return `a mark of ${type} is an object of a string type and attrs alone, the attrs JSON: this one is not` + const marks = marksRefusal(node['marks'], type) + if (marks !== undefined) return marks } if ('text' in node && typeof node['text'] !== 'string') return `the text of ${type} is a string: found ${describe(node['text'])}` if ('content' in node && !Array.isArray(node['content'])) return `the content of ${type} is an array: found ${describe(node['content'])}` return undefined } +function attrsRefusal(attrs: unknown, type: string): string | undefined { + if (!isRecord(attrs)) return `the attrs of ${type} is an object: found ${describe(attrs)}` + if (isJsonValue(attrs)) return undefined + const key = Object.keys(attrs).find((name) => !isJsonValue(attrs[name])) + return key === undefined ? `the attrs of ${type} is a plain object: this one is not` : `the ${key} attribute of ${type} holds JSON alone: this one holds a value that is not JSON` +} + +function marksRefusal(marks: unknown, type: string): string | undefined { + if (!Array.isArray(marks)) return `the marks of ${type} is an array: found ${describe(marks)}` + const index = [...marks].findIndex((mark) => !isAdfMark(mark)) + return index === -1 ? undefined : `the mark at index ${index} of ${type} is not a mark: a mark holds a string type and JSON attrs alone` +} + function marksKey(marks: readonly AdfMark[]): string { return serializeCanonicalJson( marks.map((mark) => (mark.attrs === undefined ? [mark.type] : [mark.type, mark.attrs])),