Answer product-owner round 1: name the cycle in its own message and in the error table

This commit is contained in:
2026-10-04 17:03:21 +02:00
parent 438bf8876b
commit addef08fa4
5 changed files with 31 additions and 12 deletions
+3 -2
View File
@@ -31,8 +31,9 @@
`collection`, `id` and `occurrenceKey` directly where they rode the opaque carry. `collection`, `id` and `occurrenceKey` directly where they rode the opaque carry.
- Fix an image inside another image's description: it flattens into the alt text, where it was - Fix an image inside another image's description: it flattens into the alt text, where it was
refused. refused.
- Fix a document holding itself, through a node's `content` or an attribute: `adfToMarkdown`, - Fix a cyclic document, in which a node or an attribute value holds itself: `adfToMarkdown` and
`adfToPlainMarkdown` and `isAdfDocument` refuse it as `not-an-adf-document`, where they hung. `adfToPlainMarkdown` refuse it as `not-an-adf-document` and `isAdfDocument` returns `false`,
where all three hung.
## 0.1.0 ## 0.1.0
+1 -1
View File
@@ -185,7 +185,7 @@ Emitting — `adfToMarkdown`, and `adfToHtml` at `0.2.0`:
| Code | Fires when | What you can do | | 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 | guard the boundary you receive JSON at with `isAdfDocument`; the message names the branch that refused | | `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, a node or attribute value holding itself | guard the boundary you receive JSON at with `isAdfDocument`; the message names the branch that refused |
| `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 | | `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. The emitter's own refusals are in this group — a parse reaches them by asking it Either direction. The emitter's own refusals are in this group — a parse reaches them by asking it
+3 -3
View File
@@ -384,9 +384,9 @@ input reads `message`.
must invert before it reads as a failure — and where the flavour's claim refuses ordinary prose 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 it names the escape that unclaims the form claimed: `\!adf:` for a directive, block line and
inline alike, `\|` for every pipe row. inline alike, `\|` for every pipe row.
- `not-an-adf-document` carries the document's own path throughout: seven of the guard's eight - `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 eighth — a malformed node branches read the document's own shape, and threading a path to the other two — a malformed node,
anywhere in the tree — wants the manual stack §Nothing recurses unbounded forces. The message or an object holding itself, anywhere in the tree — wants the manual stack §Nothing recurses unbounded forces. The message
names the violation instead. names the violation instead.
## Publish on a version bump ## Publish on a version bump
+6 -6
View File
@@ -83,18 +83,18 @@ test("reads a node's siblings as a walk rather than as one call's arguments", ()
assert.equal(fault(wide), 'accepted') assert.equal(fault(wide), 'accepted')
}) })
test('refuses a document that holds itself, and accepts one sharing a node or a mark', () => { test('refuses a node or an attribute value holding itself, and accepts one shared', () => {
const notANode = "an ADF document's content holds ADF nodes: one of them is not" const holdsItself = 'an ADF document is a tree: a node or an attribute value holds itself'
const paragraph: { content: unknown[]; type: string } = { content: [], type: 'paragraph' } const paragraph: { content: unknown[]; type: string } = { content: [], type: 'paragraph' }
paragraph.content.push(paragraph) paragraph.content.push(paragraph)
assert.equal(fault({ content: [paragraph], type: 'doc', version: 1 }), notANode) assert.equal(fault({ content: [paragraph], type: 'doc', version: 1 }), holdsItself)
const document: { content: unknown[]; type: string; version: number } = { content: [], type: 'doc', version: 1 } const document: { content: unknown[]; type: string; version: number } = { content: [], type: 'doc', version: 1 }
document.content.push(document) document.content.push(document)
assert.equal(fault(document), notANode) assert.equal(fault(document), holdsItself)
const attrs: Record<string, unknown> = {} const attrs: Record<string, unknown> = {}
attrs['a'] = [attrs] attrs['a'] = [attrs]
assert.equal(fault({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), notANode) assert.equal(fault({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), holdsItself)
assert.equal(fault({ content: [{ marks: [{ attrs, type: 'link' }], text: 'x', type: 'text' }], type: 'doc', version: 1 }), notANode) assert.equal(fault({ content: [{ marks: [{ attrs, type: 'link' }], text: 'x', type: 'text' }], type: 'doc', version: 1 }), holdsItself)
assert.equal(isAdfDocument({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), false) assert.equal(isAdfDocument({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), false)
const bold = { type: 'strong' } const bold = { type: 'strong' }
const text = { marks: [bold], text: 'x', type: 'text' } const text = { marks: [bold], text: 'x', type: 'text' }
+18
View File
@@ -43,6 +43,7 @@ export function adfDocumentFault(value: unknown): ConvertFault | undefined {
const held: unknown = value['content'] const held: unknown = value['content']
if (!Array.isArray(held)) return notADocument(`an ADF document's content is an array: found ${describe(held)}`) if (!Array.isArray(held)) return notADocument(`an ADF document's content is an array: found ${describe(held)}`)
const content: readonly unknown[] = held const content: readonly unknown[] = held
if (holdsItself(content)) return notADocument('an ADF document is a tree: a node or an attribute value holds itself')
if (!isNodeArray(content)) return notADocument("an ADF document's content holds ADF nodes: one of them is not") if (!isNodeArray(content)) return notADocument("an ADF document's content holds ADF nodes: one of them is not")
return nestingFault(content) return nestingFault(content)
} }
@@ -122,6 +123,23 @@ export function nodeMarks(node: { marks?: AdfMark[] }): readonly AdfMark[] {
return node.marks ?? [] return node.marks ?? []
} }
// Ancestors alone: a tree may share an object, but an object holding itself walks the guards forever.
function holdsItself(root: object): boolean {
const pending = [{ depth: 0, item: root }]
const ancestors: object[] = []
const onPath = new Set<object>()
while (pending.length > 0) {
const entry = pending.pop()
if (entry === undefined) continue
for (const left of ancestors.splice(entry.depth)) onPath.delete(left)
if (onPath.has(entry.item)) return true
ancestors.push(entry.item)
onPath.add(entry.item)
for (const child of Object.values(entry.item)) if (typeof child === 'object' && child !== null) pending.push({ depth: entry.depth + 1, item: child })
}
return false
}
function isNodeArray(value: readonly unknown[]): value is readonly AdfNode[] { function isNodeArray(value: readonly unknown[]): value is readonly AdfNode[] {
const pending: unknown[] = [...value] const pending: unknown[] = [...value]
while (pending.length > 0) { while (pending.length > 0) {