diff --git a/CHANGELOG.md b/CHANGELOG.md index 51c90f1..b2f0348 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## 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. - **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/docs/reading-cost.md b/docs/reading-cost.md index eb64a42..0d80800 100644 --- a/docs/reading-cost.md +++ b/docs/reading-cost.md @@ -997,3 +997,20 @@ Read most (opens and searches): - `corpus/README.md` 1 - `corpus/round-trip/block-nodes/expand.md` 1 - `corpus/round-trip/block-nodes/panel.md` 1 + +## Reading cost, chunk a9b6ac0f3bd0, finished 2026-11-10T20:45:20Z + +92724 new input tokens before the first commit. + +Read most (opens and searches): + +- `README.md` 10 +- `src/markdown/parse/blocks.ts` 8 +- `docs/decisions.md` 6 +- `MIGRATION.md` 5 +- `spec/flavour.md` 4 +- `src/markdown/parse/markdown-to-adf.test.ts` 3 +- `todo.md` 3 +- `CHANGELOG.md` 2 +- `src/markdown/emit/escaping.test.ts` 2 +- `src/markdown/pipe-table-syntax.ts` 2 diff --git a/src/adf/document.test.ts b/src/adf/document.test.ts index daa8741..2d3a816 100644 --- a/src/adf/document.test.ts +++ b/src/adf/document.test.ts @@ -66,9 +66,39 @@ 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 } { + const read = readAdfDocument({ content, type: 'doc', version: 1 }) + assert.equal(read.ok, false) + if (read.ok) throw new Error('unreachable') + assert.equal(read.error.code, 'not-an-adf-document') + return { message: read.error.message, path: read.error.path } +} + +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' }]) + assert.deepEqual(date.path, ['content', 1, 'content', 0]) + assert.match(date.message, /when attribute of panel/) + const topLevel = unreadableNode([{ 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' }]) + 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/) +}) + +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]) +}) + test('rejects a node whose shape ProseMirror JSON cannot hold', () => { assert.equal(fault({ content: {}, type: 'doc', version: 1 }), "an ADF document's content is an array: found an object") - assert.equal(fault({ content: [{ type: 1 }], type: 'doc', version: 1 }), "an ADF document's content holds ADF nodes: one of them is not") assert.equal(readable({ content: [{ type: 1 }], type: 'doc', version: 1 }), false) assert.equal(readable({ content: [{ text: 1, type: 'text' }], type: 'doc', version: 1 }), false) assert.equal(readable({ content: [{ node: 'x', type: 'paragraph' }], type: 'doc', version: 1 }), false) diff --git a/src/adf/document.ts b/src/adf/document.ts index b05d53c..cc8f6d3 100644 --- a/src/adf/document.ts +++ b/src/adf/document.ts @@ -1,5 +1,5 @@ import { blockNodeModel } from './block-nodes.ts' -import { failure, faulted, success, type ConvertError, type ConvertFault, type Read, type Result } from '../result.ts' +import { failure, faulted, success, type ConvertError, type ConvertErrorPath, type ConvertFault, type Result } from '../result.ts' import { inlineNodeModel } from './inline-nodes.ts' import { isJsonValue, overNested, type JsonValue } from '../json-value.ts' import { largestNesting } from '../nesting.ts' @@ -39,12 +39,11 @@ export function readAdfDocument(value: unknown): Result { return withinEngineLimits(() => { const copy = plainCopy(value) if (copy.fault !== undefined) return faulted(copy.fault, []) - const shaped = shapedDocument(copy.value) - return shaped.fault === undefined ? success(shaped.value) : faulted(shaped.fault, []) + return shapedDocument(copy.value) }) } -function shapedDocument(value: unknown): Read { +function shapedDocument(value: unknown): Result { if (!isRecord(value)) return notADocument(`an ADF document is an object: found ${describe(value)}`) const extra = extraKey(value, documentKeys) if (extra !== undefined) return notADocument(`an ADF document holds content, type and version alone: found the key ${extra}`) @@ -54,12 +53,13 @@ function shapedDocument(value: unknown): Read { if (!('version' in value)) return notADocument('an ADF document holds a version number: found no version field') const version: unknown = value['version'] if (typeof version !== 'number' || !Number.isFinite(version)) return notADocument(`an ADF document holds a version number: found ${describe(version)}`) - if (!('content' in value)) return { value: { type, version } } + if (!('content' in value)) return success({ type, version }) const content: unknown = value['content'] if (!Array.isArray(content)) return notADocument(`an ADF document's content is an array: found ${describe(content)}`) const nodes: readonly unknown[] = content - if (!isNodeArray(nodes)) return notADocument("an ADF document's content holds ADF nodes: one of them is not") - return { value: { content: nodes, type, version } } + if (isNodeArray(nodes)) return success({ content: nodes, type, version }) + const unreadable = unreadableNode(nodes) + return unreadable === undefined ? notADocument('an ADF document holds ADF nodes: one is not') : faulted(unreadable.fault, unreadable.path) } // The checks every writer opens with; `undefined` where the document holds no `content` key, which each format spells its own way. @@ -190,21 +190,50 @@ function shapeRefusal(node: AdfNode): string | undefined { } function isNodeArray(value: readonly unknown[]): value is 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) { - const content = node['content'] - if (!Array.isArray(content)) return false - for (const child of content) pending.push(child) + 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. +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 +} + +function nodeFault(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` } } - return true + 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` + } + 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 marksKey(marks: readonly AdfMark[]): string { @@ -241,10 +270,6 @@ function attributesFault(attrs: AdfAttributes, type: string): ConvertFault | und return undefined } -function isArrayOf(value: unknown, guard: (item: unknown) => item is T): value is T[] { - return Array.isArray(value) && [...value].every(guard) -} - function isAttributes(value: unknown): value is AdfAttributes { return isRecord(value) && isJsonValue(value) } @@ -253,8 +278,8 @@ export function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value) } -function notADocument(message: string): { fault: ConvertFault } { - return { fault: { code: 'not-an-adf-document', message } } +function notADocument(message: string): Result { + return failure('not-an-adf-document', message, []) } function describe(value: unknown): string { diff --git a/todo.md b/todo.md index 121a972..eaa7d45 100644 --- a/todo.md +++ b/todo.md @@ -12,7 +12,6 @@ | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| -| 80 | 0.4.0 | | | **Name the node's path and the attribute key when `not-an-adf-document` refuses a node inside the document.** | 2 | 1 | 2 | 3 | 3 | 6 by 4 | 6.0 | | 171 | 0.4.0 | | principle | **Assert the facts an error message must carry, never its whole sentence, in `markdown-to-adf.test.ts`, `document.test.ts` and `html-to-adf.test.ts`.** | 3 | 1 | 1 | 3 | 6 | code health by 3 | 4.4 | | 173 | 0.4.0 | | principle | **Move each CommonMark spelling's description out of `commonMarkSpellings`, beside the emitter's spelling it describes.** | 2 | 1 | 1 | 2 | 4 | code health by 1 | 4.4 | | 147 | 0.4.0 | | | **Ship `adfToPlainText`, writing a document's content with no markup, under a README goal for plain text.** | 2 | 2 | 1 | 3 | 4 | 3 by 4 | 4.2 | @@ -51,14 +50,6 @@ ## Details -### 80. Name the node's path and the attribute key when `not-an-adf-document` refuses a node inside the document. - -Today `readAdfDocument` and the converters refuse every node, such as one with a `Date` in an -attribute, with the message `an ADF document's content holds ADF nodes: one of them is not` and an -empty `path`. An export tool told this about one document in thousands then searches that -document's whole tree. Give the node's path, and name its type and the attribute key as -`attributeNestingMessage` does. Found by the product-owner review of items 66 and 67, 2026-10-05. - ### 171. Assert the facts an error message must carry, never its whole sentence, in `markdown-to-adf.test.ts`, `document.test.ts` and `html-to-adf.test.ts`. README → Audience says no persona may rely on a message's wording, yet `content()` in