Answer prose round 3: the ownsWhatItHolds comment names any other key

CHANGELOG.md

@@ -31,8 +31,10 @@
   `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.
+- Fix a cyclic document, in which an object holds itself: `adfToMarkdown` and `adfToPlainMarkdown`
+  refuse it as `not-an-adf-document` and `isAdfDocument` returns `false`, where all three hung. A
+  node or mark whose `attrs`, `content`, `marks`, `text` or `type` is not its own enumerable key is
+  refused alike, where it was read.
 
 ## 0.1.0
 

README.md

@@ -185,7 +185,7 @@ Emitting — `adfToMarkdown`, and `adfToHtml` at `0.2.0`:
 
 | 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, an object in the document 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 |
 
 Either direction. The emitter's own refusals are in this group — a parse reaches them by asking it

docs/decisions.md

@@ -384,10 +384,10 @@ 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 eight
-  branches read the document's own shape, and threading a path to the eighth — a malformed node
-  anywhere in the tree — wants the manual stack §Nothing recurses unbounded forces. The message
-  names the violation instead.
+- `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.
 
 ## Publish on a version bump
 

src/adf/document.test.ts

@@ -83,19 +83,39 @@ test("reads a node's siblings as a walk rather than as one call's arguments", ()
   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"
+test('refuses an object holding itself or a key it does not own, and accepts one shared', () => {
+  const holdsItself = 'an ADF document is a tree: an object in it holds itself'
   const paragraph: { content: unknown[]; type: string } = { content: [], type: '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 }
   document.content.push(document)
-  assert.equal(fault(document), notANode)
+  assert.equal(fault(document), holdsItself)
   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(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 }), holdsItself)
   assert.equal(isAdfDocument({ content: [{ attrs, type: 'paragraph' }], type: 'doc', version: 1 }), false)
+  const notANode = "an ADF document's content holds ADF nodes: one of them is not"
+  const inherits: { content?: unknown[] } = {}
+  const heir: Record<string, unknown> = Object.create(inherits)
+  heir['type'] = 'paragraph'
+  inherits.content = [heir]
+  assert.equal(fault({ content: [heir], type: 'doc', version: 1 }), notANode)
+  const hidden: Record<string, unknown> = { type: 'paragraph' }
+  Object.defineProperty(hidden, 'content', { value: [hidden] })
+  assert.equal(fault({ content: [hidden], type: 'doc', version: 1 }), notANode)
+  const link: Record<string, unknown> = Object.create({ attrs })
+  link['type'] = 'link'
+  assert.equal(fault({ content: [{ marks: [link], text: 'x', type: 'text' }], type: 'doc', version: 1 }), notANode)
+  const indexed: unknown[] = []
+  Object.defineProperty(indexed, 0, { value: indexed })
+  assert.equal(fault({ content: [{ attrs: { a: indexed }, type: 'paragraph' }], type: 'doc', version: 1 }), holdsItself)
+  const inheritedIndex: { content?: unknown[]; type: string } = { type: 'paragraph' }
+  const heirs: unknown[] = Object.setPrototypeOf([], [inheritedIndex])
+  heirs.length = 1
+  inheritedIndex.content = heirs
+  assert.equal(fault({ content: [inheritedIndex], type: 'doc', version: 1 }), holdsItself)
   const bold = { type: 'strong' }
   const text = { marks: [bold], text: 'x', type: 'text' }
   const shared = { content: [text, text], type: 'paragraph' }

src/adf/document.ts

@@ -1,5 +1,5 @@
 import type { ConvertFault } from '../result.ts'
-import { everyInTree, isJsonValue, overNested, type JsonValue } from '../json-value.ts'
+import { isJsonValue, overNested, type JsonValue } from '../json-value.ts'
 import { largestNesting } from '../nesting.ts'
 import { serializeCanonicalJson } from '../canonical-json.ts'
 
@@ -43,6 +43,7 @@ export function adfDocumentFault(value: unknown): ConvertFault | undefined {
   const held: unknown = value['content']
   if (!Array.isArray(held)) return notADocument(`an ADF document's content is an array: found ${describe(held)}`)
   const content: readonly unknown[] = held
+  if (holdsItself(content)) return notADocument('an ADF document is a tree: an object in it holds itself')
   if (!isNodeArray(content)) return notADocument("an ADF document's content holds ADF nodes: one of them is not")
   return nestingFault(content)
 }
@@ -105,7 +106,7 @@ export function isAdfNode(value: unknown): value is AdfNode {
 }
 
 export function isAdfMark(value: unknown): value is AdfMark {
-  if (!isRecord(value) || !holdsOnly(value, markKeys)) return false
+  if (!isRecord(value) || !holdsOnly(value, markKeys) || !ownsWhatItHolds(value, markKeys)) return false
   if (typeof value['type'] !== 'string') return false
   return !('attrs' in value) || isAttributes(value['attrs'])
 }
@@ -122,17 +123,39 @@ export function nodeMarks(node: { marks?: AdfMark[] }): readonly AdfMark[] {
   return node.marks ?? []
 }
 
+// Tracks 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<unknown>()
+  while (pending.length > 0) {
+    const entry = pending.pop()
+    if (entry === undefined) continue
+    while (ancestors.length > entry.depth) onPath.delete(ancestors.pop())
+    if (onPath.has(entry.item)) return true
+    ancestors.push(entry.item)
+    onPath.add(entry.item)
+    for (const child of Array.isArray(entry.item) ? [...entry.item] : 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[] {
-  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']
-    return Array.isArray(content) ? [...content] : undefined
-  })
+  const pending: unknown[] = [...value]
+  while (pending.length > 0) {
+    const node = pending.pop()
+    if (!isRecord(node) || !holdsOnly(node, nodeKeys) || !ownsWhatItHolds(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) {
+      const content = node['content']
+      if (!Array.isArray(content)) return false
+      for (const child of content) pending.push(child)
+    }
+  }
+  return true
 }
 
 function marksKey(marks: readonly AdfMark[]): string {
@@ -196,6 +219,11 @@ function extraKey(value: Record<string, unknown>, keys: readonly string[]): stri
   return Object.keys(value).find((key) => !keys.includes(key))
 }
 
+// holdsItself walks own enumerable keys alone, so a guard reading any other key could walk a cycle forever.
+function ownsWhatItHolds(value: Record<string, unknown>, keys: readonly string[]): boolean {
+  return keys.every((key) => !(key in value) || Object.prototype.propertyIsEnumerable.call(value, key))
+}
+
 function holdsOnly(value: Record<string, unknown>, keys: readonly string[]): boolean {
   return extraKey(value, keys) === undefined
 }

src/json-value.ts

@@ -2,36 +2,23 @@ import { largestNesting } from './nesting.ts'
 
 export type JsonValue = JsonValue[] | boolean | null | number | string | { [key: string]: JsonValue }
 
-// 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>()
+export function isJsonValue(value: unknown): value is JsonValue {
+  const pending: unknown[] = [value]
   while (pending.length > 0) {
-    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 })
+    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
   }
   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