CHANGELOG.md

@@ -31,6 +31,8 @@
   `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
   refused.
+- Fix a document holding itself, through a node's `content` or an attribute: `adfToMarkdown`,
+  `adfToPlainMarkdown` and `isAdfDocument` refuse it as `not-an-adf-document`, where they hung.
 
 ## 0.1.0
 

src/adf/document.test.ts

@@ -82,3 +82,23 @@ test("reads a node's siblings as a walk rather than as one call's arguments", ()
   assert.equal(isAdfDocument(wide), true)
   assert.equal(fault(wide), 'accepted')
 })
+
+test('refuses a document that holds itself, and accepts one sharing a node or a mark', () => {
+  const notANode = "an ADF document's content holds ADF nodes: one of them is not"
+  const paragraph: { content: unknown[]; type: string } = { content: [], type: 'paragraph' }
+  paragraph.content.push(paragraph)
+  assert.equal(fault({ content: [paragraph], type: 'doc', version: 1 }), notANode)
+  const document: { content: unknown[]; type: string; version: number } = { content: [], type: 'doc', version: 1 }
+  document.content.push(document)
+  assert.equal(fault(document), notANode)
+  const attrs: Record<string, unknown> = {}
+  attrs['a'] = [attrs]
+  assert.equal(fault({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), notANode)
+  assert.equal(fault({ content: [{ marks: [{ attrs, type: 'link' }], text: 'x', type: 'text' }], type: 'doc', version: 1 }), notANode)
+  assert.equal(isAdfDocument({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), false)
+  const bold = { type: 'strong' }
+  const text = { marks: [bold], text: 'x', type: 'text' }
+  const shared = { content: [text, text], type: 'paragraph' }
+  const values = { b: 1 }
+  assert.equal(fault({ content: [shared, shared, { attrs: { a: [values, values] }, type: 'paragraph' }], type: 'doc', version: 1 }), 'accepted')
+})

src/adf/document.ts

@@ -1,5 +1,5 @@
 import type { ConvertFault } from '../result.ts'
-import { isJsonValue, overNested, type JsonValue } from '../json-value.ts'
+import { everyInTree, isJsonValue, overNested, type JsonValue } from '../json-value.ts'
 import { largestNesting } from '../nesting.ts'
 import { serializeCanonicalJson } from '../canonical-json.ts'
 
@@ -123,21 +123,16 @@ export function nodeMarks(node: { marks?: AdfMark[] }): readonly AdfMark[] {
 }
 
 function isNodeArray(value: readonly unknown[]): value is readonly AdfNode[] {
-  const pending: unknown[] = [...value]
-  while (pending.length > 0) {
-    const node = pending.pop()
-    if (!isRecord(node) || !holdsOnly(node, nodeKeys)) return false
-    if (typeof node['type'] !== 'string') return false
-    if ('attrs' in node && !isAttributes(node['attrs'])) return false
-    if ('marks' in node && !isArrayOf(node['marks'], isAdfMark)) return false
-    if ('text' in node && typeof node['text'] !== 'string') return false
-    if ('content' in node) {
+  return everyInTree(value, (node) => {
+    if (!isRecord(node) || !holdsOnly(node, nodeKeys)) return undefined
+    if (typeof node['type'] !== 'string') return undefined
+    if ('attrs' in node && !isAttributes(node['attrs'])) return undefined
+    if ('marks' in node && !isArrayOf(node['marks'], isAdfMark)) return undefined
+    if ('text' in node && typeof node['text'] !== 'string') return undefined
+    if (!('content' in node)) return []
     const content = node['content']
-      if (!Array.isArray(content)) return false
-      for (const child of content) pending.push(child)
-    }
-  }
-  return true
+    return Array.isArray(content) ? [...content] : undefined
+  })
 }
 
 function marksKey(marks: readonly AdfMark[]): string {

src/json-value.ts

@@ -2,23 +2,36 @@ import { largestNesting } from './nesting.ts'
 
 export type JsonValue = JsonValue[] | boolean | null | number | string | { [key: string]: JsonValue }
 
-export function isJsonValue(value: unknown): value is JsonValue {
-  const pending: unknown[] = [value]
+// Tracks ancestors alone: a tree may share an object, but one holding itself would walk forever.
+export function everyInTree(roots: readonly unknown[], children: (item: unknown) => readonly unknown[] | undefined): boolean {
+  const pending = Array.from(roots, (item) => ({ depth: 0, item }))
+  const ancestors: unknown[] = []
+  const onPath = new Set<unknown>()
   while (pending.length > 0) {
-    const item = pending.pop()
-    if (item === null || typeof item === 'boolean' || typeof item === 'string') continue
-    if (typeof item === 'number') {
-      if (!Number.isFinite(item)) return false
-      continue
-    }
-    // A hole is not a JSON value, and Array.prototype methods skip holes — spreading materialises them.
-    if (Array.isArray(item)) for (const child of [...item]) pending.push(child)
-    else if (typeof item === 'object') for (const child of Object.values(item)) pending.push(child)
-    else return false
+    const entry = pending.pop()
+    if (entry === undefined) continue
+    while (ancestors.length > entry.depth) onPath.delete(ancestors.pop())
+    if (onPath.has(entry.item)) return false
+    const held = children(entry.item)
+    if (held === undefined) return false
+    if (held.length === 0) continue
+    ancestors.push(entry.item)
+    onPath.add(entry.item)
+    for (const child of held) pending.push({ depth: entry.depth + 1, item: child })
   }
   return true
 }
 
+export function isJsonValue(value: unknown): value is JsonValue {
+  return everyInTree([value], (item) => {
+    if (item === null || typeof item === 'boolean' || typeof item === 'string') return []
+    if (typeof item === 'number') return Number.isFinite(item) ? [] : undefined
+    // A hole is not a JSON value, and Array.prototype methods skip holes — spreading materialises them.
+    if (Array.isArray(item)) return [...item]
+    return typeof item === 'object' ? Object.values(item) : undefined
+  })
+}
+
 export function nestingDepth(value: unknown): number {
   const pending: { depth: number; item: unknown }[] = [{ depth: 0, item: value }]
   let deepest = 0

todo.md

@@ -24,7 +24,6 @@
 
 | ID | Release | Exempt | Item | R | S | A | G | Goals | Score |
 |---|---|---|---|---|---|---|---|---|---|
-| 63 | 0.2.0 | defect | **Refuse a cyclic input as `not-an-adf-document`.** | 2 | 2 | 6 | 9 | 1 | 27.5 |
 | 7 | 0.2.0 |  | **Ship HTML: `adfToHtml`, `htmlToAdf`, and `markdownToHtml` / `htmlToMarkdown` composed through ADF.** | 6 | 9 | 9 | 9 | 2, 3 | 25.8 |
 | 55 | 0.2.0 | defect | **Spell every document the emitter refuses today for a carriage return, a NUL, a code-span line start, a newline inside an emoji, mention or status, or a text node whose text is empty or missing, or which holds content.** | 5 | 5 | 7 | 9 | 1 | 25.8 |
 | 45 | 0.2.0 |  | **Replace `isAdfDocument` with a reader returning `Result<AdfDocument>`.** | 2 | 3 | 6 | 8 | 1 | 25.2 |
@@ -57,13 +56,6 @@
 
 ## Details
 
-### 63. Refuse a cyclic input as `not-an-adf-document`.
-
-`adfDocumentFault` walks with `isNodeArray` (`adf/document.ts`) and `isJsonValue` (`json-value.ts`),
-worklists that record no visited object, so a node whose `content` holds itself hangs
-`adfToMarkdown`, `adfToPlainMarkdown` and `isAdfDocument`; README §The guarantees promises no input
-loops forever.
-
 ### 7. Ship HTML: `adfToHtml`, `htmlToAdf`, and `markdownToHtml` / `htmlToMarkdown` composed through ADF.
 
 Lands after items 6, 59, 60, 61 and 62. The CommonMark spec suite also runs against