3
0
mirror of https://github.com/larvit/adf-codec.git synced 2026-11-12 11:48:29 +01:00

Name the node's path and the fault when not-an-adf-document refuses a node inside the document

This commit is contained in:
2026-11-10 21:47:18 +01:00
committed by Lillem4n
parent 328db9dd4a
commit 297ff0e7e3
5 changed files with 102 additions and 36 deletions
+3
View File
@@ -2,6 +2,9 @@
## Unreleased ## 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 - **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`; 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`. a table whose padding would add more cells than the table holds is `malformed-pipe-table`.
+17
View File
@@ -997,3 +997,20 @@ Read most (opens and searches):
- `corpus/README.md` 1 - `corpus/README.md` 1
- `corpus/round-trip/block-nodes/expand.md` 1 - `corpus/round-trip/block-nodes/expand.md` 1
- `corpus/round-trip/block-nodes/panel.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
+31 -1
View File
@@ -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') 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', () => { 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: '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: [{ type: 1 }], type: 'doc', version: 1 }), false)
assert.equal(readable({ content: [{ text: 1, type: 'text' }], 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) assert.equal(readable({ content: [{ node: 'x', type: 'paragraph' }], type: 'doc', version: 1 }), false)
+51 -26
View File
@@ -1,5 +1,5 @@
import { blockNodeModel } from './block-nodes.ts' 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 { inlineNodeModel } from './inline-nodes.ts'
import { isJsonValue, overNested, type JsonValue } from '../json-value.ts' import { isJsonValue, overNested, type JsonValue } from '../json-value.ts'
import { largestNesting } from '../nesting.ts' import { largestNesting } from '../nesting.ts'
@@ -39,12 +39,11 @@ export function readAdfDocument(value: unknown): Result<AdfDocument> {
return withinEngineLimits(() => { return withinEngineLimits(() => {
const copy = plainCopy(value) const copy = plainCopy(value)
if (copy.fault !== undefined) return faulted(copy.fault, []) if (copy.fault !== undefined) return faulted(copy.fault, [])
const shaped = shapedDocument(copy.value) return shapedDocument(copy.value)
return shaped.fault === undefined ? success(shaped.value) : faulted(shaped.fault, [])
}) })
} }
function shapedDocument(value: unknown): Read<AdfDocument> { function shapedDocument(value: unknown): Result<AdfDocument> {
if (!isRecord(value)) return notADocument(`an ADF document is an object: found ${describe(value)}`) if (!isRecord(value)) return notADocument(`an ADF document is an object: found ${describe(value)}`)
const extra = extraKey(value, documentKeys) const extra = extraKey(value, documentKeys)
if (extra !== undefined) return notADocument(`an ADF document holds content, type and version alone: found the key ${extra}`) 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<AdfDocument> {
if (!('version' in value)) return notADocument('an ADF document holds a version number: found no version field') if (!('version' in value)) return notADocument('an ADF document holds a version number: found no version field')
const version: unknown = value['version'] 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 (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'] const content: unknown = value['content']
if (!Array.isArray(content)) return notADocument(`an ADF document's content is an array: found ${describe(content)}`) if (!Array.isArray(content)) return notADocument(`an ADF document's content is an array: found ${describe(content)}`)
const nodes: readonly unknown[] = content const nodes: readonly unknown[] = content
if (!isNodeArray(nodes)) return notADocument("an ADF document's content holds ADF nodes: one of them is not") if (isNodeArray(nodes)) return success({ content: nodes, type, version })
return { value: { 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. // 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[] { function isNodeArray(value: readonly unknown[]): value is AdfNode[] {
const pending: unknown[] = [...value] return unreadableNode(value) === undefined
while (pending.length > 0) { }
const node = pending.pop()
if (!isRecord(node) || !holdsOnly(node, nodeKeys)) return false // The first node, in document order, that is no ADF node, with the path of the node that holds the fault.
if (typeof node['type'] !== 'string') return false function unreadableNode(content: readonly unknown[]): { fault: ConvertFault; path: ConvertErrorPath } | undefined {
if ('attrs' in node && !isAttributes(node['attrs'])) return false type Step = { index: number; node: unknown; parent: Step | undefined }
if ('marks' in node && !isArrayOf(node['marks'], isAdfMark)) return false const pending: Step[] = content.map((node, index) => ({ index, node, parent: undefined })).reverse()
if ('text' in node && typeof node['text'] !== 'string') return false for (let step = pending.pop(); step !== undefined; step = pending.pop()) {
if ('content' in node) { const message = nodeFault(step.node)
const content = node['content'] if (message !== undefined) {
if (!Array.isArray(content)) return false const path: (number | string)[] = []
for (const child of content) pending.push(child) 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 { function marksKey(marks: readonly AdfMark[]): string {
@@ -241,10 +270,6 @@ function attributesFault(attrs: AdfAttributes, type: string): ConvertFault | und
return undefined return undefined
} }
function isArrayOf<T>(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 { function isAttributes(value: unknown): value is AdfAttributes {
return isRecord(value) && isJsonValue(value) return isRecord(value) && isJsonValue(value)
} }
@@ -253,8 +278,8 @@ export function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value) return typeof value === 'object' && value !== null && !Array.isArray(value)
} }
function notADocument(message: string): { fault: ConvertFault } { function notADocument(message: string): Result<never> {
return { fault: { code: 'not-an-adf-document', message } } return failure('not-an-adf-document', message, [])
} }
function describe(value: unknown): string { function describe(value: unknown): string {
-9
View File
@@ -12,7 +12,6 @@
| ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | | 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 | | 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 | | 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 | | 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 ## 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`. ### 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 README → Audience says no persona may rely on a message's wording, yet `content()` in