diff --git a/packages/markdown-codec/src/ast/ast.test.ts b/packages/markdown-codec/src/ast/ast.test.ts new file mode 100644 index 000000000..ce0fb5a77 --- /dev/null +++ b/packages/markdown-codec/src/ast/ast.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { isMarkdownBlockNode, isMarkdownInlineNode } from "./ast"; +import type { MarkdownNode } from "./ast"; + +const blockNode: MarkdownNode = { type: "paragraph", children: [] }; +const inlineNode: MarkdownNode = { type: "text", value: "hi" }; + +describe("isMarkdownBlockNode / isMarkdownInlineNode", () => { + it("classifies a block node as a block and not inline", () => { + expect(isMarkdownBlockNode(blockNode)).toBe(true); + expect(isMarkdownInlineNode(blockNode)).toBe(false); + }); + + it("classifies an inline node as inline and not a block", () => { + expect(isMarkdownBlockNode(inlineNode)).toBe(false); + expect(isMarkdownInlineNode(inlineNode)).toBe(true); + }); + + it("recognises every real block node type named in the table, not just one representative", () => { + const types: MarkdownNode["type"][] = [ + "document", + "paragraph", + "heading", + "blockquote", + "list", + "listItem", + "codeBlock", + "thematicBreak", + "htmlBlock", + "table", + "tableRow", + "tableCell", + "mathBlock", + "footnoteDefinition", + ]; + for (const type of types) { + // isMarkdownBlockNode reads only `.type`, so a bare-type fixture is a faithful runtime input; the cast is unavoidable since a real MarkdownNode variant also carries fields (children, value, ...) this loop has no reason to construct per type. + expect(isMarkdownBlockNode({ type } as MarkdownNode)).toBe(true); + } + }); +}); diff --git a/packages/markdown-codec/src/block/block.test.ts b/packages/markdown-codec/src/block/block.test.ts index 954085014..e1a92452d 100644 --- a/packages/markdown-codec/src/block/block.test.ts +++ b/packages/markdown-codec/src/block/block.test.ts @@ -444,6 +444,16 @@ describe("recover-tier diagnostics", () => { ).toBe(true); }); + it("reports each duplicate definition's own line, counted from how many newlines precede it within the paragraph", () => { + const collector = createDiagnosticCollector(); + parseMarkdown("[a]: /1\n[a]: /2\n[a]: /3", { sink: collector.sink }); + const duplicates = collector.diagnostics.filter( + (diagnostic) => + diagnostic.code === MarkdownDiagnosticCodes.DUPLICATE_LINK_REFERENCE, + ); + expect(duplicates.map((diagnostic) => diagnostic.line)).toEqual([2, 3]); + }); + it("reports a math block never closed by a matching $$ before end-of-input", () => { const collector = createDiagnosticCollector(); parseMarkdown("$$\nx^2", { sink: collector.sink }); diff --git a/packages/markdown-codec/src/block/definitions.test.ts b/packages/markdown-codec/src/block/definitions.test.ts new file mode 100644 index 000000000..6012623e4 --- /dev/null +++ b/packages/markdown-codec/src/block/definitions.test.ts @@ -0,0 +1,46 @@ +// Direct tests for extractDefinitions -- the higher-level parseMarkdown suite (src/block/block.test.ts) exercises this through whole documents, which never isolates the exact cursor arithmetic that decides where one definition ends and the residual paragraph content begins. + +import { describe, expect, it } from "vitest"; +import { extractDefinitions } from "./definitions"; +import type { LinkReferenceDefinition } from "../inline/link"; + +describe("extractDefinitions", () => { + it("leaves ordinary text on a following line as the residual paragraph content", () => { + const references = new Map(); + const rest = extractDefinitions("[a]: /url\nsome text", references); + expect(rest).toBe("some text"); + expect(references.get("A")).toEqual({ destination: "/url" }); + }); + + it("ends the definition at the real line's own newline, not merely one past where the destination itself finished, when trailing spaces sit between them", () => { + const references = new Map(); + const rest = extractDefinitions("[a]: /url \nsome text", references); + expect(rest).toBe("some text"); + }); + + it("consumes a definition with no trailing newline entirely, leaving nothing behind", () => { + const references = new Map(); + const rest = extractDefinitions("[a]: /url", references); + expect(rest).toBe(""); + expect(references.get("A")).toEqual({ destination: "/url" }); + }); + + it("does not treat a label with only whitespace between its brackets as a definition at all", () => { + const references = new Map(); + const rest = extractDefinitions("[ ]: /url\nrest", references); + expect(rest).toBe("[ ]: /url\nrest"); + expect(references.size).toBe(0); + }); + + it("reports the exact duplicate-definition message, naming the losing label", () => { + const messages: string[] = []; + extractDefinitions( + "[a]: /1\n[a]: /2", + new Map(), + (diagnostic) => messages.push(diagnostic.message), + ); + expect(messages).toEqual([ + 'link reference definition "A" was already defined earlier in the document; this later definition is ignored', + ]); + }); +}); diff --git a/packages/markdown-codec/src/block/definitions.ts b/packages/markdown-codec/src/block/definitions.ts index 08b40f663..18e9e6715 100644 --- a/packages/markdown-codec/src/block/definitions.ts +++ b/packages/markdown-codec/src/block/definitions.ts @@ -19,9 +19,6 @@ import { skipInlineWhitespace, } from "../inline/link"; -// A definition needs a label with at least one non-whitespace character between its brackets, so the shortest possible match is `[x]` -- three characters. -const MIN_DEFINITION_LABEL_LENGTH = 3; - interface ParsedDefinition { readonly label: string; readonly definition: LinkReferenceDefinition; @@ -32,10 +29,8 @@ function parseDefinition( content: string, start: number, ): ParsedDefinition | undefined { + // No separate "is the label at least [x] long" length guard: matchLinkLabel returns 0 (no bracket at all) or a real bracket-pair length of 2 or more, and a length-2 match ("[]") slices to an empty inner label just as a length-0 match's own empty slice does -- both already fall out of the label.length === 0 check below, so a dedicated minimum-length rejection could never see a case the empty-label check doesn't already reject. const labelLength = matchLinkLabel(content, start); - if (labelLength < MIN_DEFINITION_LABEL_LENGTH) { - return undefined; - } const label = normalizeLinkLabel(content.slice(start, start + labelLength)); if (label.length === 0) { return undefined; @@ -109,11 +104,5 @@ export function extractDefinitions( } function countNewlines(content: string, upTo: number): number { - let count = 0; - for (let index = 0; index < upTo && index < content.length; index += 1) { - if (content.charAt(index) === "\n") { - count += 1; - } - } - return count; + return content.slice(0, upTo).split("\n").length - 1; } diff --git a/packages/markdown-codec/src/block/line.test.ts b/packages/markdown-codec/src/block/line.test.ts new file mode 100644 index 000000000..e9be8b687 --- /dev/null +++ b/packages/markdown-codec/src/block/line.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { LineCursor } from "./line"; + +describe("LineCursor", () => { + it("reports an empty line as blank as soon as it is constructed", () => { + expect(new LineCursor("").blank).toBe(true); + }); + + it("reports a non-empty line as not blank", () => { + expect(new LineCursor("foo").blank).toBe(false); + }); + + it("reports a whitespace-only line as blank", () => { + expect(new LineCursor(" ").blank).toBe(true); + }); +}); diff --git a/packages/markdown-codec/src/block/line.ts b/packages/markdown-codec/src/block/line.ts index 7540fd708..ff0e4434b 100644 --- a/packages/markdown-codec/src/block/line.ts +++ b/packages/markdown-codec/src/block/line.ts @@ -16,7 +16,8 @@ export class LineCursor { private readonly cursor: MarkdownScanCursor; private nextNonspaceMark: MarkdownScanMark; private nextNonspaceColumn = 0; - private lineIsBlank = false; + // No default value: the constructor unconditionally calls findNextNonspace() below, which always assigns this before any getter can read it, so a placeholder default would be overwritten on every construction path and could never be observed to differ. + private lineIsBlank!: boolean; constructor(text: string) { this.text = text; @@ -78,10 +79,9 @@ export class LineCursor { // Advances up to `columns` columns, stopping at end of line. A tab straddling the target is consumed only as far as needed, leaving its remaining columns for rest() to materialise -- which is exactly how `>\tfoo` puts three columns of indentation, not a whole tab, into the block quote's content. advance(columns: number): void { + // No early exit at end of line: MarkdownScanCursor.next() is already a side-effect-free no-op once rawOffset reaches the source length (src/scan/scan.ts), so looping the remaining count down regardless produces the identical end state as returning early -- an early-return branch here would be unobservable by any test, on purpose or not. for (let remaining = columns; remaining > 0; remaining -= 1) { - if (this.cursor.next() === undefined) { - return; - } + this.cursor.next(); } } diff --git a/packages/markdown-codec/src/block/list.test.ts b/packages/markdown-codec/src/block/list.test.ts new file mode 100644 index 000000000..e943dba8c --- /dev/null +++ b/packages/markdown-codec/src/block/list.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from "vitest"; +import { finalizeListTightness, listsMatch } from "./list"; +import { BlockNode } from "./node"; +import type { ListMarkerData } from "./node"; + +const bullet = (bulletChar: "-" | "*" | "+"): ListMarkerData => ({ + type: "bullet", + bulletChar, + padding: 2, + markerOffset: 0, +}); + +const ordered = (delimiter: "." | ")"): ListMarkerData => ({ + type: "ordered", + delimiter, + padding: 3, + markerOffset: 0, +}); + +describe("listsMatch", () => { + it("matches two bullet markers with the same bullet character", () => { + expect(listsMatch(bullet("-"), bullet("-"))).toBe(true); + }); + + it("never matches a bullet marker against an ordered one, even if every other field happened to line up", () => { + expect(listsMatch(bullet("-"), ordered("."))).toBe(false); + }); + + it("does not match two ordered markers with different delimiters", () => { + expect(listsMatch(ordered("."), ordered(")"))).toBe(false); + }); + + it("does not match two bullet markers with different bullet characters", () => { + expect(listsMatch(bullet("-"), bullet("*"))).toBe(false); + }); +}); + +describe("finalizeListTightness's own lastLineChecked memoisation", () => { + it("marks a descended list/listItem node's own lastLineChecked, so a later finalisation over the same chain does not re-walk it", () => { + const list = new BlockNode("list", 1); + // item1 is the one endsWithBlankLine is actually called on: finalizeListTightness only checks an item that has a FOLLOWING sibling (item1, since item2 follows it), never the last item in the list on its own account. + const item1 = new BlockNode("listItem", 1); + const leaf = new BlockNode("paragraph", 1); + item1.appendChild(leaf); + const item2 = new BlockNode("listItem", 2); + list.appendChild(item1); + list.appendChild(item2); + + expect(item1.lastLineChecked).toBe(false); + expect(leaf.lastLineChecked).toBe(false); + + finalizeListTightness(list); + + // item1 is a listItem, so descending into it (to check its own lastChild for a trailing blank line) must have marked it checked; leaf is not list/listItem-kinded, so it is marked checked at the point the descent stops on it rather than being descended into. + expect(item1.lastLineChecked).toBe(true); + expect(leaf.lastLineChecked).toBe(true); + }); + + it("keeps a list tight when nothing is blank", () => { + const list = new BlockNode("list", 1); + const item1 = new BlockNode("listItem", 1); + const item2 = new BlockNode("listItem", 2); + list.appendChild(item1); + list.appendChild(item2); + + finalizeListTightness(list); + + expect(list.tight).toBe(true); + }); + + it("marks a list loose when an earlier item ends with a blank line before a following item", () => { + const list = new BlockNode("list", 1); + const item1 = new BlockNode("listItem", 1); + item1.lastLineBlank = true; + const item2 = new BlockNode("listItem", 2); + list.appendChild(item1); + list.appendChild(item2); + + finalizeListTightness(list); + + expect(list.tight).toBe(false); + }); + + it("descends through a listItem, not just a nested list, to find a blank line one level further down", () => { + const list = new BlockNode("list", 1); + const item1 = new BlockNode("listItem", 1); + const nestedList = new BlockNode("list", 1); + const nestedItem = new BlockNode("listItem", 1); + const deepLeaf = new BlockNode("paragraph", 1); + deepLeaf.lastLineBlank = true; + nestedItem.appendChild(deepLeaf); + nestedList.appendChild(nestedItem); + item1.appendChild(nestedList); + const item2 = new BlockNode("listItem", 2); + list.appendChild(item1); + list.appendChild(item2); + + finalizeListTightness(list); + + expect(list.tight).toBe(false); + }); +}); diff --git a/packages/markdown-codec/src/block/list.ts b/packages/markdown-codec/src/block/list.ts index 2e8b0adca..64f1db659 100644 --- a/packages/markdown-codec/src/block/list.ts +++ b/packages/markdown-codec/src/block/list.ts @@ -27,16 +27,6 @@ const NON_SPACE_PATTERN = /[^ \t\f\v\r\n]/; // An ordered list may interrupt a paragraph only when it starts at 1 (spec 0.31.2: "In order for a list to interrupt a paragraph, it must start with 1"). const INTERRUPTING_ORDERED_START = 1; -function isBulletMarker(char: string): char is MarkdownBulletMarker { - return char === "-" || char === "*" || char === "+"; -} - -function isOrderedDelimiter( - char: string, -): char is MarkdownOrderedListDelimiter { - return char === "." || char === ")"; -} - interface MarkerMatch { readonly length: number; readonly data: Omit; @@ -49,26 +39,20 @@ function matchMarker( ): MarkerMatch | undefined { const bullet = BULLET_MARKER_PATTERN.exec(rest); if (bullet !== null) { - const char = bullet[0]; - if (!isBulletMarker(char)) { - return undefined; - } + // BULLET_MARKER_PATTERN's own character class (`[*+-]`) is exactly MarkdownBulletMarker's three members, so a match's own char is never anything else -- no runtime check could ever see the "else" side of that, only TypeScript's own indexed-access typing needs told. + const char = bullet[0] as MarkdownBulletMarker; return { length: bullet[0].length, data: { type: "bullet", bulletChar: char, markerOffset: indent }, }; } const ordered = ORDERED_MARKER_PATTERN.exec(rest); - const digits = ordered?.[1]; - const delimiter = ordered?.[2]; - if ( - ordered === null || - digits === undefined || - delimiter === undefined || - !isOrderedDelimiter(delimiter) - ) { + if (ordered === null) { return undefined; } + // Neither capturing group in ORDERED_MARKER_PATTERN is optional, so a successful match always populates both -- TypeScript's own RegExpExecArray typing has no way to say that (every capture reads as possibly-undefined, alternation or not), so both reads are cast the same way the bullet branch above already casts its own single capture. + const digits = ordered[1]!; + const delimiter = ordered[2] as MarkdownOrderedListDelimiter; const start = Number.parseInt(digits, 10); if (containerIsParagraph && start !== INTERRUPTING_ORDERED_START) { return undefined; @@ -110,41 +94,32 @@ export function parseListMarker( line.advanceToNextNonspace(); line.advance(match.length); - // Measure the spaces following the marker in COLUMNS, stopping at the code-indent threshold: past that point the exact count no longer changes the answer, and a single tab can supply all of them at once. The threshold IS the code indent, not a number of its own -- spaces past it make the content indented code rather than the item's own content indent. + // Measure the spaces following the marker in COLUMNS. No cap at the code-indent threshold here -- the branch below already resets the cursor back to afterMarkerMark and re-derives the item's own content indent from scratch whenever followingSpaces turns out to exceed it (or the rest of the line is blank), so a mid-scan cap would only change how many spaces this loop itself walks past, never the value parseListMarker returns or the cursor position it leaves behind. const afterMarkerMark = line.mark(); const afterMarkerColumn = line.column; // LineCursor.peek() reports a tab as a single space, one column at a time (src/scan), so testing for a space alone covers both -- there is no '\t' to compare against at this level. do { line.advance(1); - } while ( - line.column - afterMarkerColumn <= CODE_INDENT_COLUMNS && - line.peek() === " " - ); + } while (line.peek() === " "); const followingSpaces = line.column - afterMarkerColumn; const startsBlank = line.atEnd; - if ( - followingSpaces > CODE_INDENT_COLUMNS || - followingSpaces < 1 || - startsBlank - ) { + // No separate `followingSpaces < 1` disjunct: the do-while above always runs its body at least once, and LineCursor.advance() only ever leaves `line.column` unchanged when the cursor was already at the absolute end of input before that call -- so followingSpaces can never come out to 0 without startsBlank also being true, and a disjunct that can never be true on its own is not a real second condition. + if (followingSpaces > CODE_INDENT_COLUMNS || startsBlank) { // Either the content is indented code (5+ columns past the marker) or there is no content on this line at all: the item's own content indent is the marker plus a single column, and everything past that is content. line.reset(afterMarkerMark); - if (line.peek() === " ") { - line.advance(1); - } + // Unconditional, not `if (line.peek() === " ") line.advance(1)`: the marker-follows-by check above already guarantees the character right after the marker is a space/tab or end of line, so this is either consuming that one space/tab (the followingSpaces > 4 case) or a no-op past the end of input (the startsBlank case) -- never a third, unguarded shape. + line.advance(1); return { ...match.data, padding: match.length + 1 }; } return { ...match.data, padding: match.length + followingSpaces }; } // Whether a newly started item continues the list that is already open, or starts a fresh one. spec 0.31.2: "a list is a sequence of list items of the same type" -- changing the bullet character or the ordered delimiter starts a new list, even with no blank line in between. +// +// No separate a.type === b.type check: bulletChar is set only on a "bullet" marker and delimiter only on an "ordered" one (see ListMarkerData), so whenever the two markers are different variants exactly one of the two comparisons below pits a real value against undefined and is already false -- a same-type comparison could never survive that pairing without the field comparisons already agreeing too. export function listsMatch(a: ListMarkerData, b: ListMarkerData): boolean { - return ( - a.type === b.type && - a.delimiter === b.delimiter && - a.bulletChar === b.bulletChar - ); + return a.delimiter === b.delimiter && a.bulletChar === b.bulletChar; } // Whether `block` ends with a blank line, looking through the last child of a list or list item to reach the block that actually recorded one. Memoised through BlockNode.lastLineChecked so a deeply nested list is descended at most once per finalisation rather than once per item. diff --git a/packages/markdown-codec/src/block/node.test.ts b/packages/markdown-codec/src/block/node.test.ts new file mode 100644 index 000000000..73fb96305 --- /dev/null +++ b/packages/markdown-codec/src/block/node.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from "vitest"; +import { BlockNode, canContain } from "./node"; + +describe("BlockNode field defaults", () => { + it("defaults infoString/literal/headerLine/footnoteLabel to the empty string", () => { + const node = new BlockNode("paragraph", 1); + expect(node.infoString).toBe(""); + expect(node.literal).toBe(""); + expect(node.headerLine).toBe(""); + expect(node.footnoteLabel).toBe(""); + }); +}); + +describe("BlockNode.replaceWith", () => { + it("replaces the node in its parent's own children array, in place", () => { + const parent = new BlockNode("document", 1); + const original = new BlockNode("paragraph", 1); + const sibling = new BlockNode("paragraph", 2); + parent.appendChild(original); + parent.appendChild(sibling); + + const replacement = new BlockNode("heading", 1); + original.replaceWith(replacement); + + expect(parent.children).toEqual([replacement, sibling]); + expect(replacement.parent).toBe(parent); + expect(original.parent).toBeUndefined(); + }); + + it("does nothing when this node is not actually present in its own parent's children array", () => { + const parent = new BlockNode("document", 1); + const onlyChild = new BlockNode("paragraph", 1); + parent.appendChild(onlyChild); + + // A node whose own `.parent` points here, but that was never itself pushed into parent.children -- an inconsistent state replaceWith must not act on. + const detached = new BlockNode("paragraph", 2); + detached.parent = parent; + + const replacement = new BlockNode("heading", 1); + detached.replaceWith(replacement); + + expect(parent.children).toEqual([onlyChild]); + }); +}); + +describe("BlockNode.unlink", () => { + it("removes the node from its parent's own children array", () => { + const parent = new BlockNode("document", 1); + const a = new BlockNode("paragraph", 1); + const b = new BlockNode("paragraph", 2); + parent.appendChild(a); + parent.appendChild(b); + + a.unlink(); + + expect(parent.children).toEqual([b]); + expect(a.parent).toBeUndefined(); + }); + + it("does nothing to the parent's children when this node is not actually present there", () => { + const parent = new BlockNode("document", 1); + const onlyChild = new BlockNode("paragraph", 1); + parent.appendChild(onlyChild); + + const detached = new BlockNode("paragraph", 2); + detached.parent = parent; + + detached.unlink(); + + // A wrong `index !== -1` check (forced true) would splice(-1, 1) here, which deletes the LAST element of the array -- exactly the bug this pins against. + expect(parent.children).toEqual([onlyChild]); + }); +}); + +describe("canContain", () => { + it("lets a footnote definition hold an ordinary block", () => { + expect(canContain("footnoteDefinition", "paragraph")).toBe(true); + }); + + it("never lets a footnote definition hold a bare list item", () => { + expect(canContain("footnoteDefinition", "listItem")).toBe(false); + }); + + it("never lets a footnote definition nest another footnote definition", () => { + expect(canContain("footnoteDefinition", "footnoteDefinition")).toBe(false); + }); + + it("lets a document/blockquote/listItem hold an ordinary block", () => { + expect(canContain("document", "paragraph")).toBe(true); + expect(canContain("blockquote", "paragraph")).toBe(true); + expect(canContain("listItem", "paragraph")).toBe(true); + }); + + it("never lets a document/blockquote/listItem hold a bare list item directly", () => { + expect(canContain("document", "listItem")).toBe(false); + expect(canContain("blockquote", "listItem")).toBe(false); + expect(canContain("listItem", "listItem")).toBe(false); + }); + + it("lets a list hold only list items", () => { + expect(canContain("list", "listItem")).toBe(true); + expect(canContain("list", "paragraph")).toBe(false); + }); + + it("lets no leaf block hold any children", () => { + expect(canContain("paragraph", "paragraph")).toBe(false); + expect(canContain("codeBlock", "paragraph")).toBe(false); + }); +}); diff --git a/packages/markdown-codec/src/block/table.test.ts b/packages/markdown-codec/src/block/table.test.ts index d24da81ce..222a6abed 100644 --- a/packages/markdown-codec/src/block/table.test.ts +++ b/packages/markdown-codec/src/block/table.test.ts @@ -29,6 +29,37 @@ describe("splitTableRow", () => { it("splits after a doubled backslash, which escapes itself rather than the pipe", () => { expect(splitTableRow("a\\\\|b")).toEqual(["a\\\\", "b"]); }); + + it("trims leading/trailing whitespace from the whole line before reading its pipes", () => { + expect(splitTableRow(" | a | b | ")).toEqual(["a", "b"]); + }); + + it("strips a leading pipe without requiring a trailing one, and vice versa", () => { + expect(splitTableRow("| a | b")).toEqual(["a", "b"]); + expect(splitTableRow("a | b |")).toEqual(["a", "b"]); + }); + + it("treats a single trailing backslash with nothing after it as a literal character", () => { + expect(splitTableRow("a\\")).toEqual(["a\\"]); + }); +}); + +describe("endsWithUnescapedPipe (via splitTableRow's own trailing-pipe handling)", () => { + it("does not strip the trailing pipe when it is escaped by an odd run of backslashes", () => { + expect(splitTableRow("a\\|")).toEqual(["a|"]); + }); + + it("does strip the trailing pipe when it is preceded by an even run of backslashes", () => { + expect(splitTableRow("a\\\\|")).toEqual(["a\\\\"]); + }); + + it("counts a run of three trailing backslashes as odd (escaped), not stopping after one", () => { + expect(splitTableRow("a\\\\\\|")).toEqual(["a\\\\|"]); + }); + + it("counts a run of four trailing backslashes as even (unescaped), not stopping after one", () => { + expect(splitTableRow("a\\\\\\\\|")).toEqual(["a\\\\\\\\"]); + }); }); describe("parseTableDelimiterRow", () => { diff --git a/packages/markdown-codec/src/block/table.ts b/packages/markdown-codec/src/block/table.ts index 7d6ccef16..8b0945fff 100644 --- a/packages/markdown-codec/src/block/table.ts +++ b/packages/markdown-codec/src/block/table.ts @@ -27,9 +27,11 @@ export function splitTableRow(line: string): string[] { const cells: string[] = []; let current = ""; let index = 0; - while (index < text.length) { + // text.charAt(index) !== "", not index < text.length: the two are equivalent for every real index (charAt already returns "" one past the end, which none of this loop's own branches below can ever match either), but only this spelling's own mutation is actually reachable by a test rather than always landing on the identical fallthrough either way. + while (text.charAt(index) !== "") { const char = text.charAt(index); - if (char === "\\" && index + 1 < text.length) { + // No separate "is there a character after the backslash" guard: when the backslash is the very last character, text.charAt(index + 1) is already "" out of range, which the ternary below already treats as "not a pipe" and appends as char + "" -- the identical single backslash the no-escape fallthrough two branches down would append anyway, so the guard would only ever gate two provably equal outcomes. + if (char === "\\") { // An escaped pipe is resolved HERE rather than left for the inline phase's own backslash handling, because a cell's content may put it somewhere that handling never reaches: GFM's own example escapes a pipe inside a code span (`` | b `\|` az | ``), and a code span's literal is never backslash-processed. Every other escape is passed through untouched for the inline phase to resolve as usual. const escaped = text.charAt(index + 1); current += escaped === "|" ? escaped : char + escaped; @@ -54,10 +56,8 @@ function endsWithUnescapedPipe(text: string): boolean { return false; } let backslashes = 0; - while ( - backslashes + 1 < text.length && - text.charAt(text.length - 2 - backslashes) === "\\" - ) { + // No separate `backslashes + 1 < text.length` bound: charAt(text.length - 2 - backslashes) reads before the start of `text` once backslashes grows past text.length - 2, and charAt already returns "" for a negative index, which is never "\\" either -- so the loop already stops there on its own, on exactly the same iteration a length-based bound would have forced. + while (text.charAt(text.length - 2 - backslashes) === "\\") { backslashes += 1; } return backslashes % 2 === 0; @@ -85,10 +85,8 @@ export function parseTableDelimiterRow( if (!line.includes("|")) { return undefined; } + // No `cells.length === 0` guard: splitTableRow always pushes its own trailing `current.trim()` unconditionally, even over empty input, so it can never return an empty array for this function to guard against. const cells = splitTableRow(line); - if (cells.length === 0) { - return undefined; - } const alignments: MarkdownTableAlignment[] = []; for (const cell of cells) { if (!DELIMITER_CELL_PATTERN.test(cell)) { diff --git a/packages/markdown-codec/src/diagnostics/diagnostics.test.ts b/packages/markdown-codec/src/diagnostics/diagnostics.test.ts index 3c569ddee..244627df6 100644 --- a/packages/markdown-codec/src/diagnostics/diagnostics.test.ts +++ b/packages/markdown-codec/src/diagnostics/diagnostics.test.ts @@ -13,7 +13,14 @@ import { emitMarkdown } from "../emit/emit"; import { lowerMarkdown } from "../lower/lower"; import { createDiagnosticCollector } from "../test-support/diagnostics"; import { writeMarkdown } from "../write"; -import { MarkdownDiagnosticCodes } from "./diagnostics"; +import { + MarkdownDiagnosticCodes, + MarkdownInputTooLargeError, + MarkdownInvalidUtf8Error, + MarkdownNestingLimitExceededError, + MarkdownParseError, + MarkdownWriteError, +} from "./diagnostics"; function minimalDocument(blocks: readonly ContentBlock[]): ContentDocument { return { @@ -445,3 +452,89 @@ describe("every MarkdownDiagnosticCodes entry is reachable from real input", () expect(reached).toEqual(new Set(Object.values(MarkdownDiagnosticCodes))); }); }); + +// Runs `fn`, returning whatever it throws (or undefined if it doesn't) -- lets a test assert on a thrown error's own fields without a try/catch block of its own, and without vitest's `expect(fn).toThrow(...)`, which only ever checks the constructor and (optionally) the message. +function captureThrown(fn: () => void): unknown { + try { + fn(); + return undefined; + } catch (error) { + return error; + } +} + +// The throw tier's own error classes, exercised directly rather than only observed via .toThrow(SomeClass) at a real call site elsewhere: an instanceof check alone cannot distinguish a correct message/code/field from a mutated one, so each case here asserts every field the constructor sets, not just the class. +describe("throw-tier error classes carry their own precise code, message, and fields", () => { + it("MarkdownParseError: constructed directly (not through a subclass), name/code/message all carry the constructor's own arguments", () => { + // Every concrete subclass overwrites `this.name` in its own constructor right after calling super(), so a MarkdownInvalidUtf8Error/MarkdownInputTooLargeError/MarkdownNestingLimitExceededError instance can never observe MarkdownParseError's own `this.name = "MarkdownParseError"` assignment -- it is immediately clobbered. Only a direct instantiation of the base class exercises that line. + const error = new MarkdownParseError("md/some-code", "some message"); + expect(error).toBeInstanceOf(MarkdownParseError); + expect(error.name).toBe("MarkdownParseError"); + expect(error.code).toBe("md/some-code"); + expect(error.message).toBe("some message"); + }); + + it("MarkdownWriteError: constructed directly (not through a subclass), name/code/message all carry the constructor's own arguments", () => { + // The write-side twin of the MarkdownParseError case above -- every concrete subclass (MarkdownUnbalancedConstructMarkersError and siblings) overwrites `this.name` immediately after super(), so only a direct instantiation observes the base class's own assignment. + const error = new MarkdownWriteError("md/some-code", "some message"); + expect(error).toBeInstanceOf(MarkdownWriteError); + expect(error.name).toBe("MarkdownWriteError"); + expect(error.code).toBe("md/some-code"); + expect(error.message).toBe("some message"); + }); + + it("MarkdownInvalidUtf8Error: default message, code, and MarkdownParseError lineage", () => { + const error = new MarkdownInvalidUtf8Error(); + expect(error).toBeInstanceOf(MarkdownParseError); + expect(error.name).toBe("MarkdownInvalidUtf8Error"); + expect(error.code).toBe("md/invalid-utf8"); + expect(error.message).toBe("input is not valid UTF-8"); + }); + + it("MarkdownInvalidUtf8Error: a caller-supplied message overrides the default without touching the code", () => { + const error = new MarkdownInvalidUtf8Error("custom detail"); + expect(error.message).toBe("custom detail"); + expect(error.code).toBe("md/invalid-utf8"); + }); + + it("MarkdownInputTooLargeError: lowerMarkdown enforces maxInputBytes against the input's own UTF-8 byte length, not its character count", () => { + // "é" is two UTF-8 bytes but one UTF-16 code unit -- a maxInputBytes check keyed on .length rather than TextEncoder byte length would let this through at limit 5. + const source = "aaéé"; + const error = captureThrown(() => + lowerMarkdown(source, { maxInputBytes: 5 }), + ); + expect(error).toBeInstanceOf(MarkdownInputTooLargeError); + expect(error).toBeInstanceOf(MarkdownParseError); + const typed = error as MarkdownInputTooLargeError; + expect(typed.name).toBe("MarkdownInputTooLargeError"); + expect(typed.code).toBe("md/input-too-large"); + expect(typed.maxInputBytes).toBe(5); + expect(typed.actualBytes).toBe(6); + expect(typed.message).toBe( + "input is 6 bytes, exceeding the configured maximum of 5 bytes", + ); + }); + + it("MarkdownInputTooLargeError: input at exactly maxInputBytes does not throw", () => { + expect(() => lowerMarkdown("aaéé", { maxInputBytes: 6 })).not.toThrow(); + }); + + it("MarkdownNestingLimitExceededError: parseMarkdown enforces maxNesting against the open-block stack depth", () => { + // Three levels of blockquote nesting against a maxNesting of 2 -- the third open (nestingDepth reaching the limit) must throw, not the first or second. + const source = "> > > deep"; + const error = captureThrown(() => parseMarkdown(source, { maxNesting: 2 })); + expect(error).toBeInstanceOf(MarkdownNestingLimitExceededError); + expect(error).toBeInstanceOf(MarkdownParseError); + const typed = error as MarkdownNestingLimitExceededError; + expect(typed.name).toBe("MarkdownNestingLimitExceededError"); + expect(typed.code).toBe("md/nesting-limit-exceeded"); + expect(typed.maxNesting).toBe(2); + expect(typed.message).toBe( + "block nesting exceeds the configured limit of 2", + ); + }); + + it("MarkdownNestingLimitExceededError: nesting at exactly maxNesting does not throw", () => { + expect(() => parseMarkdown("> shallow", { maxNesting: 2 })).not.toThrow(); + }); +}); diff --git a/packages/markdown-codec/src/emit/emit.test.ts b/packages/markdown-codec/src/emit/emit.test.ts index 07fc9b350..5f4ff6276 100644 --- a/packages/markdown-codec/src/emit/emit.test.ts +++ b/packages/markdown-codec/src/emit/emit.test.ts @@ -50,6 +50,19 @@ describe("headings", () => { ).toBe("### foo"); }); + it("does NOT fire HEADING_LEVEL_CLAMPED for a heading whose own level needs no clamping at all", () => { + const collector = createDiagnosticCollector(); + emitMarkdown( + doc([ + { kind: "paragraph", runs: [{ text: "foo" }], styleId: "Heading3" }, + ]), + { sink: collector.sink }, + ); + expect(collector.has(MarkdownDiagnosticCodes.HEADING_LEVEL_CLAMPED)).toBe( + false, + ); + }); + it('emits level 1/2 as setext when headingStyle: "setext" is requested, and falls back to ATX beyond level 2', () => { expect( emitMarkdown( @@ -103,6 +116,11 @@ describe("headings", () => { expect( collector.has(MarkdownDiagnosticCodes.HEADING_LINE_BREAK_COLLAPSED), ).toBe(true); + const diagnostic = collector.diagnostics.find( + (d) => d.code === MarkdownDiagnosticCodes.HEADING_LINE_BREAK_COLLAPSED, + ); + expect(diagnostic?.message).toContain("3"); + expect(diagnostic?.message).toContain("line break"); }); it("measures the setext underline's length against the CommonMark first line even when its own embedded break is a bare CR, not an LF", () => { @@ -163,6 +181,13 @@ describe("headings", () => { MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, ), ).toBe(true); + const diagnostic = collector.diagnostics.find( + (d) => + d.code === + MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, + ); + expect(diagnostic?.message).toContain("no heading text"); + expect(diagnostic?.message).toContain("attach to"); expect( collector.has(MarkdownDiagnosticCodes.HEADING_LINE_BREAK_COLLAPSED), ).toBe(false); @@ -655,6 +680,62 @@ describe("headings", () => { describe("an explicit headingStyle: 'setext' request against a break-free heading that is unsafe on its own terms is still refused, with a diagnostic (ExaDev/documents.js#940)", () => { // Every OTHER unsafe-for-setext test in this file exercises a heading whose text embeds an actual line break -- the break itself is what makes setext a candidate rendering at all when headingStyle is left at its 'atx' default. This heading has NO embedded break anywhere: headingStyle: 'setext' is the ONLY reason setext is even attempted, and unsafeSetextBreakReason's own first-line-indentation check applies exactly as much to a single-line heading as to a multi-line one. Pre-fix, every heading-related diagnostic sat behind an `embedsLineBreak` guard, so this exact shape silently fell through to a bare, unmarked ATX heading -- an explicit caller preference honoured in appearance (setext was refused, correctly) but with zero signal that it happened. + it("does NOT fire HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT for a break-free, 4+-column-indented level-3 heading even with headingStyle: 'setext' requested -- level <= MAX_SETEXT_LEVEL is its own genuine gate, not implied by setextRequested and unsafeForSetext alone", () => { + const collector = createDiagnosticCollector(); + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: " foo" }], + styleId: "Heading3", + }, + ]), + { headingStyle: "setext", sink: collector.sink }, + ); + expect(written).toBe("### foo"); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, + ), + ).toBe(false); + expect( + collector.has(MarkdownDiagnosticCodes.HEADING_LINE_BREAK_COLLAPSED), + ).toBe(false); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_STYLE_OVERRIDDEN_FOR_LINE_BREAK, + ), + ).toBe(false); + }); + + it("does NOT fire HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT for a break-free, 4+-column-indented level-1 heading when setext was never requested at all -- unsafeForSetext alone, with setextRequested false, must not enter the unsafe-diagnostic branch", () => { + const collector = createDiagnosticCollector(); + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: " foo" }], + styleId: "Heading1", + }, + ]), + { sink: collector.sink }, + ); + expect(written).toBe("# foo"); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, + ), + ).toBe(false); + expect( + collector.has(MarkdownDiagnosticCodes.HEADING_LINE_BREAK_COLLAPSED), + ).toBe(false); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_STYLE_OVERRIDDEN_FOR_LINE_BREAK, + ), + ).toBe(false); + }); + it("collapses to ATX with HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT when the heading's own (break-free) text is indented 4 or more columns", () => { const collector = createDiagnosticCollector(); const written = emitMarkdown( @@ -715,6 +796,30 @@ describe("headings", () => { { text: "> q", source: { format: "markdown" as const, xml: "> q" } }, ], }, + { + level: "Heading1" as const, + shape: "ATX-heading-shaped ('# x')", + runs: [ + { text: "# x", source: { format: "markdown" as const, xml: "# x" } }, + ], + }, + { + level: "Heading2" as const, + shape: "math-block-shaped ('$$')", + runs: [ + { text: "$$", source: { format: "markdown" as const, xml: "$$" } }, + ], + }, + { + level: "Heading1" as const, + shape: "list-marker-shaped ('- item')", + runs: [ + { + text: "- item", + source: { format: "markdown" as const, xml: "- item" }, + }, + ], + }, ])( "collapses to ATX with HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT when the heading's own single (break-free) line is itself $shape (ExaDev/documents.js#940)", ({ level, runs }) => { @@ -788,12 +893,58 @@ describe("headings", () => { expect(headingBlock.runs.map((run) => run.text).join("")).toBe("foo"); }); + it("refuses to promote a break-free heading whose ENTIRE text is an ordered-list marker not starting at 1 -- interruptsSetextParagraph's first-line call must use the genuine block-start sense (any start number counts), not the paragraph-continuation sense (only start-at-1 counts)", () => { + const collector = createDiagnosticCollector(); + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [ + { + text: "2. foo", + source: { format: "markdown", xml: "2. foo" }, + }, + ], + styleId: "Heading1", + }, + ]), + { headingStyle: "setext", sink: collector.sink }, + ); + expect(written).toBe("# 2. foo"); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, + ), + ).toBe(true); + }); + + it("still safely promotes to setext when a NON-FIRST line is an ordered-list marker not starting at 1 -- interruptsSetextParagraph's non-first-line call must use the paragraph-continuation sense (CommonMark's own exception absorbs it as continuation text), not the block-start sense", () => { + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [ + { text: "foo" }, + { text: "\n" }, + { + text: "2. bar", + source: { format: "markdown", xml: "2. bar" }, + }, + ], + styleId: "Heading1", + }, + ]), + ); + expect(written).toBe("foo\\\n2. bar\n===="); + }); + it.each([ { level: "Heading1" as const, underline: "=" }, { level: "Heading2" as const, underline: "-" }, ])( "still promotes $level to setext and round-trips the leading break losslessly", ({ level, underline }) => { + const collector = createDiagnosticCollector(); const written = emitMarkdown( doc([ { @@ -802,8 +953,17 @@ describe("headings", () => { styleId: level, }, ]), + { sink: collector.sink }, ); expect(written).toBe(`\\\nfoo\n${underline}`); + const diagnostic = collector.diagnostics.find( + (d) => + d.code === + MarkdownDiagnosticCodes.HEADING_STYLE_OVERRIDDEN_FOR_LINE_BREAK, + ); + // Unlike the genuinely-absorbed leading-break case above, this break survives losslessly -- the diagnostic must say so, not claim it was absorbed. + expect(diagnostic?.message).toContain("so the break survives"); + expect(diagnostic?.message).not.toContain("absorbed"); const reparsed = lowerMarkdown(written); if (reparsed.kind !== "wordprocessing") { @@ -1254,6 +1414,25 @@ describe("math (ExaDev/markdown-codec#53)", () => { ).toBe("$$\n$$"); }); + it("does not render the $$ math shortcut when objectKind disagrees with the document's own kind, even though the document itself is a formula carrying real presentation LaTeX -- both fields must agree, not just the document's own kind", () => { + expect( + emitMarkdown( + doc([ + { + kind: "embeddedObject", + objectKind: "wordprocessing", + document: { + kind: "formula", + metadata: {}, + formula: { mathml: [], presentation: { latex: "x^2" } }, + }, + frame: { xPt: 0, yPt: 0, widthPt: 1, heightPt: 1 }, + }, + ]), + ), + ).toBe(""); + }); + it("still silently drops an embedded object of any other kind, and a formula with no presentation LaTeX, which have no markdown spelling", () => { expect( emitMarkdown( @@ -1411,6 +1590,32 @@ describe("blockquotes", () => { expect(collector.has(MarkdownDiagnosticCodes.CONSTRUCT_UNREPRESENTED)).toBe( true, ); + const diagnostic = collector.diagnostics.find( + (d) => d.code === MarkdownDiagnosticCodes.CONSTRUCT_UNREPRESENTED, + ); + expect(diagnostic?.message).toContain("division"); + expect(diagnostic?.message).toContain("no markdown syntax"); + }); + + it("renders a division transparently (no '> ' wrapping) when only SOME of its wrapped paragraphs carry the dual-carry quote indent, not all of them -- isMaterialisedDivision requires EVERY child to qualify, not just one", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { kind: "division", name: "mixed" }, + }, + { + kind: "paragraph", + runs: [{ text: "quoted" }], + styleId: "Quote", + indentLeftPt: 36, + }, + { kind: "paragraph", runs: [{ text: "plain" }] }, + { kind: "constructEnd" }, + ]), + ); + // Transparent, not materialised -- so each wrapped paragraph still recovers (or doesn't) its own quote depth independently, exactly as if the division weren't there at all: "quoted" keeps its own '> ' from indentLeftPt, "plain" has none. + expect(markdown).toBe("> quoted\n\nplain"); }); it("round-trips blockquote shapes byte for byte through lower -> emit -> lower, including nesting and adjacency", () => { @@ -1532,76 +1737,368 @@ describe("lists", () => { expect(markdown).toBe("- [x] done"); }); - it("renders every block of one itemId as a single item -- a blank line and the continuation indent between blocks, one marker only", () => { + it("recognises the pre-field UNCHECKED glyph spelling on its OWN, with no checked item preceding it in the same call, and with real text following the glyph in the SAME run (so startsWith and endsWith genuinely disagree)", () => { const markdown = emitMarkdown( doc([ { kind: "paragraph", - runs: [{ text: "a" }], - list: { numId: "md1:bullet+loose", level: 0, itemId: "md-i1" }, + runs: [{ text: "☐ todo" }], + list: { numId: "md1:bullet+task", level: 0 }, }, + ]), + ); + expect(markdown).toBe("- [ ] todo"); + }); + + it("never strips a run's own leading text when the checkbox comes from membership.checked instead of a legacy glyph, even when that text happens to look exactly like the legacy glyph spelling", () => { + const markdown = emitMarkdown( + doc([ { kind: "paragraph", - runs: [{ text: "second block" }], - list: { numId: "md1:bullet+loose", level: 0, itemId: "md-i1" }, + runs: [{ text: "☒ literal text not a glyph to strip" }], + list: { + numId: "md1:bullet+task", + level: 0, + checked: true, + itemId: "i1", + }, }, ]), ); - expect(markdown).toBe("- a\n\n second block"); + // stripGlyph must be false here -- the checkbox already came from membership.checked, so this run's own text is ordinary content, never a legacy glyph prefix to strip back off. + expect(markdown).toBe("- [x] ☒ literal text not a glyph to strip"); }); - it("renders same-level paragraphs with DIFFERENT itemIds as separate items even when they share a numId", () => { + it("strips a legacy checkbox glyph from a run that ALSO carries its own following text, not just when the glyph fills a whole separate run of its own", () => { const markdown = emitMarkdown( doc([ { kind: "paragraph", - runs: [{ text: "a" }], - list: { numId: "md1:bullet", level: 0, itemId: "md-i1" }, + runs: [{ text: "☒ done" }], + list: { numId: "md1:bullet+task", level: 0 }, }, + ]), + ); + expect(markdown).toBe("- [x] done"); + }); + + it("renders an ordinary bullet with no checkbox at all for a task-flagged numId whose leading text matches neither legacy glyph", () => { + const markdown = emitMarkdown( + doc([ { kind: "paragraph", - runs: [{ text: "b" }], - list: { numId: "md1:bullet", level: 0, itemId: "md-i2" }, + runs: [{ text: "ordinary" }], + list: { numId: "md1:bullet+task", level: 0 }, }, ]), ); - expect(markdown).toBe("- a\n- b"); + expect(markdown).toBe("- ordinary"); }); - it("keeps one item per paragraph for memberships with no itemId at all -- the cross-format shape every foreign producer sends", () => { + it("never misreads a ballot-box glyph as a checkbox for an ORDINARY (non-task-flagged) numId, even though its leading text happens to match the legacy glyph spelling exactly", () => { const markdown = emitMarkdown( doc([ { kind: "paragraph", - runs: [{ text: "a" }], + runs: [{ text: "☒ not a checkbox" }], list: { numId: "md1:bullet", level: 0 }, }, + ]), + ); + expect(markdown).toBe("- ☒ not a checkbox"); + }); + + it("does not pad a genuinely blank line inside a NESTED sub-list's own rendering with trailing indent whitespace once that rendering is indented under its parent item", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, { kind: "paragraph", runs: [{ text: "b" }], - list: { numId: "md1:bullet", level: 0 }, + list: { numId: "md1:bullet+loose", level: 1 }, + }, + { + kind: "paragraph", + runs: [{ text: "c" }], + list: { numId: "md1:bullet+loose", level: 1 }, }, ]), ); - expect(markdown).toBe("- a\n- b"); + expect(markdown).toBe("- a\n - b\n\n - c"); + // Split on "\n" and re-check the blank line specifically: exactly "", never " " (indent with nothing on it). + expect(markdown.split("\n")).toContain(""); }); - it("round-trips a task list byte for byte, and a multi-block item semantically with a stable re-emission", () => { - const task = "- [x] done\n- [ ] todo"; - expect(emitMarkdown(lowerMarkdown(task))).toBe(task); - expect(lowerMarkdown(emitMarkdown(lowerMarkdown(task)))).toEqual( - lowerMarkdown(task), + it("recognises a construct as carrying an item's own itemId when ONLY ONE of its several children actually carries it, not requiring every child to -- constructCarriesListItemId is an ANY match, not an ALL match", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "constructStart", + descriptor: { kind: "division", name: "d1" }, + }, + { + kind: "paragraph", + runs: [{ text: "carries i1" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { kind: "paragraph", runs: [{ text: "other" }] }, + { kind: "constructEnd" }, + ]), ); + // The construct is recognised as belonging to item i1 (one of its two children carries that itemId, and ANY match is enough) and stays absorbed into i1's own run, rather than fracturing out as an unrelated top-level construct. + expect(markdown).toBe("- a\n\n carries i1\n\n other"); + }); - // A loose multi-block item re-emits with the loose sibling spacing the numId itself records, so the text is not byte-identical to a source whose author ran the sibling tight -- but the reparse reproduces the identical document and a second pass is a fixed point. - const multi = lowerMarkdown("- a\n\n continuation of a\n- b"); - const written = emitMarkdown(multi); - expect(lowerMarkdown(written)).toEqual(multi); - expect(emitMarkdown(lowerMarkdown(written))).toBe(written); + it("pops a SIBLING item's own membership off openMemberships before pushing the next one at the SAME level, not just a genuinely deeper one -- a stale sibling entry left on the stack could wrongly absorb a later construct that only carries THAT earlier sibling's own itemId", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "i1" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "i2" }], + list: { numId: "md1:bullet", level: 0, itemId: "i2" }, + }, + { + kind: "constructStart", + descriptor: { kind: "division", name: "d1" }, + }, + { + kind: "paragraph", + runs: [{ text: "carries i1" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { kind: "constructEnd" }, + ]), + ); + // i1's own membership must already be off the stack once i2 (its sibling at the SAME level) is pushed -- so this construct, which carries only i1's itemId, cannot still be absorbed into the (no-longer-open) i1 item; it fractures out and re-enters as its OWN fresh list region instead (its wrapped paragraph still carries itemId i1, but as a new region, not a continuation of the item above). + expect(markdown).toBe("- i1\n- i2\n\n- carries i1"); }); - it("round-trips a TIGHT list item containing a paragraph and a fenced code block as ONE item, not two", () => { - const source = "- a\n ```\n code\n ```\n- b"; + it("finds the REAL last styleId of a NESTED sub-list's own last block, not just undefined, so the outer item's own resuming block reflects what that sub-list actually ends on", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "b" }], + styleId: "CodeBlock", + list: { numId: "md1:bullet", level: 1, itemId: "i2" }, + }, + { + kind: "paragraph", + runs: [{ text: "z" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]), + ); + // No forced blank line before "z": the nested sub-list's own last (and only) item is a CodeBlock, which terminates cleanly -- reading segment.blocks[segment.blocks.length - 1] must actually find that item, not silently report undefined (which would wrongly force a blank line here). + expect(markdown).toBe("- a\n - ```\n b\n ```\n z"); + }); + + it("needs no forced blank line before a construct whose own FIRST child is an EMPTY, non-division nested construct, in the SAME list item -- emitItemCanInterrupt's construct-recursion base case (an empty children array) defaults to interrupting, exactly like the non-paragraph fallback it mirrors", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "constructStart", + descriptor: { + kind: "link", + target: { kind: "external", uri: "https://example.com" }, + }, + }, + { + kind: "constructStart", + descriptor: { kind: "anchor", anchorType: "bookmark", name: "empty" }, + }, + { kind: "constructEnd" }, + { + kind: "paragraph", + runs: [{ text: "caption" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { kind: "constructEnd" }, + ]), + ); + expect(markdown).toBe("- a\n caption"); + }); + + it("needs no forced blank line between an open (styleId-less) paragraph and a following link-construct whose FIRST child is a non-paragraph IMAGE block, in the SAME list item -- a non-paragraph block always interrupts an open paragraph unconditionally, per emitItemCanInterrupt's non-construct fallback", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "constructStart", + descriptor: { + kind: "link", + target: { kind: "external", uri: "https://example.com/a.png" }, + }, + }, + { + kind: "image", + format: "png", + base64: "AAAA", + widthPt: 1, + heightPt: 1, + altText: "img", + }, + { + kind: "paragraph", + runs: [{ text: "caption" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { kind: "constructEnd" }, + ]), + ); + expect(markdown).toBe( + "- a\n ![img](data:image/png;base64,AAAA)\n\n caption", + ); + }); + + it("finds the REAL last styleId inside a construct that resumes a list item, not just undefined, so a following block's own blank-line decision reflects what that construct actually ends on", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "constructStart", + descriptor: { kind: "division", name: "d1" }, + }, + { + kind: "paragraph", + runs: [{ text: "mid" }], + styleId: "CodeBlock", + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { kind: "constructEnd" }, + { + kind: "paragraph", + runs: [{ text: "z" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]), + ); + // No forced blank line before "z": the construct's own last (and only) wrapped block is a CodeBlock, which terminates cleanly -- lastStyleIdOf must actually find that CodeBlock styleId through the construct's own children, not silently report undefined (which would wrongly force a blank line here). + expect(markdown).toBe("- a\n ```\n mid\n ```\n z"); + }); + + it("renders every block of one itemId as a single item -- a blank line and the continuation indent between blocks, one marker only", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet+loose", level: 0, itemId: "md-i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "second block" }], + list: { numId: "md1:bullet+loose", level: 0, itemId: "md-i1" }, + }, + ]), + ); + expect(markdown).toBe("- a\n\n second block"); + }); + + it("does not pad a genuinely blank line inside a LATER (continuation) block's own body with trailing indent whitespace once that block is indented under the item's marker", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "md-i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "x\n\ny" }], + styleId: "CodeBlock", + list: { numId: "md1:bullet", level: 0, itemId: "md-i1" }, + }, + ]), + ); + expect(markdown).toBe("- a\n ```\n x\n\n y\n ```"); + expect(markdown.split("\n")).toContain(""); + }); + + it("renders same-level paragraphs with DIFFERENT itemIds as separate items even when they share a numId", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "md-i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "b" }], + list: { numId: "md1:bullet", level: 0, itemId: "md-i2" }, + }, + ]), + ); + expect(markdown).toBe("- a\n- b"); + }); + + it("keeps one item per paragraph for memberships with no itemId at all -- the cross-format shape every foreign producer sends", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0 }, + }, + { + kind: "paragraph", + runs: [{ text: "b" }], + list: { numId: "md1:bullet", level: 0 }, + }, + ]), + ); + expect(markdown).toBe("- a\n- b"); + }); + + it("round-trips a task list byte for byte, and a multi-block item semantically with a stable re-emission", () => { + const task = "- [x] done\n- [ ] todo"; + expect(emitMarkdown(lowerMarkdown(task))).toBe(task); + expect(lowerMarkdown(emitMarkdown(lowerMarkdown(task)))).toEqual( + lowerMarkdown(task), + ); + + // A loose multi-block item re-emits with the loose sibling spacing the numId itself records, so the text is not byte-identical to a source whose author ran the sibling tight -- but the reparse reproduces the identical document and a second pass is a fixed point. + const multi = lowerMarkdown("- a\n\n continuation of a\n- b"); + const written = emitMarkdown(multi); + expect(lowerMarkdown(written)).toEqual(multi); + expect(emitMarkdown(lowerMarkdown(written))).toBe(written); + }); + + it("round-trips a TIGHT list item containing a paragraph and a fenced code block as ONE item, not two", () => { + const source = "- a\n ```\n code\n ```\n- b"; const first = lowerMarkdown(source); if (first.kind !== "wordprocessing") { throw new Error("expected a wordprocessing ContentDocument"); @@ -2041,6 +2538,46 @@ describe("lists", () => { expect(headingBlock.runs.map((run) => run.text).join("")).toBe("h"); }); + it("inserts a blank line between a paragraph and a following Heading2 rendered as setext too, not just Heading1 -- willRenderAsSetext's own level > MAX_SETEXT_LEVEL check must correctly admit level 2 AT the boundary, not treat it the same as a level that exceeds it", () => { + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + styleId: "Heading2", + runs: [{ text: "h" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]), + { headingStyle: "setext" }, + ); + expect(written).toBe("- a\n\n h\n -"); + }); + + it("keeps a paragraph and a following Heading3 TIGHT even with headingStyle: 'setext' requested -- level 3 always renders as ATX regardless of the configured style (there is no setext spelling beyond level 2), so willRenderAsSetext must still refuse it rather than treating any level as eligible whenever setext is merely requested", () => { + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + styleId: "Heading3", + runs: [{ text: "h" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]), + { headingStyle: "setext" }, + ); + expect(written).toBe("- a\n ### h"); + }); + it("inserts a blank line between a paragraph and a following heading that is forced to setext by its OWN embedded line break, even with the default ATX headingStyle -- the interrupt guard must key off what the heading will actually render as, not the configured style, or the preceding paragraph is silently absorbed into it on reparse (ExaDev/documents.js#940)", () => { const softBreakRuns = [ { text: "h1" }, @@ -2389,6 +2926,74 @@ describe("lists", () => { expect(mathBlock?.kind).toBe("embeddedObject"); }); + it("needs no forced blank line between a CodeBlock and a following plain paragraph in the SAME tight list item -- a fenced code block's own closing fence terminates cleanly, with nothing left open for the next line to lazily continue", () => { + const source = doc([ + { + kind: "paragraph", + styleId: "CodeBlock", + runs: [{ text: "x" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "y" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]); + expect(emitMarkdown(source)).toBe("- ```\n x\n ```\n y"); + }); + + it("needs no forced blank line between a MathBlock and a following plain paragraph in the SAME tight list item -- a $$ block's own closing delimiter terminates cleanly", () => { + const source = doc([ + { + kind: "paragraph", + styleId: "MathBlock", + runs: [{ text: "x" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "y" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]); + expect(emitMarkdown(source)).toBe("- $$\n x\n $$\n y"); + }); + + it("needs no forced blank line between a HorizontalRule and a following plain paragraph in the SAME tight list item -- a thematic break is a single complete line with nothing left open", () => { + const source = doc([ + { + kind: "paragraph", + styleId: "HorizontalRule", + runs: [], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "y" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]); + expect(emitMarkdown(source, { thematicBreakChar: "*" })).toBe("- ***\n y"); + }); + + it("DOES force a blank line between two plain paragraphs sharing an unrecognised, non-quotable, non-clean-terminating styleId in the same tight list item -- src/lower's own reader can only ever have produced this pair from a genuine source blank line, so the write side must reinsert it even though the list itself is tight", () => { + const source = doc([ + { + kind: "paragraph", + styleId: "SomeUnrecognisedStyle", + runs: [{ text: "a" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + { + kind: "paragraph", + runs: [{ text: "b" }], + list: { numId: "md1:bullet", level: 0, itemId: "i1" }, + }, + ]); + expect(emitMarkdown(source)).toBe("- a\n\n b"); + }); + it("separates loose-list siblings with a blank line and tight-list siblings with none", () => { const tight = emitMarkdown( doc([ @@ -2514,6 +3119,15 @@ describe("adjacent same-type lists get different marker glyphs (ExaDev/markdown- }); describe("tables", () => { + it("emits an empty string for a table with no rows at all, rather than a header/delimiter line of nothing", () => { + const table: ContentTable = { + kind: "table", + columnWidthsPt: [], + rows: [], + }; + expect(emitMarkdown(doc([table]))).toBe(""); + }); + it("emits alignment markers read from the header row's own cell alignment", () => { const table: ContentTable = { kind: "table", @@ -2682,13 +3296,24 @@ describe("images", () => { ); expect(emitMarkdown(doc([image]), { images: false })).toBe("![alt]()"); }); -}); -describe("round trip through src/lower", () => { - it("renders a code span run back as backticks and a plain autolink run back as ", () => { - const source = "`code` and "; - const lowered = lowerMarkdown(source); - expect(emitMarkdown(lowered)).toBe("`code` and "); + it("emits an empty alt attribute for an image block with no altText at all", () => { + const image: ContentImageBlock = { + kind: "image", + format: "png", + base64: "AA==", + widthPt: 1, + heightPt: 1, + }; + expect(emitMarkdown(doc([image]))).toBe("![](data:image/png;base64,AA==)"); + }); +}); + +describe("round trip through src/lower", () => { + it("renders a code span run back as backticks and a plain autolink run back as ", () => { + const source = "`code` and "; + const lowered = lowerMarkdown(source); + expect(emitMarkdown(lowered)).toBe("`code` and "); }); it("preserves inline raw HTML as literal HTML, not escaped text, across a full lower -> emit -> lower round trip", () => { @@ -2907,6 +3532,37 @@ describe("link and image titles (the `link` construct annotation)", () => { ); }); + it("does NOT render the image-shortcut spelling for a link construct wrapping MORE than one child, even when the first of them is an image -- the mint condition is exactly one child, not merely 'starts with an image'", () => { + const collector = createDiagnosticCollector(); + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { + kind: "link", + target: { kind: "external", uri: "https://example.com/a.png" }, + }, + }, + { + kind: "image", + format: "png", + base64: "AAAA", + widthPt: 1, + heightPt: 1, + altText: "alt", + }, + { kind: "paragraph", runs: [{ text: "caption" }] }, + { kind: "constructEnd" }, + ]), + { sink: collector.sink }, + ); + // The construct falls through to the generic, transparent rendering -- its own image child renders as ITSELF (a plain data: URI image, not the link-shortcut's own remote-destination spelling), and the caption follows as an ordinary paragraph. + expect(markdown).toBe("![alt](data:image/png;base64,AAAA)\n\ncaption"); + expect(collector.has(MarkdownDiagnosticCodes.CONSTRUCT_UNREPRESENTED)).toBe( + true, + ); + }); + it("falls back to the plain no-bytes image rendering when the construct destination is itself a data: URI and images: false asks for no bytes", () => { const blocks: ContentBlock[] = [ { @@ -2931,7 +3587,8 @@ describe("link and image titles (the `link` construct annotation)", () => { }); it("throws for a paragraph whose run-level construct extent does not name real runs", () => { - expect(() => { + let beyondRuns: unknown; + try { emitMarkdown( doc([ { @@ -2951,8 +3608,22 @@ describe("link and image titles (the `link` construct annotation)", () => { }, ]), ); - }).toThrow(MarkdownInvalidRunConstructExtentError); - expect(() => { + } catch (error) { + beyondRuns = error; + } + expect(beyondRuns).toBeInstanceOf(MarkdownInvalidRunConstructExtentError); + const beyondRunsTyped = + beyondRuns as MarkdownInvalidRunConstructExtentError; + expect(beyondRunsTyped.name).toBe("MarkdownInvalidRunConstructExtentError"); + expect(beyondRunsTyped.faultKind).toBe("beyondRuns"); + expect(beyondRunsTyped.entryIndex).toBe(0); + expect(beyondRunsTyped.code).toBe("md/run-construct-extent-invalid"); + expect(beyondRunsTyped.message).toBe( + "a paragraph's run-level construct extent reaches outside the paragraph's own runs (constructs entry 0); a run extent must name real runs in 0..runs.length", + ); + + let invertedRange: unknown; + try { emitMarkdown( doc([ { @@ -2972,7 +3643,61 @@ describe("link and image titles (the `link` construct annotation)", () => { }, ]), ); - }).toThrow(/ends before it starts/); + } catch (error) { + invertedRange = error; + } + expect(invertedRange).toBeInstanceOf( + MarkdownInvalidRunConstructExtentError, + ); + const invertedRangeTyped = + invertedRange as MarkdownInvalidRunConstructExtentError; + expect(invertedRangeTyped.faultKind).toBe("invertedRange"); + expect(invertedRangeTyped.entryIndex).toBe(0); + expect(invertedRangeTyped.message).toBe( + "a paragraph's run-level construct extent ends before it starts (constructs entry 0); a run extent must name real runs in 0..runs.length", + ); + }); + + it("also throws for an invalid run-level construct extent buried inside a TABLE CELL's own paragraph, not just a top-level one -- validateRunConstructExtents must actually recurse into every row's every cell", () => { + const table: ContentTable = { + kind: "table", + columnWidthsPt: [100], + rows: [ + { + cells: [ + { + blocks: [ + { + kind: "paragraph", + runs: [{ text: "text", hyperlink: "/u" }], + constructs: [ + { + descriptor: { + kind: "link", + target: { kind: "external", uri: "/u" }, + title: "t", + }, + startRun: 0, + endRun: 5, + }, + ], + }, + ], + }, + ], + }, + ], + }; + let inCell: unknown; + try { + emitMarkdown(doc([table])); + } catch (error) { + inCell = error; + } + expect(inCell).toBeInstanceOf(MarkdownInvalidRunConstructExtentError); + expect((inCell as MarkdownInvalidRunConstructExtentError).faultKind).toBe( + "beyondRuns", + ); }); }); @@ -3060,6 +3785,11 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { expect(collector.has(MarkdownDiagnosticCodes.HEADING_LEVEL_CLAMPED)).toBe( true, ); + const diagnostic = collector.diagnostics.find( + (d) => d.code === MarkdownDiagnosticCodes.HEADING_LEVEL_CLAMPED, + ); + expect(diagnostic?.message).toContain("9"); + expect(diagnostic?.message).toContain("6"); }); it("ADJACENT_LINKS_MERGED fires when two consecutive runs share a hyperlink", () => { @@ -3108,6 +3838,26 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { ).toBe(true); }); + it("PARAGRAPH_INDENT_DROPPED also fires for a DEFINED but unrecognised styleId carrying indentLeftPt, not only an absent styleId -- isQuotableStyle's own QUOTABLE_STYLE_IDS/heading check must actually run, not just its undefined short-circuit", () => { + const collector = createDiagnosticCollector(); + const markdown = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "x" }], + styleId: "SomeUnrecognisedStyle", + indentLeftPt: 36, + }, + ]), + { sink: collector.sink }, + ); + expect(markdown).toBe("x"); + expect(markdown).not.toContain(">"); + expect( + collector.has(MarkdownDiagnosticCodes.PARAGRAPH_INDENT_DROPPED), + ).toBe(true); + }); + it("LIST_NUMID_FALLBACK fires for a numId this package never minted, falling back to a plain bullet", () => { const collector = createDiagnosticCollector(); const markdown = emitMarkdown( @@ -3124,26 +3874,56 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { expect(collector.has(MarkdownDiagnosticCodes.LIST_NUMID_FALLBACK)).toBe( true, ); + const diagnostic = collector.diagnostics.find( + (d) => d.code === MarkdownDiagnosticCodes.LIST_NUMID_FALLBACK, + ); + expect(diagnostic?.message).toContain("list1"); + expect(diagnostic?.message).toContain("not minted"); }); - it("LIST_NUMID_FALLBACK fires once for depth-only memberships with no numId, falling back to one tight plain-bullet list", () => { + it("LIST_NUMID_FALLBACK fires only once for two items sharing the SAME never-minted numId, not once per item", () => { const collector = createDiagnosticCollector(); const markdown = emitMarkdown( doc([ - { kind: "paragraph", runs: [{ text: "x" }], list: { level: 0 } }, - { kind: "paragraph", runs: [{ text: "y" }], list: { level: 1 } }, + { + kind: "paragraph", + runs: [{ text: "a" }], + list: { numId: "list1", level: 0 }, + }, + { + kind: "paragraph", + runs: [{ text: "b" }], + list: { numId: "list1", level: 0 }, + }, ]), { sink: collector.sink }, ); - expect(markdown).toBe("- x\n - y"); + expect(markdown).toBe("- a\n- b"); expect( collector.diagnostics.filter( - (diagnostic) => - diagnostic.code === MarkdownDiagnosticCodes.LIST_NUMID_FALLBACK, + (d) => d.code === MarkdownDiagnosticCodes.LIST_NUMID_FALLBACK, ), ).toHaveLength(1); }); + it("LIST_NUMID_FALLBACK fires once for depth-only memberships with no numId, falling back to one tight plain-bullet list", () => { + const collector = createDiagnosticCollector(); + const markdown = emitMarkdown( + doc([ + { kind: "paragraph", runs: [{ text: "x" }], list: { level: 0 } }, + { kind: "paragraph", runs: [{ text: "y" }], list: { level: 1 } }, + ]), + { sink: collector.sink }, + ); + expect(markdown).toBe("- x\n - y"); + const fallbacks = collector.diagnostics.filter( + (diagnostic) => + diagnostic.code === MarkdownDiagnosticCodes.LIST_NUMID_FALLBACK, + ); + expect(fallbacks).toHaveLength(1); + expect(fallbacks[0]?.message).toContain("no numId"); + }); + it("TABLE_CELL_FORMATTING_DROPPED fires for a non-paragraph/non-image/non-lone-nested-table cell block even inside the HTML-table fallback, once colSpan already triggers it", () => { const collector = createDiagnosticCollector(); const table: ContentTable = { @@ -3169,6 +3949,14 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { expect(collector.has(MarkdownDiagnosticCodes.TABLE_HTML_FALLBACK)).toBe( true, ); + expect( + collector.diagnostics.find( + (diagnostic) => + diagnostic.code === MarkdownDiagnosticCodes.TABLE_HTML_FALLBACK, + )?.message, + ).toBe( + "a cell in this table needs colSpan/rowSpan/background, or holds a block a GFM table cell cannot represent at all (most commonly a nested table); GFM's own table extension holds inline content only (github.github.com/gfm, \"Tables (extension)\"), so no single cell can carry an HTML sub-block inside an otherwise pipe-syntax table -- the whole table is rendered as a raw HTML block instead (CommonMark spec 0.31.2, HTML blocks condition 6, https://spec.commonmark.org/0.31.2/#html-blocks), which src/html/html-table.ts's own reader recognises back into an equal ContentTable", + ); expect( collector.has(MarkdownDiagnosticCodes.TABLE_CELL_FORMATTING_DROPPED), ).toBe(true); @@ -3198,6 +3986,33 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { expect( collector.has(MarkdownDiagnosticCodes.TABLE_CELL_MULTI_PARAGRAPH_JOINED), ).toBe(true); + expect( + collector.diagnostics.find( + (diagnostic) => + diagnostic.code === + MarkdownDiagnosticCodes.TABLE_CELL_MULTI_PARAGRAPH_JOINED, + )?.message, + ).toBe( + "a table cell with 2 blocks has no multi-paragraph equivalent in a GFM table cell; their own rendered text is joined with a literal
line break", + ); + }); + + it("does not fire TABLE_CELL_MULTI_PARAGRAPH_JOINED for a cell with exactly one block", () => { + const collector = createDiagnosticCollector(); + const table: ContentTable = { + kind: "table", + columnWidthsPt: [100], + rows: [ + { cells: [{ blocks: [{ kind: "paragraph", runs: [{ text: "h" }] }] }] }, + { + cells: [{ blocks: [{ kind: "paragraph", runs: [{ text: "one" }] }] }], + }, + ], + }; + emitMarkdown(doc([table]), { sink: collector.sink }); + expect( + collector.has(MarkdownDiagnosticCodes.TABLE_CELL_MULTI_PARAGRAPH_JOINED), + ).toBe(false); }); it("TABLE_CELL_IMAGE_DEGRADED fires for an image-kind cell block, which emits inline rather than being dropped", () => { @@ -3232,11 +4047,40 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { expect( collector.has(MarkdownDiagnosticCodes.TABLE_CELL_IMAGE_DEGRADED), ).toBe(true); + expect( + collector.diagnostics.find( + (diagnostic) => + diagnostic.code === MarkdownDiagnosticCodes.TABLE_CELL_IMAGE_DEGRADED, + )?.message, + ).toBe( + "a table cell's own image block has no GFM table equivalent; it emits inline instead, degrading on read-back to a run carrying the alt text with the image's data as that run's hyperlink", + ); expect( collector.has(MarkdownDiagnosticCodes.TABLE_CELL_FORMATTING_DROPPED), ).toBe(false); }); + it("joins a cell's own paragraphs skipping any that render to empty text, without an extra
", () => { + const table: ContentTable = { + kind: "table", + columnWidthsPt: [100], + rows: [ + { cells: [{ blocks: [{ kind: "paragraph", runs: [{ text: "h" }] }] }] }, + { + cells: [ + { + blocks: [ + { kind: "paragraph", runs: [] }, + { kind: "paragraph", runs: [{ text: "one" }] }, + ], + }, + ], + }, + ], + }; + expect(emitMarkdown(doc([table]))).toContain("| one |"); + }); + it("round-trips a table cell image as a run carrying the alt text with the image's own data as that run's hyperlink, the same shape a nested image inside emphasis/a link already degrades to", () => { const table: ContentTable = { kind: "table", @@ -3314,3 +4158,321 @@ describe("gaps (MarkdownDiagnosticCodes)", () => { expect(text).toBe("one
two"); }); }); + +describe("quoteDepthOf, longestRunLength, and leadingIndentColumns boundaries", () => { + it("treats indentLeftPt: 0 the same as no indentLeftPt at all -- no quote depth, no '>' prefix", () => { + expect( + emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: "x" }], + styleId: "Quote", + indentLeftPt: 0, + }, + ]), + ), + ).toBe("x"); + }); + + it("resets the fence-character run counter after a non-fence character interrupts it, rather than compounding the interrupted run's own length into a later run of the SAME length as if nothing had broken it", () => { + // The genuine longest run of '`' here is 4 (the second one); a counter that failed to reset after 'xxx' would instead carry the first run's own length of 3 into the second, overcounting to 7 and picking an unnecessarily long fence. + const literal = "```xxx````"; + expect( + emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [{ text: literal }], + styleId: "CodeBlock", + }, + ]), + ), + ).toBe(`\`\`\`\`\`\n${literal}\n\`\`\`\`\``); + }); + + it("expands a leading tab to the correct tab-stop-aligned column count, not merely to SOME value past the 4-column indented-code-block threshold, when the tab is not the first character of the line", () => { + // Two leading spaces (column 2) then a tab: the correct tab-stop rule rounds up to the NEXT multiple of 4, landing on column 4 (2 + 2) -- exactly at, not past, CODE_INDENT_COLUMNS. A `%` -> `*` mutation of the tab-stop arithmetic computes 2 + (4 - 2*4) = 2 + -4 = -2 instead, which is NOT >= 4 and would wrongly let this promote to setext. + const collector = createDiagnosticCollector(); + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [ + { + text: " \tx", + source: { format: "markdown", xml: " \tx" }, + }, + ], + styleId: "Heading1", + }, + ]), + { headingStyle: "setext", sink: collector.sink }, + ); + expect(written).toBe("# \tx"); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, + ), + ).toBe(true); + }); + + it("stops counting leading indentation at the first non-space, non-tab character, rather than resuming the count at a LATER space in the same line as if it were still leading", () => { + // Correct: 'a' immediately stops the leading-indent scan at column 0 (well under the 4-column threshold), so this promotes safely to setext. A dropped `break` would instead skip over 'a' and keep scanning, picking up the run of 4 spaces that follows it as if it were still leading indentation, reaching column 4 and wrongly refusing the promotion. + const collector = createDiagnosticCollector(); + const written = emitMarkdown( + doc([ + { kind: "paragraph", runs: [{ text: "a x" }], styleId: "Heading1" }, + ]), + { headingStyle: "setext", sink: collector.sink }, + ); + expect(written).toBe("a x\n======"); + expect( + collector.has( + MarkdownDiagnosticCodes.HEADING_LINE_BREAK_UNSAFE_FOR_SETEXT, + ), + ).toBe(false); + }); + + it("does not treat a heading's own SECOND line, indented 4+ columns, as an interrupting construct -- CommonMark absorbs indented content as ordinary paragraph continuation, exactly like its own indented-code paragraph-interruption exception requires", () => { + // " - item" would itself match parseListMarker if the leading 4-column indent were not first exempted -- indented content is absorbed as continuation instead, so this must still promote safely to setext rather than being refused as an interrupting list marker. + const written = emitMarkdown( + doc([ + { + kind: "paragraph", + runs: [ + { text: "foo" }, + { text: "\n" }, + { + text: " - item", + source: { format: "markdown", xml: " - item" }, + }, + ], + styleId: "Heading1", + }, + ]), + ); + expect(written).toBe("foo\\\n - item\n===="); + + const reparsed = lowerMarkdown(written); + if (reparsed.kind !== "wordprocessing") { + throw new Error("expected a wordprocessing ContentDocument"); + } + const [headingBlock] = reparsed.sections[0]?.blocks ?? []; + if (headingBlock?.kind !== "paragraph") { + throw new Error("expected a paragraph block"); + } + expect(headingBlock.styleId).toBe("Heading1"); + }); +}); + +describe("renderConstruct's own unrepresentable shapes", () => { + it("reports CONSTRUCT_UNREPRESENTED, with the invalid label named, for a footnote anchor whose name cannot be spelled as a [^label]: marker", () => { + const collector = createDiagnosticCollector(); + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { + kind: "anchor", + anchorType: "footnote", + name: "bad label", + }, + }, + { kind: "paragraph", runs: [{ text: "body" }] }, + { kind: "constructEnd" }, + ]), + { sink: collector.sink }, + ); + expect(markdown).toBe("body"); + const diagnostic = collector.diagnostics.find( + (d) => d.code === MarkdownDiagnosticCodes.CONSTRUCT_UNREPRESENTED, + ); + expect(diagnostic?.message).toContain("bad label"); + expect(diagnostic?.message).toContain("footnote"); + }); + + it("reports CONSTRUCT_UNREPRESENTED with 'anchor (bookmark)' as the detail for a non-footnote anchor, distinguishing it from a bare 'anchor'", () => { + const collector = createDiagnosticCollector(); + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { kind: "anchor", anchorType: "bookmark", name: "b1" }, + }, + { kind: "paragraph", runs: [{ text: "body" }] }, + { kind: "constructEnd" }, + ]), + { sink: collector.sink }, + ); + expect(markdown).toBe("body"); + const diagnostic = collector.diagnostics.find( + (d) => d.code === MarkdownDiagnosticCodes.CONSTRUCT_UNREPRESENTED, + ); + expect(diagnostic?.message).toContain("anchor (bookmark)"); + }); + + it("does not leave an extra blank-line gap for a CONSTRUCT that renders to nothing at all, such as a bodyless footnote anchor (a point anchor with an empty extent) sitting between two paragraphs", () => { + const markdown = emitMarkdown( + doc([ + { kind: "paragraph", runs: [{ text: "a" }] }, + { + kind: "constructStart", + descriptor: { kind: "anchor", anchorType: "bookmark", name: "empty" }, + }, + { kind: "constructEnd" }, + { kind: "paragraph", runs: [{ text: "b" }] }, + ]), + ); + expect(markdown).toBe("a\n\nb"); + }); + + it("does not leave an extra blank-line gap for a top-level block that renders to nothing at all, such as a page break sitting between two paragraphs", () => { + const markdown = emitMarkdown( + doc([ + { kind: "paragraph", runs: [{ text: "a" }] }, + { kind: "pageBreak" }, + { kind: "paragraph", runs: [{ text: "b" }] }, + ]), + ); + // Exactly one blank line between "a" and "b" -- not two, which pushing the page break's own empty string into the joined parts array would produce. + expect(markdown).toBe("a\n\nb"); + }); + + it("does not double-count a division-wrapped paragraph's own indentLeftPt as additional quote depth on top of the division's own '> ' wrapping", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { kind: "division", name: "d1" }, + }, + { + kind: "paragraph", + runs: [{ text: "x" }], + styleId: "Quote", + indentLeftPt: 72, + }, + { kind: "constructEnd" }, + ]), + ); + // Exactly one level of '> ' from the division itself -- NOT '> > x', which double-counting the paragraph's own indentLeftPt (72pt, two quote levels' worth) on top of the division's own wrapping would produce. + expect(markdown).toBe("> x"); + }); + + it("restores divisionDepth to its own PRIOR value once a division closes, rather than leaking an elevated depth into whatever renders after it", () => { + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { kind: "division", name: "d1" }, + }, + { + kind: "paragraph", + runs: [{ text: "x" }], + styleId: "Quote", + indentLeftPt: 36, + }, + { kind: "constructEnd" }, + { + kind: "paragraph", + runs: [{ text: "y" }], + styleId: "Quote", + indentLeftPt: 36, + }, + ]), + ); + // A STANDALONE paragraph after the division closes must recover its own '> ' from indentLeftPt alone -- a decrement that failed to restore divisionDepth to 0 would leave this second paragraph's own quote prefix wrongly suppressed, rendering plain "y" instead of "> y". + expect(markdown).toBe("> x\n\n> y"); + }); + + it("still re-embeds a data: URI destination for a link construct wrapping exactly one image when images is left at its own default (true), rather than always falling back to the no-bytes rendering", () => { + const dataUri = "data:image/png;base64,AAAA"; + const markdown = emitMarkdown( + doc([ + { + kind: "constructStart", + descriptor: { + kind: "link", + target: { kind: "external", uri: dataUri }, + }, + }, + { + kind: "image", + format: "png", + base64: "AAAA", + widthPt: 1, + heightPt: 1, + altText: "alt", + }, + { kind: "constructEnd" }, + ]), + ); + expect(markdown).toBe(`![alt](${dataUri})`); + }); +}); + +describe("emitMarkdown's own top-level assembly", () => { + it("joins multiple sections with a blank line, not concatenating them directly", () => { + const document: ContentDocument = { + kind: "wordprocessing", + metadata: {}, + sections: [ + { + pageSize: PAGE_SIZE_A4, + margins: DEFAULT_MARGINS, + blocks: [{ kind: "paragraph", runs: [{ text: "first" }] }], + }, + { + pageSize: PAGE_SIZE_A4, + margins: DEFAULT_MARGINS, + blocks: [{ kind: "paragraph", runs: [{ text: "second" }] }], + }, + ], + }; + expect(emitMarkdown(document)).toBe("first\n\nsecond"); + }); + + it("prepends a YAML front matter block, separated from the body by a blank line, when frontMatter: true and the metadata carries a field it can emit", () => { + const document: ContentDocument = { + kind: "wordprocessing", + metadata: { title: "My Title" }, + sections: [ + { + pageSize: PAGE_SIZE_A4, + margins: DEFAULT_MARGINS, + blocks: [{ kind: "paragraph", runs: [{ text: "body" }] }], + }, + ], + }; + expect(emitMarkdown(document, { frontMatter: true })).toBe( + "---\ntitle: My Title\n---\n\nbody", + ); + }); + + it("emits no front matter block at all when frontMatter is not requested, even though the metadata carries a field emitFrontMatter could have emitted", () => { + const document: ContentDocument = { + kind: "wordprocessing", + metadata: { title: "My Title" }, + sections: [ + { + pageSize: PAGE_SIZE_A4, + margins: DEFAULT_MARGINS, + blocks: [{ kind: "paragraph", runs: [{ text: "body" }] }], + }, + ], + }; + expect(emitMarkdown(document)).toBe("body"); + }); + + it("rewrites every line ending to CRLF when lineEnding: 'crlf' is requested", () => { + expect( + emitMarkdown( + doc([ + { kind: "paragraph", runs: [{ text: "first" }] }, + { kind: "paragraph", runs: [{ text: "second" }] }, + ]), + { lineEnding: "crlf" }, + ), + ).toBe("first\r\n\r\nsecond"); + }); +}); diff --git a/packages/markdown-codec/src/emit/emit.ts b/packages/markdown-codec/src/emit/emit.ts index eee36d819..57a587462 100644 --- a/packages/markdown-codec/src/emit/emit.ts +++ b/packages/markdown-codec/src/emit/emit.ts @@ -54,7 +54,6 @@ import { } from "../defaults/defaults"; import { matchHtmlBlockStart } from "../html/html"; import { isValidFootnoteLabel } from "../inline/footnote"; -import { MARKDOWN_TAB_STOP_WIDTH } from "../scan/scan"; import type { MarkdownHeadingStyle, WriteMarkdownOptions, @@ -117,10 +116,19 @@ const SETEXT_LEVEL_1_CHAR = "="; const SETEXT_LEVEL_2_CHAR = "-"; const MIN_SETEXT_UNDERLINE_LENGTH = 1; +// String.prototype.split never returns an empty array for any input, even the empty string ("".split(x) === [""]) -- so a split result's own first line is always genuinely present. Returning a tuple type here, rather than a plain string[], lets every call site destructure or index its own first element directly: TypeScript already knows a tuple's fixed leading position is defined regardless of noUncheckedIndexedAccess, so no call site needs a dead "?? ''"/"= ''" fallback for a branch this invariant guarantees it can never actually take. The non-null assertion below is the one place that invariant is asserted, rather than repeated at every call site. +function splitLines( + text: string, + pattern: string | RegExp, +): readonly [string, ...string[]] { + const [first, ...rest] = text.split(pattern); + return [first!, ...rest]; +} + function renderSetextHeading(level: number, text: string): string { const underlineChar = level === 1 ? SETEXT_LEVEL_1_CHAR : SETEXT_LEVEL_2_CHAR; // A setext underline's own length has no semantic meaning beyond "one or more" -- matching the heading text's own rendered length keeps the output visually tidy without claiming any significance for the exact count, so a CR- or CRLF-delimited first line (LINE_ENDING_PATTERN, not a bare '\n' split) still measures the SAME first line the rest of this module's own line-ending-aware checks agree on, rather than treating the whole multi-line text as a single "line" whenever its own first break is not an LF. - const firstLine = text.split(LINE_ENDING_PATTERN)[0] ?? ""; + const [firstLine] = splitLines(text, LINE_ENDING_PATTERN); const underline = underlineChar.repeat( Math.max(MIN_SETEXT_UNDERLINE_LENGTH, firstLine.length), ); @@ -168,20 +176,13 @@ function isQuotableStyle(styleId: string | undefined): boolean { ); } -// Whether a rendered block of this styleId closes itself unambiguously -- so a non-blank line immediately following it is always scanned by a reparse as a FRESH block rather than being absorbed backward into this one as ordinary continuation text. This is the "safe as PREVIOUS" half of requiresBlankLineBefore's compound check below, and unlike canInterruptOpenParagraph it does not depend on any emit option: a fenced code block and a math block each close at their own explicit closing delimiter, a thematic break and an ATX heading are each a single complete line, and a SETEXT heading's own underline line closes it exactly as definitively -- nothing can lazily continue a heading once its underline has been read, so the setext spelling is only unsafe on the OTHER side, as something that ITSELF follows an open paragraph (see canInterruptOpenParagraph). Deliberately false for QUOTE_STYLE_ID (renders through the same prefix-free renderParagraphBody as a plain paragraph here, so carries no boundary of its own) and for HTML_PREFORMATTED_STYLE_ID (this package re-emits raw HTML as a bare literal with no record of which CommonMark HTML-block start condition produced it, and several of those seven conditions close only at a blank line -- with no closing condition of its own to fall back to, anything following without one keeps being read as more of the same literal HTML content). +// Whether a rendered block of this styleId closes itself unambiguously -- so a non-blank line immediately following it is always scanned by a reparse as a FRESH block rather than being absorbed backward into this one as ordinary continuation text. This is the "safe as PREVIOUS" half of requiresBlankLineBefore's compound check below, and unlike canInterruptOpenParagraph it does not depend on any emit option: a fenced code block and a math block each close at their own explicit closing delimiter, a thematic break and an ATX heading are each a single complete line, and a SETEXT heading's own underline line closes it exactly as definitively -- nothing can lazily continue a heading once its underline has been read, so the setext spelling is only unsafe on the OTHER side, as something that ITSELF follows an open paragraph (see canInterruptOpenParagraph). False for QUOTE_STYLE_ID (renders through the same prefix-free renderParagraphBody as a plain paragraph here, so carries no boundary of its own) and for HTML_PREFORMATTED_STYLE_ID (this package re-emits raw HTML as a bare literal with no record of which CommonMark HTML-block start condition produced it, and several of those seven conditions close only at a blank line -- with no closing condition of its own to fall back to, anything following without one keeps being read as more of the same literal HTML content) -- neither needs its own explicit branch, since neither matches any of the four positive checks below either, so both already fall out to false on their own. function terminatesCleanly(styleId: string | undefined): boolean { - if ( - styleId === undefined || - styleId === QUOTE_STYLE_ID || - styleId === HTML_PREFORMATTED_STYLE_ID - ) { - return false; - } return ( styleId === CODE_BLOCK_STYLE_ID || styleId === MATH_BLOCK_STYLE_ID || styleId === HORIZONTAL_RULE_STYLE_ID || - parseHeadingStyleId(styleId) !== undefined + (styleId !== undefined && parseHeadingStyleId(styleId) !== undefined) ); } @@ -197,19 +198,16 @@ function firstContentLineIndex(text: string): number { .findIndex((line) => !BLANK_OR_WHITESPACE_ONLY_LINE.test(line)); } -// The column width of a line's own leading run of spaces and tabs, expanded per CommonMark's own tab-stop rule (spec 0.31.2, "Tabs": "in contexts where spaces help to define block structure, tabs behave as if they were replaced by spaces with a tab stop of 4 characters", counted from the start of the LINE, not the whole document). Shares MARKDOWN_TAB_STOP_WIDTH with src/scan/scan.ts's own MarkdownScanCursor so a tab's width agrees with the read side's parse of the very text this function is predicting the reparse of. -function leadingIndentColumns(line: string): number { +// Whether a line's own leading run of spaces and tabs reaches CommonMark's own 4-column indented-code-block threshold (spec 0.31.2, "Tabs": "in contexts where spaces help to define block structure, tabs behave as if they were replaced by spaces with a tab stop of 4 characters", counted from the start of the LINE, not the whole document). Shares MARKDOWN_TAB_STOP_WIDTH with src/scan/scan.ts's own MarkdownScanCursor so a tab's width agrees with the read side's parse of the very text this function is predicting the reparse of. The sole caller below only ever asks a >= CODE_INDENT_COLUMNS boundary question, never the exact column count beyond it, so this returns that boundary directly. Once the leading run of plain spaces ends, only the SINGLE character right after it can still change the answer: a tab there is always itself sufficient to reach the threshold (CODE_INDENT_COLUMNS <= MARKDOWN_TAB_STOP_WIDTH means expanding a tab from any column short of the threshold already lands exactly on it), and anything else stops the leading run outright -- so this needs no loop-exhausted fallback the way a step-by-step scan through every remaining character would: `line[column]` reads as `undefined` past the string's own end, which compares unequal to "\t" exactly as a real non-tab character would. +function leadingIndentReachesCodeThreshold(line: string): boolean { let column = 0; - for (const char of line) { - if (char === " ") { - column += 1; - } else if (char === "\t") { - column += MARKDOWN_TAB_STOP_WIDTH - (column % MARKDOWN_TAB_STOP_WIDTH); - } else { - break; - } + while (column < line.length && line[column] === " ") { + column += 1; + } + if (column >= CODE_INDENT_COLUMNS) { + return true; } - return column; + return line[column] === "\t"; } // CommonMark's own list-item grammar (spec 0.31.2, section 5.2 "List items"): "A list item can begin with at most one blank line." -- the bound the leading-run exemption above is held to, applied universally regardless of which context (top-level, blockquote, list item) the heading being checked is actually about to render through, since this function cannot see that and the bound is harmless where it is not strictly required. @@ -275,7 +273,7 @@ function unsafeSetextBreakReason(text: string): UnsafeSetextBreakReason { continue; } sawContentLine = true; - if (leadingIndentColumns(line) >= CODE_INDENT_COLUMNS) { + if (leadingIndentReachesCodeThreshold(line)) { return "leading-indentation"; } if (interruptsSetextParagraph(line, true)) { @@ -349,11 +347,8 @@ function canInterruptOpenParagraph( context: EmitContext, ): boolean { const styleId = paragraph.styleId; - if ( - styleId === undefined || - styleId === QUOTE_STYLE_ID || - styleId === HTML_PREFORMATTED_STYLE_ID - ) { + // Undefined needs its own early return purely so parseHeadingStyleId below gets a definite string -- QUOTE_STYLE_ID and HTML_PREFORMATTED_STYLE_ID need no explicit check of their own alongside it, since neither matches any of the positive branches below (parseHeadingStyleId included), so both already fall out to the final `return false` on their own. + if (styleId === undefined) { return false; } if (styleId === CODE_BLOCK_STYLE_ID || styleId === MATH_BLOCK_STYLE_ID) { @@ -607,10 +602,10 @@ function toEmitItem(item: ListRegionItem): EmitItem { return item.kind === "paragraph" ? { block: item.block } : item.item; } -// One item's first-block preparation: the checkbox text its marker line carries, and whether that block's own leading run is a legacy checkbox glyph that must be stripped from the body. The membership's own checked field is the current spelling and needs no task-flagged numId behind it; the glyph sniff is gated on the numId's task flag AND on the first block actually being a paragraph (a construct has no runs of its own to sniff a glyph from), so an ordinary item whose text happens to begin with a ballot-box glyph is never misread as a checkbox. +// One item's first-block preparation: the checkbox text its marker line carries, and the SAME paragraph with any legacy checkbox glyph run already stripped out of it, when one was found. The membership's own checked field is the current spelling and needs no task-flagged numId behind it; the glyph sniff is gated on the numId's task flag AND on the first block actually being a paragraph (a construct has no runs of its own to sniff a glyph from), so an ordinary item whose text happens to begin with a ballot-box glyph is never misread as a checkbox. Doing the strip here, once, rather than returning a separate "please strip" boolean for listRegionItemBody to act on later, means no second site ever needs to re-derive from the run text whether stripping applies -- the one place that already found the glyph is the one place that removes it. interface FirstBlockCheckbox { readonly checkboxText: string; - readonly stripGlyph: boolean; + readonly strippedFirstBlock: ContentParagraph | undefined; } function firstBlockCheckbox( @@ -620,20 +615,26 @@ function firstBlockCheckbox( if (first.list.checked !== undefined) { return { checkboxText: first.list.checked ? "[x] " : "[ ] ", - stripGlyph: false, + strippedFirstBlock: undefined, }; } if (!taskNumId || first.kind !== "paragraph") { - return { checkboxText: "", stripGlyph: false }; + return { checkboxText: "", strippedFirstBlock: undefined }; } const leading = first.block.runs[0]?.text ?? ""; if (leading.startsWith(`${TASK_CHECKBOX_CHECKED} `)) { - return { checkboxText: "[x] ", stripGlyph: true }; + return { + checkboxText: "[x] ", + strippedFirstBlock: stripCheckboxRun(first.block), + }; } if (leading.startsWith(`${TASK_CHECKBOX_UNCHECKED} `)) { - return { checkboxText: "[ ] ", stripGlyph: true }; + return { + checkboxText: "[ ] ", + strippedFirstBlock: stripCheckboxRun(first.block), + }; } - return { checkboxText: "", stripGlyph: false }; + return { checkboxText: "", strippedFirstBlock: undefined }; } interface RenderedListMarker { @@ -721,7 +722,8 @@ function consumeSameItemRun( itemId: string, ): number { let end = from; - while (end < items.length) { + // No separate `end < items.length` bound: `items[end]` running off the end already returns undefined, which the very next check below catches and breaks on -- an explicit length comparison here would be redundant with that undefined check on every real input, never independently true or false. + for (;;) { const candidate = items[end]; if (candidate?.list.level !== level || candidate.list.itemId !== itemId) { break; @@ -750,7 +752,8 @@ function collectListItem( for (;;) { let nestedEnd = index; - while (nestedEnd < items.length) { + // No separate `nestedEnd < items.length` bound: `items[nestedEnd]` running off the end already yields `candidateLevel === undefined`, which the check below already breaks on. + for (;;) { const candidateLevel = items[nestedEnd]?.list.level; if (candidateLevel === undefined || candidateLevel <= level) { break; @@ -766,10 +769,8 @@ function collectListItem( if (itemId === undefined) { break; } + // No separate "did anything actually resume?" check: when nothing does, resumedEnd stays equal to index, so this pushes a harmless empty "own" segment (segments.push/segment.blocks are never read for their COUNT, only segments[0] and each segment's own blocks) and the loop's own nested-run check above terminates it on the very next pass, since index is unchanged from this one. const resumedEnd = consumeSameItemRun(items, index, level, itemId); - if (resumedEnd === index) { - break; - } segments.push({ kind: "own", blocks: items.slice(index, resumedEnd) }); index = resumedEnd; } @@ -811,17 +812,14 @@ function lastStyleIdOfRegionItem(item: ListRegionItem): string | undefined { return lastStyleIdOf(toEmitItem(item)); } -// One list-region item's own rendered body, with no marker/indent applied yet. A plain paragraph renders through renderParagraphBody exactly as before (optionally with its checkbox glyph stripped); a construct renders through renderConstruct -- the SAME function renderItems reaches for a construct that is NOT part of any list region, so a construct's own markdown spelling never diverges depending on whether it happens to sit inside a list item, EXCEPT for context.enclosingItemId, set here for the duration of that one call: it is what lets renderItems' own recursive walk over the construct's children tell inherited pass-through membership (this exact item, see EmitContext's own field comment) apart from a genuinely fresh nested list. +// One list-region item's own rendered body, with no marker/indent applied yet. A plain paragraph renders through renderParagraphBody exactly as before (using `overrideParagraph` in place of the item's own block when the caller already prepared a checkbox-glyph-stripped version, per firstBlockCheckbox above); a construct renders through renderConstruct -- the SAME function renderItems reaches for a construct that is NOT part of any list region, so a construct's own markdown spelling never diverges depending on whether it happens to sit inside a list item, EXCEPT for context.enclosingItemId, set here for the duration of that one call: it is what lets renderItems' own recursive walk over the construct's children tell inherited pass-through membership (this exact item, see EmitContext's own field comment) apart from a genuinely fresh nested list. function listRegionItemBody( item: ListRegionItem, context: EmitContext, - stripGlyph: boolean, + overrideParagraph: ContentParagraph | undefined, ): string { if (item.kind === "paragraph") { - return renderParagraphBody( - stripGlyph ? stripCheckboxRun(item.block) : item.block, - context, - ); + return renderParagraphBody(overrideParagraph ?? item.block, context); } const previousEnclosingItemId = context.enclosingItemId; context.enclosingItemId = item.list.itemId; @@ -854,7 +852,8 @@ function renderListRegion( let index = 0; // The immediately preceding numId's own resolved type/glyph, local to this call (never read across a recursive call into a nested sub-list, or across a separate top-level renderListRegion call) -- exactly the scope resolveListGlyph's own collision check needs: two lists are only a genuine ADJACENCY risk when nothing else renders between them, which is precisely what "both sit in the SAME renderListRegion call's own items array" already guarantees. Left unset (and never consulted) for a depth-only membership (numId undefined, the cross-format shape LIST_NUMID_FALLBACK already documents) -- a rare cross-format edge case this glyph-alternation scheme does not extend to. let previousSibling: ListSiblingSignature | undefined; - while (index < items.length) { + // No separate `index < items.length` bound: `items[index]` running off the end already returns undefined, which the very next check breaks on. + for (;;) { const item = items[index]; if (item === undefined) { break; @@ -863,11 +862,10 @@ function renderListRegion( const info = listInfoFor(numId, context); const loose = info?.loose === true; const type = info?.type ?? "bullet"; + // A depth-only membership (numId undefined) always resolves through listInfoFor's OWN undefined-numId branch, which never returns real ListNumIdInfo -- so `type` above is always its own "bullet" default here, and `type === "ordered"` can never be true in this branch specifically; only the numId-carrying side ever sees a genuinely ordered type. const glyph = numId === undefined - ? type === "ordered" - ? context.orderedDelimiter - : context.bulletMarker + ? context.bulletMarker : resolveListGlyph(numId, type, previousSibling, context); if (numId !== undefined) { previousSibling = { numId, type, glyph }; @@ -878,7 +876,7 @@ function renderListRegion( if (first === undefined) { break; } - const { checkboxText, stripGlyph } = firstBlockCheckbox( + const { checkboxText, strippedFirstBlock } = firstBlockCheckbox( first, info?.task === true, ); @@ -911,12 +909,10 @@ function renderListRegion( } for (const block of segment.blocks) { if (!renderedFirstLine) { - const bodyLines = listRegionItemBody( - block, - context, - stripGlyph, - ).split("\n"); - const [firstLine = "", ...restLines] = bodyLines; + const [firstLine, ...restLines] = splitLines( + listRegionItemBody(block, context, strippedFirstBlock), + "\n", + ); text = [ `${marker.full}${firstLine}`, ...restLines.map((line) => `${indent}${line}`), @@ -925,7 +921,7 @@ function renderListRegion( previousStyleId = lastStyleIdOfRegionItem(block); continue; } - const rendered = listRegionItemBody(block, context, false) + const rendered = listRegionItemBody(block, context, undefined) .split("\n") .map((line) => (line.length === 0 ? line : `${indent}${line}`)) .join("\n"); @@ -994,7 +990,8 @@ function groupConstructItems( ): { readonly items: EmitItem[]; readonly next: number } { const items: EmitItem[] = []; let index = start; - while (index < blocks.length) { + // No separate `index < blocks.length` bound: `blocks[index]` running off the end already returns undefined, which the very next check breaks on. + for (;;) { const block = blocks[index]; if (block === undefined) { break; @@ -1024,7 +1021,7 @@ function renderFootnoteDefinition(name: string, body: string): string { return marker; } const indent = " ".repeat(FOOTNOTE_CONTINUATION_INDENT); - const [firstLine = "", ...restLines] = body.split("\n"); + const [firstLine, ...restLines] = splitLines(body, "\n"); return [ `${marker} ${firstLine}`, ...restLines.map((line) => (line.length === 0 ? line : `${indent}${line}`)), @@ -1048,17 +1045,15 @@ function renderConstruct(item: ConstructItem, context: EmitContext): string { }); return body; } - if (descriptor.kind === "division") { - // The blockquote spelling is gated on this package's own dual carry, not on the descriptor alone -- see isMaterialisedDivision above for exactly what that gate checks and why. A division whose paragraphs carry no such indent is a FOREIGN one -- an ODF text:section, a tagged-PDF /Sect -- and renders transparently below: a named section is not a markdown blockquote, and rendering it as one would invent a construct the source never had. - if (isMaterialisedDivision(item)) { - context.divisionDepth += 1; - const body = renderItems(item.children, context); - context.divisionDepth -= 1; - return body - .split("\n") - .map((line) => (line.length === 0 ? ">" : `> ${line}`)) - .join("\n"); - } + // The blockquote spelling is gated on this package's own dual carry, not on the descriptor kind alone -- see isMaterialisedDivision above for exactly what that gate checks and why. A division whose paragraphs carry no such indent is a FOREIGN one -- an ODF text:section, a tagged-PDF /Sect -- and renders transparently below: a named section is not a markdown blockquote, and rendering it as one would invent a construct the source never had. No separate `descriptor.kind === "division"` guard here: isMaterialisedDivision's own first check already tests that, so a non-division descriptor is refused there regardless, making an outer duplicate of the same check redundant. + if (isMaterialisedDivision(item)) { + context.divisionDepth += 1; + const body = renderItems(item.children, context); + context.divisionDepth -= 1; + return body + .split("\n") + .map((line) => (line.length === 0 ? ">" : `> ${line}`)) + .join("\n"); } if (descriptor.kind === "link" && descriptor.target.kind === "external") { // The mint condition is exact -- a pair around precisely one image block, the shape this package's own read side mints. A link construct of any other shape (an annotated block extent from another codec, a run-level pair flattened into a block list) renders transparently below rather than being guessed at. @@ -1127,7 +1122,8 @@ function isInheritedListMembership( function renderItems(items: readonly EmitItem[], context: EmitContext): string { const parts: string[] = []; let index = 0; - while (index < items.length) { + // No separate `index < items.length` bound: `items[index]` running off the end already returns undefined, which the very next check breaks on. + for (;;) { const item = items[index]; if (item === undefined) { break; @@ -1227,10 +1223,10 @@ function emitBlocks( return renderItems(groupConstructItems(blocks, 0).items, context); } -// A paragraph's run-level construct extents must name real runs before anything renders them -- the run-level twin of the marker-balance check above, through document-schema.js's own findRunConstructFault so every codec and consumer agree on one definition of well-formed. Tables are walked into because a cell's block list holds its own paragraphs (and nothing else descends further: a table inside a table cell is not a shape GFM or this model produces). +// A paragraph's run-level construct extents must name real runs before anything renders them -- the run-level twin of the marker-balance check above, through document-schema.js's own findRunConstructFault so every codec and consumer agree on one definition of well-formed. Tables are walked into because a cell's block list holds its own paragraphs (and nothing else descends further: a table inside a table cell is not a shape GFM or this model produces). No separate `block.constructs !== undefined` guard here: findRunConstructFault already checks that itself and returns undefined immediately, so a paragraph with no constructs at all is exactly as safe to pass through unconditionally. function validateRunConstructExtents(blocks: readonly ContentBlock[]): void { for (const block of blocks) { - if (block.kind === "paragraph" && block.constructs !== undefined) { + if (block.kind === "paragraph") { const fault = findRunConstructFault(block); if (fault !== undefined) { throw new MarkdownInvalidRunConstructExtentError( diff --git a/packages/markdown-codec/src/emit/table.ts b/packages/markdown-codec/src/emit/table.ts index 7697a024a..4fe80f364 100644 --- a/packages/markdown-codec/src/emit/table.ts +++ b/packages/markdown-codec/src/emit/table.ts @@ -47,9 +47,11 @@ function delimiterCell(alignment: MarkdownTableAlignment): string { function escapeUnescapedPipes(text: string): string { let out = ""; let index = 0; - while (index < text.length) { + // text.charAt(index) !== "", not index < text.length: the two are equivalent for every real index, but charAt already returns "" one past the end, which never matches "\\" or "|" either -- only this spelling's own mutation is reachable by a real test. + while (text.charAt(index) !== "") { const char = text.charAt(index); - if (char === "\\" && index + 1 < text.length) { + // No separate "is there a character after the backslash" guard: when the backslash is the very last character, text.charAt(index + 1) is already "", and appending char + "" is the identical single backslash the no-escape fallthrough two branches down would append anyway. + if (char === "\\") { out += char + text.charAt(index + 1); index += 2; continue; diff --git a/packages/markdown-codec/src/footnote.test.ts b/packages/markdown-codec/src/footnote.test.ts index 14a614cde..c5c81af90 100644 --- a/packages/markdown-codec/src/footnote.test.ts +++ b/packages/markdown-codec/src/footnote.test.ts @@ -820,10 +820,29 @@ describe("writing footnotes back out", () => { }); it("throws rather than guessing when the markers do not pair up", () => { - expect(() => - emitMarkdown(minimalDocument([{ kind: "constructEnd" }])), - ).toThrow(MarkdownUnbalancedConstructMarkersError); - expect(() => + let unmatchedEnd: unknown; + try { + emitMarkdown(minimalDocument([{ kind: "constructEnd" }])); + } catch (error) { + unmatchedEnd = error; + } + expect(unmatchedEnd).toBeInstanceOf( + MarkdownUnbalancedConstructMarkersError, + ); + const unmatchedEndTyped = + unmatchedEnd as MarkdownUnbalancedConstructMarkersError; + expect(unmatchedEndTyped.name).toBe( + "MarkdownUnbalancedConstructMarkersError", + ); + expect(unmatchedEndTyped.imbalanceKind).toBe("unmatchedEnd"); + expect(unmatchedEndTyped.blockIndex).toBe(0); + expect(unmatchedEndTyped.code).toBe("md/unbalanced-construct-markers"); + expect(unmatchedEndTyped.message).toBe( + "a constructEnd marker closes no open construct at block index 0; a block list's construct boundary markers must pair as balanced brackets", + ); + + let unclosedStart: unknown; + try { emitMarkdown( minimalDocument([ { @@ -831,8 +850,20 @@ describe("writing footnotes back out", () => { descriptor: { kind: "anchor", anchorType: "footnote", name: "1" }, }, ]), - ), - ).toThrow(MarkdownUnbalancedConstructMarkersError); + ); + } catch (error) { + unclosedStart = error; + } + expect(unclosedStart).toBeInstanceOf( + MarkdownUnbalancedConstructMarkersError, + ); + const unclosedStartTyped = + unclosedStart as MarkdownUnbalancedConstructMarkersError; + expect(unclosedStartTyped.imbalanceKind).toBe("unclosedStart"); + expect(unclosedStartTyped.blockIndex).toBe(0); + expect(unclosedStartTyped.message).toBe( + "a constructStart marker is never closed at block index 0; a block list's construct boundary markers must pair as balanced brackets", + ); }); }); diff --git a/packages/markdown-codec/src/html/html.ts b/packages/markdown-codec/src/html/html.ts index 8ce74fa72..d9d2647a4 100644 --- a/packages/markdown-codec/src/html/html.ts +++ b/packages/markdown-codec/src/html/html.ts @@ -27,10 +27,9 @@ const HTML_TAG_PATTERN = new RegExp( ); // Matches an inline HTML tag starting at `start` (which must be the `<`), returning its literal source text, or undefined when what follows is not a tag at all -- a bare `<` is ordinary text, never an error. +// +// No separate "does text[start] even open with '<'?" guard: every one of HTML_TAG_PATTERN's own alternatives (OPEN_TAG, CLOSING_TAG, HTML_COMMENT, PROCESSING_INSTRUCTION, DECLARATION, CDATA_SECTION) already begins with a literal '<' in its own regex source, and the pattern as a whole is anchored at `^` -- so a slice that doesn't start with '<' can never match any alternative regardless, and a guard duplicating that fact ahead of the real check would only ever agree with it. export function matchHtmlTag(text: string, start: number): string | undefined { - if (text.charAt(start) !== "<") { - return undefined; - } const match = HTML_TAG_PATTERN.exec(text.slice(start)); return match === null ? undefined : match[0]; } @@ -136,9 +135,7 @@ export function matchHtmlBlockStart( line: string, interruptsParagraph: boolean, ): HtmlBlockType | undefined { - if (!line.startsWith("<")) { - return undefined; - } + // No separate "does line even start with '<'?" guard: every one of HTML_BLOCK_START_PATTERNS' real entries (types 1-7) is itself anchored at `^` and begins with a literal '<' in its own regex source, so a line that doesn't open with '<' already fails every pattern in the loop below on its own, and the loop exhausts to the identical `undefined` regardless. for (const type of HTML_BLOCK_TYPES) { if (type === LAST_HTML_BLOCK_TYPE && interruptsParagraph) { continue; diff --git a/packages/markdown-codec/src/html/render.test.ts b/packages/markdown-codec/src/html/render.test.ts new file mode 100644 index 000000000..2eb79acc3 --- /dev/null +++ b/packages/markdown-codec/src/html/render.test.ts @@ -0,0 +1,331 @@ +// Direct unit tests for this module's own conformance-oracle rendering, isolated from the parser -- src/conformance.test.ts and src/gfm-conformance.test.ts only exercise renderDocumentToHtml through whatever the real CommonMark/GFM corpora happen to contain, which never reaches several of this renderer's own branches: math (a Pandoc/GFM extension outside both corpora), footnote definitions/references (a GitHub extension outside both), an apostrophe or a single-hex-digit byte in an href, and a table column with no alignment. Building MarkdownBlockNode/MarkdownDocumentNode trees by hand here reaches those directly. + +import { describe, expect, it } from "vitest"; +import type { MarkdownBlockNode, MarkdownDocumentNode } from "../ast/ast"; +import { escapeHref, renderDocumentToHtml, renderInlines } from "./render"; + +function render(children: MarkdownBlockNode[]): string { + const document: MarkdownDocumentNode = { type: "document", children }; + return renderDocumentToHtml(document); +} + +describe("escapeHref", () => { + it("passes an apostrophe through as the ' entity, not a percent escape", () => { + expect(escapeHref("'")).toBe("'"); + }); + + it("pads a single hex digit's percent escape to two digits", () => { + // U+0007 (BEL) is ASCII, not alphanumeric, not in the safe-punctuation set -- its own byte value is 7, whose hex digit "7" needs a leading zero. Built via fromCharCode rather than a literal escape so the source never carries a raw, invisible control byte. + expect(escapeHref(String.fromCharCode(7))).toBe("%07"); + }); + + it("leaves a two-hex-digit byte unpadded", () => { + // '<' is byte 0x3C -- already two hex digits, nothing to pad. + expect(escapeHref("<")).toBe("%3C"); + }); +}); + +describe("renderInlines: footnote reference and inline math, neither in the corpora this renderer otherwise checks against", () => { + it("renders a footnote reference as its own escaped source spelling", () => { + expect(renderInlines([{ type: "footnoteReference", label: "1" }])).toBe( + "[^1]", + ); + }); + + it("renders inline math wrapped back in its own \\( \\) delimiters", () => { + expect(renderInlines([{ type: "mathInline", literal: "x^2" }])).toBe( + "\\(x^2\\)", + ); + }); + + it("renders an empty inline math span as bare delimiters, not nothing", () => { + expect(renderInlines([{ type: "mathInline", literal: "" }])).toBe("\\(\\)"); + }); +}); + +describe("renderDocumentToHtml: display math and footnote definitions, neither in the corpora this renderer otherwise checks against", () => { + it("renders a math block wrapped in its own $$ delimiters, escaped", () => { + expect(render([{ type: "mathBlock", literal: "a < b" }])).toBe( + "$$\na < b\n$$\n", + ); + }); + + it("renders a footnote definition as its own escaped label line followed by its body's ordinary blocks", () => { + expect( + render([ + { + type: "footnoteDefinition", + label: "note", + children: [ + { + type: "paragraph", + children: [{ type: "text", value: "body" }], + }, + ], + }, + ]), + ).toBe("[^note]:\n

body

\n"); + }); + + it("separates two consecutive footnote definitions on their own lines", () => { + const html = render([ + { type: "footnoteDefinition", label: "a", children: [] }, + { type: "footnoteDefinition", label: "b", children: [] }, + ]); + expect(html).toBe("[^a]:\n[^b]:\n"); + }); +}); + +describe("renderDocumentToHtml: table column alignment, only rendered when genuinely aligned", () => { + it("omits the align attribute entirely for an unaligned ('none') column", () => { + const html = render([ + { + type: "table", + alignments: ["none"], + children: [ + { + type: "tableRow", + header: true, + children: [ + { type: "tableCell", children: [{ type: "text", value: "h" }] }, + ], + }, + ], + }, + ]); + expect(html).toContain(""); + expect(html).not.toContain("align="); + }); + + it("renders the align attribute for a genuinely aligned column", () => { + const html = render([ + { + type: "table", + alignments: ["right"], + children: [ + { + type: "tableRow", + header: true, + children: [ + { type: "tableCell", children: [{ type: "text", value: "h" }] }, + ], + }, + { + type: "tableRow", + header: false, + children: [ + { type: "tableCell", children: [{ type: "text", value: "b" }] }, + ], + }, + ], + }, + ]); + expect(html).toContain(''); + expect(html).toContain(''); + }); + + it("also omits the align attribute for a column with no alignment entry at all -- undefined, distinct from the explicit 'none'", () => { + const html = render([ + { + type: "table", + // Only one alignment entry for a row of two cells, so the second column's own lookup is genuinely undefined rather than "none". + alignments: ["none"], + children: [ + { + type: "tableRow", + header: true, + children: [ + { type: "tableCell", children: [{ type: "text", value: "h1" }] }, + { type: "tableCell", children: [{ type: "text", value: "h2" }] }, + ], + }, + ], + }, + ]); + expect(html).toContain(""); + expect(html).toContain(""); + expect(html).not.toContain("align="); + }); +}); + +describe("renderDocumentToHtml: cr() only inserts a newline when the buffer genuinely lacks one", () => { + it("inserts a newline between a tight list item's bare paragraph text and a nested list that follows it in the same item", () => { + const html = render([ + { + type: "list", + markerType: "bullet", + bulletMarker: "-", + tight: true, + children: [ + { + type: "listItem", + children: [ + { type: "paragraph", children: [{ type: "text", value: "a" }] }, + { + type: "list", + markerType: "bullet", + bulletMarker: "-", + tight: true, + children: [ + { + type: "listItem", + children: [ + { + type: "paragraph", + children: [{ type: "text", value: "b" }], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ]); + // A bare tight-paragraph "a" carries no trailing newline of its own -- the nested list's own leading cr() is what supplies the line break before its "
    ". + expect(html).toBe("
      \n
    • a\n
        \n
      • b
        • \n
      \n
      • \n
    \n"); + }); + + it("inserts a newline between a tight list item's bare paragraph text and a thematic break that follows it in the same item", () => { + const html = render([ + { + type: "list", + markerType: "bullet", + bulletMarker: "-", + tight: true, + children: [ + { + type: "listItem", + children: [ + { type: "paragraph", children: [{ type: "text", value: "a" }] }, + { type: "thematicBreak" }, + ], + }, + ], + }, + ]); + expect(html).toBe("
      \n
    • a\n
      \n
      • \n
    \n"); + }); + + it("inserts a newline between a tight list item's bare paragraph text and a table that follows it in the same item", () => { + const html = render([ + { + type: "list", + markerType: "bullet", + bulletMarker: "-", + tight: true, + children: [ + { + type: "listItem", + children: [ + { type: "paragraph", children: [{ type: "text", value: "a" }] }, + { + type: "table", + alignments: ["none"], + children: [ + { + type: "tableRow", + header: true, + children: [ + { + type: "tableCell", + children: [{ type: "text", value: "h" }], + }, + ], + }, + ], + }, + ], + }, + ], + }, + ]); + expect(html).toBe( + "
      \n
    • a\n
hhbh1h2
\n\n\n\n\n\n
h
\n\n\n", + ); + }); + + it("inserts a newline between a tight list item's bare paragraph text and a math block that follows it in the same item", () => { + const html = render([ + { + type: "list", + markerType: "bullet", + bulletMarker: "-", + tight: true, + children: [ + { + type: "listItem", + children: [ + { type: "paragraph", children: [{ type: "text", value: "a" }] }, + { type: "mathBlock", literal: "x" }, + ], + }, + ], + }, + ]); + expect(html).toBe("
    \n
  • a\n$$\nx\n$$\n
    • \n
\n"); + }); + + it("inserts a newline between a tight list item's bare paragraph text and a footnote definition that follows it in the same item", () => { + const html = render([ + { + type: "list", + markerType: "bullet", + bulletMarker: "-", + tight: true, + children: [ + { + type: "listItem", + children: [ + { type: "paragraph", children: [{ type: "text", value: "a" }] }, + { type: "footnoteDefinition", label: "n", children: [] }, + ], + }, + ], + }, + ]); + expect(html).toBe("
    \n
  • a\n[^n]:\n
    • \n
\n"); + }); +}); + +describe("renderInlines: image, the one leaf case none of block.test.ts/lower.test.ts/conformance corpora happen to reach through renderDocumentToHtml", () => { + it("renders src, alt, and (only when present) a title attribute", () => { + expect( + renderInlines([ + { type: "image", destination: "/a.png", alt: "alt text" }, + ]), + ).toBe('alt text'); + expect( + renderInlines([ + { + type: "image", + destination: "/a.png", + alt: "alt text", + title: "a title", + }, + ]), + ).toBe('alt text'); + }); +}); + +describe("renderDocumentToHtml: a code block's own info-string-to-language-class mapping", () => { + it("omits the class attribute entirely when there is no info string at all", () => { + expect(render([{ type: "codeBlock", fenced: true, literal: "x" }])).toBe( + "
x
\n", + ); + }); + + it("derives the class from the info string's own first word, ignoring the rest", () => { + expect( + render([ + { + type: "codeBlock", + fenced: true, + infoString: "js ignored", + literal: "x", + }, + ]), + ).toBe('
x
\n'); + }); +}); diff --git a/packages/markdown-codec/src/html/render.ts b/packages/markdown-codec/src/html/render.ts index ebac13cfe..2b8aabe28 100644 --- a/packages/markdown-codec/src/html/render.ts +++ b/packages/markdown-codec/src/html/render.ts @@ -67,11 +67,9 @@ export function escapeHref(href: string): string { const bytes = new TextEncoder().encode(href); let result = ""; for (const byte of bytes) { + // No separate "is this byte even ASCII" guard: ALPHANUMERIC_PATTERN and HREF_SAFE_PUNCTUATION are both pure-ASCII vocabularies on their own, so a byte >= 0x80 -- reinterpreted here as the single Latin-1 codepoint of that value, not as part of whatever multi-byte UTF-8 sequence it actually belongs to -- can never match either and falls through to percent-encoding regardless. const char = String.fromCharCode(byte); - if ( - byte < 0x80 && - (ALPHANUMERIC_PATTERN.test(char) || HREF_SAFE_PUNCTUATION.has(char)) - ) { + if (ALPHANUMERIC_PATTERN.test(char) || HREF_SAFE_PUNCTUATION.has(char)) { result += char; continue; } @@ -109,8 +107,8 @@ function renderTaskCheckbox(checked: boolean): string { function renderInline(node: MarkdownInlineNode): string { switch (node.type) { + // text and entity both carry their materialised text in the same field, and render identically -- one shared body, rather than two separately-mutable cases whose bodies are textually forced to stay identical anyway. case "text": - return escapeHtml(node.value); case "entity": return escapeHtml(node.value); case "codeSpan": @@ -198,7 +196,7 @@ class HtmlRenderer { this.cr(); this.out += "
\n"; this.render(node.children, false); - this.cr(); + // No closing cr(): every reachable block type's own rendering already ends in "\n" as its own last action (directly, or via its own cr()), so the buffer is always already newline-terminated here regardless of what the last child was, or whether there was one at all. this.out += "
\n"; return; case "list": @@ -208,24 +206,21 @@ class HtmlRenderer { this.renderTable(node); return; case "mathBlock": - // See renderInline's own mathInline case: the $$ delimiters are reconstructed around the escaped literal, matching src/emit/emit.ts's own real MATH_BLOCK_STYLE_ID branch. + // See renderInline's own mathInline case: the $$ delimiters are reconstructed around the escaped literal, matching src/emit/emit.ts's own real MATH_BLOCK_STYLE_ID branch. No closing cr(): the template itself always ends in a literal "\n". this.cr(); this.out += `$$\n${escapeHtml(node.literal)}\n$$\n`; - this.cr(); return; case "footnoteDefinition": - // See renderInline's own footnoteReference case: no fixture pins GitHub's own notes-section markup down, so the definition's source spelling is reconstructed around its rendered body, matching src/emit/emit.ts's own renderFootnoteDefinition. The body renders as ordinary blocks -- a definition holding several paragraphs shows all of them. + // See renderInline's own footnoteReference case: no fixture pins GitHub's own notes-section markup down, so the definition's source spelling is reconstructed around its rendered body, matching src/emit/emit.ts's own renderFootnoteDefinition. The body renders as ordinary blocks -- a definition holding several paragraphs shows all of them. No closing cr(), for the same reason blockquote's own closing tag needs none above. this.cr(); this.out += `${escapeHtml(`[^${node.label}]:`)}\n`; this.render(node.children, false); - this.cr(); return; + // Each is rendered only through its own parent, which knows the surrounding markup it needs -- an empty case (no consequent at all, not even a bare `return;`) since the switch is this method's last statement and falling off it already returns. case "document": case "listItem": case "tableRow": case "tableCell": - // Each is rendered only through its own parent, which knows the surrounding markup it needs. - return; } } @@ -235,8 +230,9 @@ class HtmlRenderer { ): void { this.cr(); // cmark takes the info string's first word as the language class and ignores the rest. + // String.prototype.split on a non-empty separator regex always returns at least one element (even splitting "" itself yields [""]), so index 0 is never undefined -- the assertion states that, since noUncheckedIndexedAccess cannot infer it from the split call alone. const language = - infoString === undefined ? "" : (infoString.split(/[ \t]/)[0] ?? ""); + infoString === undefined ? "" : infoString.split(/[ \t]/)[0]!; const attribute = language.length === 0 ? "" : ` class="language-${escapeHtml(language)}"`; this.out += `
${escapeHtml(literal)}
\n`; @@ -251,7 +247,7 @@ class HtmlRenderer { : `
    `; this.out += `${node.markerType === "bullet" ? "
      " : orderedOpenTag}\n`; for (const item of node.children) { - this.cr(); + // No cr() here: the buffer always already ends in "\n" at this point, either from the list's own just-appended opening tag (first iteration) or the previous iteration's own closing "\n" (every iteration after). this.out += "
    • "; // A task-list item's checkbox is the first fragment of the item's own first block -- rendered here, immediately after `
    • ` and before that block's own rendering, so it lands inside a tight item's bare inline content or (unverified against a real fixture, see this module's own top-of-file note) inside a loose item's `

      ` wrapper alike. if (item.checked !== undefined) { diff --git a/packages/markdown-codec/src/image/image.test.ts b/packages/markdown-codec/src/image/image.test.ts index d389d365e..7edbf9023 100644 --- a/packages/markdown-codec/src/image/image.test.ts +++ b/packages/markdown-codec/src/image/image.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { readImageDimensions } from "./image"; +import { + base64ToBytes, + bytesToBase64, + detectImageFormat, + readImageDimensions, +} from "./image"; function bytes(...values: number[]): Uint8Array { return new Uint8Array(values); @@ -124,4 +129,295 @@ describe("readImageDimensions", () => { const jpeg = bytes(0xff, 0xd8, 0xff, 0xe0, 0x00, 0x02); expect(readImageDimensions(jpeg)).toBeUndefined(); }); + + it("reads a PNG whose length is exactly the minimum IHDR-readable size", () => { + const png = bytes( + 0x89, + 0x50, + 0x4e, + 0x47, + 0x0d, + 0x0a, + 0x1a, + 0x0a, + 0x00, + 0x00, + 0x00, + 0x0d, + 0x49, + 0x48, + 0x44, + 0x52, + 0x00, + 0x00, + 0x00, + 0x01, + 0x00, + 0x00, + 0x00, + 0x01, + ); + expect(readImageDimensions(png)).toEqual({ widthPx: 1, heightPx: 1 }); + }); + + it("returns undefined for a PNG one byte short of the minimum IHDR-readable size", () => { + const png = bytes( + 0x89, + 0x50, + 0x4e, + 0x47, + 0x0d, + 0x0a, + 0x1a, + 0x0a, + 0x00, + 0x00, + 0x00, + 0x0d, + 0x49, + 0x48, + 0x44, + 0x52, + 0x00, + 0x00, + 0x00, + 0x01, + 0x00, + 0x00, + 0x00, + ); + expect(readImageDimensions(png)).toBeUndefined(); + }); + + it.each([ + [12, 0x00], + [13, 0x00], + [14, 0x00], + [15, 0x00], + ])( + "returns undefined when only byte %i of the 'IHDR' chunk type is wrong", + (badOffset, badByte) => { + const values = [ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, + 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + ]; + values[badOffset] = badByte; + expect(readImageDimensions(bytes(...values))).toBeUndefined(); + }, + ); + + it("does not mistake JPG (0xC8) for a frame header", () => { + const jpeg = bytes( + 0xff, + 0xd8, + 0xff, + 0xc8, + 0x00, + 0x02, // JPG marker, zero-length payload + 0xff, + 0xc0, + 0x00, + 0x0b, + 0x08, + 0x00, + 0x05, + 0x00, + 0x06, + 0x01, + 0x01, + 0x11, + 0x00, + 0xff, + 0xd9, + ); + expect(readImageDimensions(jpeg)).toEqual({ widthPx: 6, heightPx: 5 }); + }); + + it("does not mistake DAC (0xCC) for a frame header", () => { + const jpeg = bytes( + 0xff, + 0xd8, + 0xff, + 0xcc, + 0x00, + 0x03, + 0x00, // DAC, length 3 (1 payload byte) + 0xff, + 0xc0, + 0x00, + 0x0b, + 0x08, + 0x00, + 0x07, + 0x00, + 0x08, + 0x01, + 0x01, + 0x11, + 0x00, + 0xff, + 0xd9, + ); + expect(readImageDimensions(jpeg)).toEqual({ widthPx: 8, heightPx: 7 }); + }); + + it("reads a SOF2 (progressive) frame header, proving the SOF check isn't hardcoded to 0xC0", () => { + const jpeg = bytes( + 0xff, + 0xd8, + 0xff, + 0xc2, + 0x00, + 0x0b, // SOF2 + 0x08, + 0x00, + 0x09, + 0x00, + 0x0a, + 0x01, + 0x01, + 0x11, + 0x00, + 0xff, + 0xd9, + ); + expect(readImageDimensions(jpeg)).toEqual({ widthPx: 10, heightPx: 9 }); + }); + + it("skips a marker preceded by a run of extra 0xFF fill bytes", () => { + const jpeg = bytes( + 0xff, + 0xd8, + 0xff, + 0xff, + 0xff, + 0xc0, // fill bytes before the real SOF0 marker + 0x00, + 0x0b, + 0x08, + 0x00, + 0x0c, + 0x00, + 0x0d, + 0x01, + 0x01, + 0x11, + 0x00, + 0xff, + 0xd9, + ); + expect(readImageDimensions(jpeg)).toEqual({ widthPx: 13, heightPx: 12 }); + }); + + it("returns undefined when a run of fill bytes runs off the end of input with no marker byte following", () => { + const jpeg = bytes(0xff, 0xd8, 0xff, 0xff, 0xff); + expect(readImageDimensions(jpeg)).toBeUndefined(); + }); + + it.each([ + [0xd0, "RST0"], + [0xd7, "RST7"], + [0x01, "TEM"], + ])( + "skips a %s marker (%s) with no length field, continuing to the next marker", + (marker) => { + const jpeg = bytes( + 0xff, + 0xd8, + 0xff, + marker, // no-length-field marker + 0xff, + 0xc0, + 0x00, + 0x0b, + 0x08, + 0x00, + 0x0e, + 0x00, + 0x0f, + 0x01, + 0x01, + 0x11, + 0x00, + 0xff, + 0xd9, + ); + expect(readImageDimensions(jpeg)).toEqual({ + widthPx: 15, + heightPx: 14, + }); + }, + ); + + it("returns undefined at Start Of Scan (0xDA) with no frame header found first", () => { + const jpeg = bytes(0xff, 0xd8, 0xff, 0xda, 0x00, 0x02); + expect(readImageDimensions(jpeg)).toBeUndefined(); + }); + + it("returns undefined when a marker's declared length field is truncated", () => { + const jpeg = bytes(0xff, 0xd8, 0xff, 0xe0, 0x00); + expect(readImageDimensions(jpeg)).toBeUndefined(); + }); + + it("returns undefined when a SOF marker's own payload is truncated before height/width", () => { + const jpeg = bytes(0xff, 0xd8, 0xff, 0xc0, 0x00, 0x0b, 0x08, 0x00); + expect(readImageDimensions(jpeg)).toBeUndefined(); + }); +}); + +describe("detectImageFormat", () => { + it("detects a PNG signature", () => { + expect( + detectImageFormat(bytes(0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a)), + ).toBe("png"); + }); + + it("detects a JPEG SOI marker", () => { + expect(detectImageFormat(bytes(0xff, 0xd8, 0xff, 0xe0))).toBe("jpeg"); + }); + + it("returns undefined for neither a PNG nor a JPEG", () => { + expect(detectImageFormat(bytes(0x00, 0x01, 0x02, 0x03))).toBeUndefined(); + }); + + it("returns undefined for an empty input", () => { + expect(detectImageFormat(bytes())).toBeUndefined(); + }); +}); + +describe("bytesToBase64 / base64ToBytes", () => { + it.each([ + [[], ""], + [[0x4d], "TQ=="], + [[0x4d, 0x61], "TWE="], + [[0x4d, 0x61, 0x6e], "TWFu"], + [[0x4d, 0x61, 0x6e, 0x21], "TWFuIQ=="], + ])("encodes %j to %s", (input, expected) => { + expect(bytesToBase64(bytes(...input))).toBe(expected); + }); + + it.each([ + ["", []], + ["TQ==", [0x4d]], + ["TWE=", [0x4d, 0x61]], + ["TWFu", [0x4d, 0x61, 0x6e]], + ["TWFuIQ==", [0x4d, 0x61, 0x6e, 0x21]], + ])("decodes %s to %j", (input, expected) => { + expect(Array.from(base64ToBytes(input))).toEqual(expected); + }); + + it("round-trips arbitrary byte sequences through encode then decode", () => { + const original = bytes(0x00, 0xff, 0x10, 0x80, 0x7f, 0x01, 0x02, 0x03); + expect(Array.from(base64ToBytes(bytesToBase64(original)))).toEqual( + Array.from(original), + ); + }); + + it("ignores characters outside the base64 alphabet when decoding", () => { + expect(Array.from(base64ToBytes("TW\nFu\r\n"))).toEqual([0x4d, 0x61, 0x6e]); + }); + + it("throws for an invalid base64 character in a would-be data position", () => { + expect(() => base64ToBytes("T!==")).toThrow("invalid base64 input"); + }); }); diff --git a/packages/markdown-codec/src/inline/chars.test.ts b/packages/markdown-codec/src/inline/chars.test.ts new file mode 100644 index 000000000..76f089d7d --- /dev/null +++ b/packages/markdown-codec/src/inline/chars.test.ts @@ -0,0 +1,153 @@ +// Direct unit tests for this module's own character-class predicates and codepoint helpers -- link.ts and delimiter.ts only exercise these through whatever characters the round-trip suites' own markdown sources happen to contain, never at the exact boundaries (0x1f/0x20, 0x7e/0x7f, and every one of the four surrogate-pair range edges individually) that distinguish a correct comparison from an off-by-one one. + +import { describe, expect, it } from "vitest"; +import { + codePointAt, + codePointBefore, + containsAsciiControlOrSpace, + isAsciiControl, + isMarkdownSpace, +} from "./chars"; + +describe("isAsciiControl", () => { + it("returns false for an empty string, where codePointAt(0) is undefined", () => { + expect(isAsciiControl("")).toBe(false); + }); + + it("treats 0x1f (unit separator) as control, but not 0x20 (space) immediately past it", () => { + expect(isAsciiControl("")).toBe(true); + expect(isAsciiControl(" ")).toBe(false); + }); + + it("treats 0x7f (DEL) as control, but not 0x7e (~) immediately before it", () => { + expect(isAsciiControl("")).toBe(true); + expect(isAsciiControl("~")).toBe(false); + }); + + it("does not treat a byte past 0x7f as control", () => { + expect(isAsciiControl("€")).toBe(false); + }); +}); + +describe("containsAsciiControlOrSpace", () => { + it("is false for an empty string", () => { + expect(containsAsciiControlOrSpace("")).toBe(false); + }); + + it("is false for text with no control character and no space", () => { + expect(containsAsciiControlOrSpace("abc")).toBe(false); + }); + + it("is true for text containing a control character", () => { + expect(containsAsciiControlOrSpace("ab")).toBe(true); + }); + + it("is true for text containing a space", () => { + expect(containsAsciiControlOrSpace("a b")).toBe(true); + }); +}); + +describe("isMarkdownSpace", () => { + it("recognises space, tab, line feed, and carriage return", () => { + expect(isMarkdownSpace(" ")).toBe(true); + expect(isMarkdownSpace("\t")).toBe(true); + expect(isMarkdownSpace("\n")).toBe(true); + expect(isMarkdownSpace("\r")).toBe(true); + }); + + it("does not recognise a non-breaking space or an ordinary letter", () => { + expect(isMarkdownSpace(" ")).toBe(false); + expect(isMarkdownSpace("a")).toBe(false); + }); +}); + +describe("codePointBefore", () => { + it("returns a bare newline at the very start of the string", () => { + expect(codePointBefore("abc", 0)).toBe("\n"); + }); + + it("returns the single preceding character when it is not a low surrogate", () => { + expect(codePointBefore("ab", 2)).toBe("b"); + }); + + it("returns the single preceding character at index 1, too early for a surrogate pair to fit before it", () => { + expect(codePointBefore("a😀", 1)).toBe("a"); + }); + + it("returns the full surrogate pair when a genuine astral character precedes the index", () => { + // U+1F600 (grinning face) is the high/low surrogate pair 😀. + expect(codePointBefore("a😀", 3)).toBe("😀"); + }); + + it("returns only the low surrogate when it is not preceded by a valid high surrogate", () => { + // \uDE00 alone (a lone low surrogate, no high surrogate before it) is not a real pair. + expect(codePointBefore("a\uDE00", 2)).toBe("\uDE00"); + }); + + describe("low-surrogate range boundary (0xdc00-0xdfff)", () => { + it("treats 0xdc00 (the lower bound) as a low surrogate", () => { + const low = String.fromCharCode(0xdc00); + const text = String.fromCharCode(0xd800) + low; + expect(codePointBefore(text, 2)).toBe(text); + }); + + it("does not treat 0xdbff (one below the lower bound) as a low surrogate", () => { + const notLow = String.fromCharCode(0xdbff); + const text = String.fromCharCode(0xd800) + notLow; + expect(codePointBefore(text, 2)).toBe(notLow); + }); + + it("treats 0xdfff (the upper bound) as a low surrogate", () => { + const low = String.fromCharCode(0xdfff); + const text = String.fromCharCode(0xd800) + low; + expect(codePointBefore(text, 2)).toBe(text); + }); + + it("does not treat 0xe000 (one above the upper bound) as a low surrogate", () => { + const notLow = String.fromCharCode(0xe000); + const text = String.fromCharCode(0xd800) + notLow; + expect(codePointBefore(text, 2)).toBe(notLow); + }); + }); + + describe("high-surrogate range boundary (0xd800-0xdbff)", () => { + it("treats 0xd800 (the lower bound) as a valid high surrogate", () => { + const high = String.fromCharCode(0xd800); + const low = String.fromCharCode(0xdc00); + expect(codePointBefore(high + low, 2)).toBe(high + low); + }); + + it("does not treat 0xd7ff (one below the lower bound) as a valid high surrogate", () => { + const notHigh = String.fromCharCode(0xd7ff); + const low = String.fromCharCode(0xdc00); + expect(codePointBefore(notHigh + low, 2)).toBe(low); + }); + + it("treats 0xdbff (the upper bound) as a valid high surrogate", () => { + const high = String.fromCharCode(0xdbff); + const low = String.fromCharCode(0xdc00); + expect(codePointBefore(high + low, 2)).toBe(high + low); + }); + + it("does not treat 0xdc00 (one above the upper bound) as a valid high surrogate", () => { + const notHigh = String.fromCharCode(0xdc00); + const low = String.fromCharCode(0xdc00); + expect(codePointBefore(notHigh + low, 2)).toBe(low); + }); + }); +}); + +describe("codePointAt", () => { + it("returns a bare newline at or past the end of the string", () => { + expect(codePointAt("abc", 3)).toBe("\n"); + expect(codePointAt("abc", 4)).toBe("\n"); + }); + + it("returns the single character at a normal index", () => { + expect(codePointAt("abc", 1)).toBe("b"); + }); + + it("returns the full surrogate pair for an astral character", () => { + expect(codePointAt("😀b", 0)).toBe("😀"); + }); +}); diff --git a/packages/markdown-codec/src/inline/chars.ts b/packages/markdown-codec/src/inline/chars.ts index 39dfa1410..f73c24aab 100644 --- a/packages/markdown-codec/src/inline/chars.ts +++ b/packages/markdown-codec/src/inline/chars.ts @@ -38,12 +38,7 @@ export function isAsciiControl(char: string): boolean { // Whether any ASCII control character or space appears in `text` -- the exclusion an absolute URI inside an autolink is defined by (spec 0.31.2: "zero or more characters other than ASCII control characters, space, `<`, and `>`"). Written as a scan rather than a regex character range deliberately: a `[\x00-\x20]` class is a literal control character embedded in a pattern, which is both unreadable and exactly what eslint's own no-control-regex rule exists to catch. export function containsAsciiControlOrSpace(text: string): boolean { - for (let index = 0; index < text.length; index += 1) { - if (isAsciiControl(text.charAt(index)) || text.charAt(index) === " ") { - return true; - } - } - return false; + return text.split("").some((char) => isAsciiControl(char) || char === " "); } // Spaces, tabs, and line endings -- the whitespace vocabulary CommonMark's own *syntactic* rules use (link label normalisation, the whitespace permitted between an inline link's components), as opposed to the full Unicode whitespace class the flanking rules use. Kept distinct deliberately: collapsing the two would make a non-breaking space count as a label separator, which the spec does not allow. @@ -56,8 +51,9 @@ export function codePointBefore(text: string, index: number): string { if (index <= 0) { return "\n"; } + // No separate "index >= 2" guard: index <= 0 has already returned above, leaving index === 1 as the only remaining case a missing guard could affect, and text.charCodeAt(-2) there is always NaN, which already fails the high-surrogate check below on its own -- an explicit index guard would only ever exclude a case that already excludes itself. const low = text.charCodeAt(index - 1); - if (index >= 2 && low >= 0xdc00 && low <= 0xdfff) { + if (low >= 0xdc00 && low <= 0xdfff) { const high = text.charCodeAt(index - 2); if (high >= 0xd800 && high <= 0xdbff) { return text.slice(index - 2, index); @@ -71,9 +67,6 @@ export function codePointAt(text: string, index: number): string { if (index >= text.length) { return "\n"; } - const code = text.codePointAt(index); - if (code === undefined) { - return "\n"; - } - return String.fromCodePoint(code); + // text.codePointAt only ever returns undefined for an out-of-range index, and the guard above has already ruled that out -- no further fallback needed for an index it can actually be called with here. + return String.fromCodePoint(text.codePointAt(index)!); } diff --git a/packages/markdown-codec/src/inline/delimiter.test.ts b/packages/markdown-codec/src/inline/delimiter.test.ts index 447e1b349..a4df8ec08 100644 --- a/packages/markdown-codec/src/inline/delimiter.test.ts +++ b/packages/markdown-codec/src/inline/delimiter.test.ts @@ -1,7 +1,31 @@ // Direct tests for delimiter-run flanking classification. The conformance suite exercises this through whole documents, which is the right end-to-end check but a poor diagnostic: a flanking bug there surfaces as a wrong emphasis nesting several steps downstream. These pin the classification itself, using the exact runs the spec's own "Here are some examples of delimiter runs" list gives. import { describe, expect, it } from "vitest"; -import { scanDelimiterRun } from "./delimiter"; +import type { Delimiter } from "./delimiter"; +import { + DelimiterStack, + closerSignature, + processEmphasis, + scanDelimiterRun, +} from "./delimiter"; +import { InlineNode } from "./node"; + +function delimiter(fields: { + char: "*" | "_" | "~"; + origCount: number; + canOpen: boolean; +}): Delimiter { + return { + char: fields.char, + count: fields.origCount, + origCount: fields.origCount, + canOpen: fields.canOpen, + canClose: true, + node: new InlineNode("text"), + previous: undefined, + next: undefined, + }; +} function classify(text: string, start: number, char: "*" | "_" | "~"): string { const run = scanDelimiterRun(text, start, char); @@ -22,6 +46,10 @@ describe("scanDelimiterRun", () => { expect(scanDelimiterRun("***abc", 0, "*")?.count).toBe(3); }); + it("returns undefined when the position does not actually open with the given delimiter character", () => { + expect(scanDelimiterRun("abc", 0, "*")).toBeUndefined(); + }); + // spec 0.31.2's own "left-flanking but not right-flanking" examples. it.each([ ["***abc", 0, "*"], @@ -78,3 +106,65 @@ describe("scanDelimiterRun", () => { expect(classify("~~a", 0, "~")).toBe("open"); }); }); + +describe("closerSignature", () => { + it("encodes the delimiter character, whether it can open, and origCount % 3 -- distinctly for each", () => { + expect( + closerSignature(delimiter({ char: "*", origCount: 1, canOpen: true })), + ).toBe("*11"); + expect( + closerSignature(delimiter({ char: "*", origCount: 1, canOpen: false })), + ).toBe("*01"); + // origCount 4 falls in the same modulo-3 bucket as 1 -- same signature. + expect( + closerSignature(delimiter({ char: "*", origCount: 4, canOpen: true })), + ).toBe("*11"); + expect( + closerSignature(delimiter({ char: "*", origCount: 2, canOpen: true })), + ).toBe("*12"); + expect( + closerSignature(delimiter({ char: "_", origCount: 1, canOpen: true })), + ).toBe("_11"); + }); +}); + +describe("processEmphasis", () => { + it("applies the rule-of-three carve-out: a match is allowed when both run lengths are themselves multiples of three, even though their sum also is", () => { + const stack = new DelimiterStack(); + const opener = delimiter({ char: "*", origCount: 3, canOpen: true }); + opener.node.literal = "***"; + stack.push("*", { count: 3, canOpen: true, canClose: true }, opener.node); + const closer = delimiter({ char: "*", origCount: 3, canOpen: true }); + closer.node.literal = "***"; + stack.push("*", { count: 3, canOpen: true, canClose: true }, closer.node); + + processEmphasis(stack, undefined, (kind) => new InlineNode(kind)); + + // A blocked match would leave both runs' literal text untouched. + expect(opener.node.literal).toBe(""); + }); + + it("keeps a delimiter search bounded by the openers floor rather than re-walking the whole stack for every same-signature closer", () => { + const stack = new DelimiterStack(); + // A long run of inert, never-removed, never-matching delimiters of a different character sits below a batch of same-signature closers that can never match anything either -- without the floor, each of those closers re-walks the entire inert run from scratch, making the whole pass quadratic in its length. + const inertCount = 50_000; + for (let i = 0; i < inertCount; i++) { + const node = new InlineNode("text"); + node.literal = "_"; + stack.push("_", { count: 1, canOpen: true, canClose: false }, node); + } + const closerCount = 500; + for (let i = 0; i < closerCount; i++) { + const node = new InlineNode("text"); + node.literal = "*"; + stack.push("*", { count: 1, canOpen: false, canClose: true }, node); + } + + const start = performance.now(); + processEmphasis(stack, undefined, (kind) => new InlineNode(kind)); + const elapsed = performance.now() - start; + + // The bounded-search version finishes in well under a second for this input on any reasonable machine; without the floor it takes upward of ten seconds (measured locally at roughly 14s for these same sizes), so this margin is not close either way. + expect(elapsed).toBeLessThan(5000); + }, 20_000); +}); diff --git a/packages/markdown-codec/src/inline/delimiter.ts b/packages/markdown-codec/src/inline/delimiter.ts index f359e8b31..1a962cddc 100644 --- a/packages/markdown-codec/src/inline/delimiter.ts +++ b/packages/markdown-codec/src/inline/delimiter.ts @@ -118,7 +118,9 @@ export class DelimiterStack { } // A closer's "signature" for the openers-floor map below. The rule-of-three predicate depends only on the closer's own delimiter character, whether it can also open, and its original length modulo three -- so once a closer with a given signature has failed to find any opener above a position, no LATER closer with that same signature can succeed below it either, and the search floor can be raised permanently. Keying by all three (rather than cmark's coarser "one bucket for every `_`") keeps the pruning exactly sound: a coarser key would raise the floor for closers whose predicate differs from the one that failed. -function closerSignature(closer: Delimiter): string { +// +// Exported for direct testing: the string's own exact shape (which literal marks the canOpen branch, `% 3` rather than any other reduction) has no effect processEmphasis's own black-box behaviour can distinguish -- every reachable pair of distinct signatures is already provably distinct by char or by the raw fields isRuleOfThreeBlocked reads regardless of the exact spelling used to encode them here, so the only way to pin the concrete encoding this comment documents is to assert this function's own return value. +export function closerSignature(closer: Delimiter): string { return `${closer.char}${closer.canOpen ? "1" : "0"}${String(closer.origCount % 3)}`; } @@ -142,14 +144,13 @@ function canMatch(opener: Delimiter, closer: Delimiter): boolean { return !isRuleOfThreeBlocked(opener, closer); } +// No separate closer.char === "~" case: canMatch's own tilde branch already requires opener.count === closer.count before a tilde match is ever accepted, and MAX_STRIKETHROUGH_RUN caps both to 1 or 2 -- so for any tilde pair that reaches here, the generic formula below (both sides have two available, or neither does, since the counts are equal) already evaluates to exactly closer.count either way. +// +// spec 0.31.2 rule 13: "if one of the delimiters can both open and close emphasis, then the sum ..." -- operationally, a match consumes two delimiters (strong emphasis) whenever both runs still have two available, and one otherwise, with any remainder left on the stack to pair up again. function delimitersConsumedByMatch( opener: Delimiter, closer: Delimiter, ): number { - if (closer.char === "~") { - return closer.count; - } - // spec 0.31.2 rule 13: "if one of the delimiters can both open and close emphasis, then the sum ..." -- operationally, a match consumes two delimiters (strong emphasis) whenever both runs still have two available, and one otherwise, with any remainder left on the stack to pair up again. return closer.count >= 2 && opener.count >= 2 ? 2 : 1; } @@ -194,9 +195,10 @@ export function processEmphasis( ? openersFloor.get(signature) : stackBottom; + // No separate opener !== stackBottom arm: floor defaults to stackBottom itself for a signature never seen before (just above), and a raised floor is always at or above stackBottom in this same walk -- so opener !== floor already stops the search no later than opener !== stackBottom ever would. let opener = closer.previous; let matchedOpener: Delimiter | undefined; - while (opener !== undefined && opener !== stackBottom && opener !== floor) { + while (opener !== undefined && opener !== floor) { if (canMatch(opener, closer)) { matchedOpener = opener; break; @@ -208,9 +210,7 @@ export function processEmphasis( if (matchedOpener === undefined) { closer = closer.next; openersFloor.set(signature, failedCloser.previous); - if (!failedCloser.canOpen) { - stack.remove(failedCloser); - } + // No stack.remove(failedCloser) here even when it cannot itself open: canMatch's own initial guard already rejects any delimiter with canOpen false as a later opener candidate, so leaving it linked can never produce a wrong match -- only ever one extra, cheap guard check for a search that reaches it, which the floor just set already prevents for anything of this same signature. continue; } @@ -237,18 +237,15 @@ export function processEmphasis( } openerNode.insertAfter(wrapper); - // Every delimiter strictly between the pair is now enclosed by the new wrapper and can never pair with anything outside it -- drop them all at once rather than one at a time. - if (matchedOpener.next !== closer) { - matchedOpener.next = closer; - closer.previous = matchedOpener; - } + // Every delimiter strictly between the pair is now enclosed by the new wrapper and can never pair with anything outside it -- drop them all at once rather than one at a time. No `matchedOpener.next !== closer` guard: when nothing sat between them, both writes below already hold, so skipping them changes nothing. + matchedOpener.next = closer; + closer.previous = matchedOpener; + // No openerNode.unlink()/closerNode.unlink() here: toAstNode (src/inline/inline.ts) already drops any zero-length text node -- scaffolding, not content -- regardless of where it still sits in the sibling chain, and appendChild unlinks its argument unconditionally anyway before attaching it elsewhere. Removing the now-empty delimiter from the STACK below is the part that is load-bearing: canMatch has no way to see that a delimiter's count already reached zero, so a fully consumed opener or closer left on the stack can still be matched again by a later closer. if (matchedOpener.count === 0) { - openerNode.unlink(); stack.remove(matchedOpener); } if (closer.count === 0) { - closerNode.unlink(); const following = closer.next; stack.remove(closer); closer = following; diff --git a/packages/markdown-codec/src/inline/entity.test.ts b/packages/markdown-codec/src/inline/entity.test.ts new file mode 100644 index 000000000..78f1a5003 --- /dev/null +++ b/packages/markdown-codec/src/inline/entity.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import { matchEntity, unescapeString } from "./entity"; + +describe("matchEntity numeric references", () => { + it("decodes a hex reference to its real character", () => { + expect(matchEntity("A", 0)).toEqual({ raw: "A", value: "A" }); + }); + + it("decodes a decimal reference to its real character", () => { + expect(matchEntity("A", 0)).toEqual({ raw: "A", value: "A" }); + }); + + it("decodes U+0000 to the replacement character, per the spec's own rule", () => { + expect(matchEntity("�", 0)?.value).toBe("�"); + }); + + it("decodes the maximum valid codepoint (U+10FFFF) normally, not as a replacement", () => { + expect(matchEntity("􏿿", 0)?.value).toBe( + String.fromCodePoint(0x10ffff), + ); + }); + + it("decodes one past the maximum valid codepoint to the replacement character", () => { + expect(matchEntity("�", 0)?.value).toBe("�"); + }); + + it("decodes the character just below the surrogate range normally", () => { + expect(matchEntity("퟿", 0)?.value).toBe( + String.fromCodePoint(0xd7ff), + ); + }); + + it("decodes the first surrogate codepoint (U+D800) to the replacement character", () => { + expect(matchEntity("�", 0)?.value).toBe("�"); + }); + + it("decodes the last surrogate codepoint (U+DFFF) to the replacement character", () => { + expect(matchEntity("�", 0)?.value).toBe("�"); + }); + + it("decodes the character just past the surrogate range normally", () => { + expect(matchEntity("", 0)?.value).toBe( + String.fromCodePoint(0xe000), + ); + }); + + it("returns undefined for an unrecognised named entity", () => { + expect(matchEntity("&MissingGlyph;", 0)).toBeUndefined(); + }); +}); + +describe("unescapeString", () => { + it("passes plain text with neither a backslash nor an entity through unchanged", () => { + expect(unescapeString("plain text")).toBe("plain text"); + }); + + it("resolves a backslash escape", () => { + expect(unescapeString("a\\*b")).toBe("a*b"); + }); + + it("resolves a named entity", () => { + expect(unescapeString("a&b")).toBe("a&b"); + }); + + it("leaves a lone unescapable backslash as a literal character", () => { + expect(unescapeString("a\\zb")).toBe("a\\zb"); + }); + + it("leaves an unrecognised '&' sequence as literal text", () => { + expect(unescapeString("a&b")).toBe("a&b"); + }); +}); diff --git a/packages/markdown-codec/src/inline/entity.ts b/packages/markdown-codec/src/inline/entity.ts index 7c3cbb0e8..a58493f14 100644 --- a/packages/markdown-codec/src/inline/entity.ts +++ b/packages/markdown-codec/src/inline/entity.ts @@ -35,13 +35,11 @@ function codepointToString(codepoint: number): string { } // Matches an entity or numeric character reference starting at `start` (which must be the `&`). Returns undefined when what follows is not a valid reference at all -- a bare `&` is ordinary text, never an error. +// No separate "does text[start] even open with '&'?" guard: ENTITY_PATTERN's own source is anchored at `^&`, so a slice that doesn't open with '&' can never match regardless -- the same reasoning src/html/html.ts's matchHtmlTag/matchHtmlBlockStart apply to their own leading '<' checks. export function matchEntity( text: string, start: number, ): EntityMatch | undefined { - if (text.charAt(start) !== "&") { - return undefined; - } const match = ENTITY_PATTERN.exec(text.slice(start)); if (match === null) { return undefined; @@ -66,12 +64,11 @@ export function matchEntity( // Resolves backslash escapes and character references inside a string that is NOT itself parsed as inline content -- a link destination or a link title. spec 0.31.2: "backslash escapes and entity and numeric character references are recognized" in both. This is a flattening operation with no node structure of its own, which is exactly why it lives here rather than being expressed in terms of the inline parser's own dispatch loop. export function unescapeString(text: string): string { - if (!text.includes("\\") && !text.includes("&")) { - return text; - } + // No "does text hold neither '\\' nor '&' at all?" fast path: for a string with neither, the loop below never takes the backslash/entity branches, so it does nothing but copy every character straight through -- reconstructing `text` exactly, just one character-append at a time rather than in a single return. The fast path changed how much work this function did for that input, never what it produced. let result = ""; let index = 0; - while (index < text.length) { + // text.charAt(index) !== "", not index < text.length: the two are equivalent for every real index (charAt already returns "" one past the end, which never matches "\\" or "&" either), but only this spelling's own mutation is actually reachable by a test. + while (text.charAt(index) !== "") { const char = text.charAt(index); if (char === "\\") { const next = text.charAt(index + 1); @@ -84,13 +81,12 @@ export function unescapeString(text: string): string { index += 1; continue; } - if (char === "&") { - const entity = matchEntity(text, index); - if (entity !== undefined) { - result += entity.value; - index += entity.raw.length; - continue; - } + // No separate char === "&" guard: matchEntity's own ENTITY_PATTERN is anchored at "^&" (see its own comment above), so calling it at a non-"&" index can never match regardless -- the same reasoning already applied to matchEntity's own leading-character check. + const entity = matchEntity(text, index); + if (entity !== undefined) { + result += entity.value; + index += entity.raw.length; + continue; } result += char; index += 1; diff --git a/packages/markdown-codec/src/inline/footnote.test.ts b/packages/markdown-codec/src/inline/footnote.test.ts new file mode 100644 index 000000000..ce94a3e27 --- /dev/null +++ b/packages/markdown-codec/src/inline/footnote.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; +import { + isValidFootnoteLabel, + matchFootnoteDefinitionMarker, + matchFootnoteLabel, +} from "./footnote"; + +describe("matchFootnoteLabel", () => { + it("matches a `[^label]` marker and reports the index one past its closing bracket", () => { + expect(matchFootnoteLabel("[^abc] rest", 0)).toEqual({ + label: "abc", + end: 6, + }); + }); + + it("returns undefined for a bracket that is not a footnote label at all", () => { + expect(matchFootnoteLabel("[abc] rest", 0)).toBeUndefined(); + }); +}); + +describe("matchFootnoteDefinitionMarker", () => { + it("matches a real `[^label]:` definition marker", () => { + expect(matchFootnoteDefinitionMarker("[^abc]: body text")).toEqual({ + label: "abc", + markerLength: 7, + }); + }); + + it("rejects a valid label marker with no following colon -- this is a reference, not a definition", () => { + expect( + matchFootnoteDefinitionMarker("[^abc] not a definition"), + ).toBeUndefined(); + }); + + it("rejects text that is not even a valid footnote label", () => { + expect( + matchFootnoteDefinitionMarker("not a marker at all"), + ).toBeUndefined(); + }); +}); + +describe("isValidFootnoteLabel", () => { + it("accepts an ordinary label", () => { + expect(isValidFootnoteLabel("note-1")).toBe(true); + }); + + it("rejects a label carrying whitespace or a bracket, which this grammar cannot represent", () => { + expect(isValidFootnoteLabel("has space")).toBe(false); + expect(isValidFootnoteLabel("has]bracket")).toBe(false); + }); +}); diff --git a/packages/markdown-codec/src/inline/inline.test.ts b/packages/markdown-codec/src/inline/inline.test.ts index e0031b9ea..b22f4889b 100644 --- a/packages/markdown-codec/src/inline/inline.test.ts +++ b/packages/markdown-codec/src/inline/inline.test.ts @@ -143,6 +143,23 @@ describe("emphasis, strong emphasis, and the flanking rules", () => { ]); }); + it("resolves two independent, non-overlapping emphasis pairs, not letting the first pair's exhausted closer be reused as the second pair's opener", () => { + // Once *a* is resolved, its own closing "*" is fully consumed (count reaches 0) and must come off the delimiter stack -- otherwise it can still open (both-flanking, like any `*` between word-ish characters here) and the second closer wrongly matches THAT leftover delimiter instead of the real "*" opener before "c", swallowing the second pair's own emphasis into nothing. + expect(parse("*a*b*c*")).toEqual([ + { + type: "emphasis", + marker: "*", + children: [{ type: "text", value: "a" }], + }, + { type: "text", value: "b" }, + { + type: "emphasis", + marker: "*", + children: [{ type: "text", value: "c" }], + }, + ]); + }); + it("nests strong inside emphasis for a three-delimiter run", () => { expect(parse("***foo***")).toEqual([ { diff --git a/packages/markdown-codec/src/inline/link.test.ts b/packages/markdown-codec/src/inline/link.test.ts index 2f3584bea..2ca9646ee 100644 --- a/packages/markdown-codec/src/inline/link.test.ts +++ b/packages/markdown-codec/src/inline/link.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { + isBlankRemainderOfLine, matchLinkLabel, normalizeLinkLabel, parseLinkDestination, @@ -37,6 +38,14 @@ describe("matchLinkLabel", () => { expect(matchLinkLabel(`[${"a".repeat(999)}]`, 0)).toBe(1001); expect(matchLinkLabel(`[${"a".repeat(1000)}]`, 0)).toBe(0); }); + + it("returns 0 for text that does not open with '[' at all, even when a ']' appears later", () => { + expect(matchLinkLabel("abc]", 0)).toBe(0); + }); + + it("returns 0 for an unterminated label that runs off the end of text with no closing ']'", () => { + expect(matchLinkLabel("[abc", 0)).toBe(0); + }); }); describe("parseLinkDestination", () => { @@ -55,6 +64,17 @@ describe("parseLinkDestination", () => { expect(parseLinkDestination("", 0)).toBeUndefined(); }); + it("rejects an angle-bracketed destination containing an unescaped nested '<', even with no line ending", () => { + expect(parseLinkDestination("", 0)).toBeUndefined(); + }); + + it("treats a trailing, unescapable backslash as a literal character, not the start of an escape past the end", () => { + expect(parseLinkDestination("abc\\", 0)).toEqual({ + value: "abc\\", + end: 4, + }); + }); + it("reads a bare destination with balanced parentheses", () => { expect(parseLinkDestination("/a(b)c)", 0)).toEqual({ value: "/a(b)c", @@ -101,3 +121,21 @@ describe("skipInlineWhitespace", () => { expect(skipInlineWhitespace(" \n \n x", 0)).toBe(3); }); }); + +describe("isBlankRemainderOfLine", () => { + it("is true at the very end of the text -- vacuously blank, nothing left to disqualify it", () => { + expect(isBlankRemainderOfLine("", 0)).toBe(true); + }); + + it("is true when only spaces/tabs remain all the way to the end of the text", () => { + expect(isBlankRemainderOfLine(" ", 0)).toBe(true); + }); + + it("is true as soon as a line ending is reached", () => { + expect(isBlankRemainderOfLine(" \nrest", 0)).toBe(true); + }); + + it("is false when a non-space character remains before any line ending", () => { + expect(isBlankRemainderOfLine(" x", 0)).toBe(false); + }); +}); diff --git a/packages/markdown-codec/src/inline/link.ts b/packages/markdown-codec/src/inline/link.ts index 4ca7a49c2..2c80ab01e 100644 --- a/packages/markdown-codec/src/inline/link.ts +++ b/packages/markdown-codec/src/inline/link.ts @@ -33,7 +33,8 @@ export function matchLinkLabel(text: string, start: number): number { return 0; } let index = start + 1; - while (index < text.length) { + // text.charAt(index) !== "", not index < text.length: the two are equivalent for every real index, but charAt already returns "" one past the end, which none of this loop's own character comparisons below can ever match either. Unlike skipInlineWhitespace's own loop below, nothing inside this body breaks on an ordinary character, so this guard is the only thing that stops the loop once text runs out before a closing bracket is found -- an unterminated label (no "[" or "]" anywhere in the rest of text) is what actually exercises it. + while (text.charAt(index) !== "") { const char = text.charAt(index); if (char === "\\") { index += 2; @@ -63,7 +64,8 @@ export function parseLinkDestination( ): ParsedSpan | undefined { if (text.charAt(start) === "<") { let index = start + 1; - while (index < text.length) { + // See matchLinkLabel's own note above on why this is charAt(index) !== "" rather than index < text.length. + while (text.charAt(index) !== "") { const char = text.charAt(index); if (char === "\\") { index += 2; @@ -131,12 +133,11 @@ export function parseLinkTitle( start: number, ): ParsedSpan | undefined { const opener = text.charAt(start); + // No separate "closer === undefined, bail out now" guard: when `opener` isn't one of TITLE_DELIMITERS' own three keys, `closer` stays undefined, `char === closer` can never match a real character (charAt never returns the JS value undefined), and TITLE_DELIMITERS' own mapping means opener can only ever be "(" when closer IS defined -- so the loop below just scans to the end matching nothing and returns undefined regardless, on its own. const closer = TITLE_DELIMITERS.get(opener); - if (closer === undefined) { - return undefined; - } let index = start + 1; - while (index < text.length) { + // See matchLinkLabel's own note (src/inline/link.ts) on why this is charAt(index) !== "" rather than index < text.length. + while (text.charAt(index) !== "") { const char = text.charAt(index); if (char === "\\") { index += 2; @@ -160,7 +161,8 @@ export function parseLinkTitle( export function skipInlineWhitespace(text: string, start: number): number { let index = start; let seenLineEnding = false; - while (index < text.length) { + // No separate "in range" guard: running off the end of text makes charAt(index) "", which is neither " " nor "\t" nor "\n", so the character-kind check below already breaks the loop on that same condition -- a guard here would only ever fire at a point this loop already stops at. + for (;;) { const char = text.charAt(index); if (char === "\n") { if (seenLineEnding) { diff --git a/packages/markdown-codec/src/inline/math.test.ts b/packages/markdown-codec/src/inline/math.test.ts new file mode 100644 index 000000000..b7228cdf5 --- /dev/null +++ b/packages/markdown-codec/src/inline/math.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { matchMathInlineSpan } from "./math"; + +describe("matchMathInlineSpan", () => { + it("matches a real \\(...\\) span, delimiters included", () => { + expect(matchMathInlineSpan("\\(x^2\\)", 0)).toBe("\\(x^2\\)"); + }); + + it("returns undefined for a bare '\\(' with no closing '\\)' anywhere", () => { + expect(matchMathInlineSpan("\\(unterminated", 0)).toBeUndefined(); + }); + + it("returns undefined when the char at index is not a backslash, even with a literal '(' immediately after and a '\\)' reachable later", () => { + // A=charAt(index)!=='\\' is true, B=charAt(index+1)!=='(' is false -- neither guard clause alone should let the scan fall through to a bogus match against the trailing '\)'. + expect(matchMathInlineSpan("x(later\\)", 0)).toBeUndefined(); + }); + + it("returns undefined for a backslash not followed by '(', even with a '\\)' reachable later", () => { + // A=charAt(index)!=='\\' is false, B=charAt(index+1)!=='(' is true. + expect(matchMathInlineSpan("\\xlater\\)", 0)).toBeUndefined(); + }); + + it("searches for the closing '\\)' starting strictly after the opening '\\(', never before it", () => { + // A bogus "\)" sits just before the real "\(x\)" span; searching backwards from the opener (an off-by-arithmetic-sign bug) would match that bogus pair instead of the real close two characters further in. + const text = "abc\\)\\(x\\)"; + const openerIndex = text.indexOf("\\("); + expect(matchMathInlineSpan(text, openerIndex)).toBe("\\(x\\)"); + }); +}); diff --git a/packages/markdown-codec/src/inline/node.test.ts b/packages/markdown-codec/src/inline/node.test.ts new file mode 100644 index 000000000..3a27c3e77 --- /dev/null +++ b/packages/markdown-codec/src/inline/node.test.ts @@ -0,0 +1,128 @@ +import { describe, expect, it } from "vitest"; +import { InlineNode, createTextNode } from "./node"; + +describe("InlineNode field defaults", () => { + it("defaults literal/destination/raw/label to the empty string for a node kind that never sets them", () => { + const node = new InlineNode("link"); + expect(node.literal).toBe(""); + expect(node.destination).toBe(""); + expect(node.raw).toBe(""); + expect(node.label).toBe(""); + }); +}); + +describe("InlineNode.appendChild / unlink", () => { + it("links three children in order under one parent", () => { + const parent = new InlineNode("container"); + const a = createTextNode("a"); + const b = createTextNode("b"); + const c = createTextNode("c"); + parent.appendChild(a); + parent.appendChild(b); + parent.appendChild(c); + + expect(parent.firstChild).toBe(a); + expect(parent.lastChild).toBe(c); + expect(a.next).toBe(b); + expect(b.previous).toBe(a); + expect(b.next).toBe(c); + expect(c.previous).toBe(b); + }); + + it("unlink() removes a middle node and re-links its former neighbours to each other", () => { + const parent = new InlineNode("container"); + const a = createTextNode("a"); + const b = createTextNode("b"); + const c = createTextNode("c"); + parent.appendChild(a); + parent.appendChild(b); + parent.appendChild(c); + + b.unlink(); + + expect(a.next).toBe(c); + expect(c.previous).toBe(a); + expect(parent.firstChild).toBe(a); + expect(parent.lastChild).toBe(c); + }); + + it("unlink() fixes up the parent's firstChild/lastChild when the removed node was an end", () => { + const parent = new InlineNode("container"); + const a = createTextNode("a"); + const b = createTextNode("b"); + parent.appendChild(a); + parent.appendChild(b); + + a.unlink(); + expect(parent.firstChild).toBe(b); + expect(b.previous).toBeUndefined(); + + b.unlink(); + expect(parent.lastChild).toBeUndefined(); + }); +}); + +describe("InlineNode.insertAfter", () => { + it("inserts a brand-new node right after this one", () => { + const parent = new InlineNode("container"); + const a = createTextNode("a"); + const c = createTextNode("c"); + parent.appendChild(a); + parent.appendChild(c); + const b = createTextNode("b"); + + a.insertAfter(b); + + expect(a.next).toBe(b); + expect(b.previous).toBe(a); + expect(b.next).toBe(c); + expect(c.previous).toBe(b); + }); + + it("detaches a node from its OLD location before splicing it into its new one", () => { + // b starts out linked between a and c under `oldParent`; inserting it after x under a different parent must first unlink it from the old chain, or a/c are left with stale pointers to a node that no longer belongs there. + const oldParent = new InlineNode("container"); + const a = createTextNode("a"); + const b = createTextNode("b"); + const c = createTextNode("c"); + oldParent.appendChild(a); + oldParent.appendChild(b); + oldParent.appendChild(c); + + const newParent = new InlineNode("container"); + const x = createTextNode("x"); + newParent.appendChild(x); + + x.insertAfter(b); + + expect(a.next).toBe(c); + expect(c.previous).toBe(a); + expect(oldParent.lastChild).toBe(c); + expect(x.next).toBe(b); + expect(b.parent).toBe(newParent); + }); + + it("updates the parent's own lastChild when the sibling is inserted at the end", () => { + const parent = new InlineNode("container"); + const a = createTextNode("a"); + parent.appendChild(a); + const b = createTextNode("b"); + + a.insertAfter(b); + + expect(parent.lastChild).toBe(b); + }); + + it("leaves the parent's own lastChild unchanged when the sibling is inserted before an existing later node", () => { + const parent = new InlineNode("container"); + const a = createTextNode("a"); + const c = createTextNode("c"); + parent.appendChild(a); + parent.appendChild(c); + const b = createTextNode("b"); + + a.insertAfter(b); + + expect(parent.lastChild).toBe(c); + }); +}); diff --git a/packages/markdown-codec/src/lower/front-matter.test.ts b/packages/markdown-codec/src/lower/front-matter.test.ts new file mode 100644 index 000000000..4adfbd3fb --- /dev/null +++ b/packages/markdown-codec/src/lower/front-matter.test.ts @@ -0,0 +1,251 @@ +// Direct unit tests for extractFrontMatter's own scalar/keyword-list parsing and block-boundary scanning -- lower.test.ts (round-tripped through readMarkdown) only exercises whichever quoting/whitespace/malformed shapes its own fixtures happen to contain, never the exact quote-length boundary (a lone quote character, an empty quoted value), a mismatched-bracket keywords list, a blank line inside the block, or a document with no front matter at all. + +import { describe, expect, it } from "vitest"; +import { MarkdownDiagnosticCodes } from "../diagnostics/diagnostics"; +import { createDiagnosticCollector } from "../test-support/diagnostics"; +import { extractFrontMatter } from "./front-matter"; + +describe("extractFrontMatter: no front matter present at all", () => { + it("leaves an ordinary document entirely unchanged", () => { + const result = extractFrontMatter("# Title\n\nbody\n"); + expect(result).toStrictEqual({ + metadata: {}, + rest: "# Title\n\nbody\n", + source: undefined, + }); + }); + + it("leaves a document with an unclosed leading '---' unchanged -- CommonMark's own thematic-break-then-paragraph reading", () => { + const source = "---\ntitle: x\nno closing delimiter\n"; + const result = extractFrontMatter(source); + expect(result).toStrictEqual({ + metadata: {}, + rest: source, + source: undefined, + }); + }); + + it("leaves a document unchanged even when a later line happens to look like a closing delimiter, since the first line never opened a block at all", () => { + // The first line's own check must genuinely gate the whole function: without it, a body that merely contains a bare "---" or "..." later on could be misread as if it closed a front-matter block that was never opened. + const source = "not front matter\n---\nbody\n"; + const result = extractFrontMatter(source); + expect(result).toStrictEqual({ + metadata: {}, + rest: source, + source: undefined, + }); + }); +}); + +describe("extractFrontMatter: closing delimiter shapes", () => { + it("accepts '...' as a closing delimiter, not just a second '---'", () => { + const result = extractFrontMatter("---\ntitle: x\n...\nbody\n"); + expect(result.metadata).toStrictEqual({ title: "x" }); + expect(result.rest).toBe("body\n"); + }); + + it("skips a blank line inside the block without ending it", () => { + const result = extractFrontMatter( + "---\ntitle: x\n\nauthor: y\n---\nbody\n", + ); + expect(result.metadata).toStrictEqual({ title: "x", author: "y" }); + }); + + it("silently skips a line that is not key: value shaped, with no diagnostic", () => { + const collector = createDiagnosticCollector(); + const result = extractFrontMatter( + "---\ntitle: x\nnot a key value line\n---\nbody\n", + collector.sink, + ); + expect(result.metadata).toStrictEqual({ title: "x" }); + expect(collector.diagnostics).toHaveLength(0); + }); +}); + +describe("extractFrontMatter: scalar quote stripping, at the exact length-2 boundary", () => { + it("strips a genuinely double-quoted value", () => { + expect( + extractFrontMatter('---\ntitle: "abc"\n---\n').metadata, + ).toStrictEqual({ + title: "abc", + }); + }); + + it("strips a genuinely single-quoted value", () => { + expect( + extractFrontMatter("---\ntitle: 'abc'\n---\n").metadata, + ).toStrictEqual({ + title: "abc", + }); + }); + + it("strips an empty double-quoted value (length exactly 2)", () => { + expect(extractFrontMatter('---\ntitle: ""\n---\n').metadata).toStrictEqual({ + title: "", + }); + }); + + it("does not strip a lone quote character (length 1, below the boundary)", () => { + expect(extractFrontMatter('---\ntitle: "\n---\n').metadata).toStrictEqual({ + title: '"', + }); + }); + + it("does not strip when only the opening quote matches -- no closing quote at all", () => { + expect( + extractFrontMatter('---\ntitle: "abc\n---\n').metadata, + ).toStrictEqual({ + title: '"abc', + }); + }); + + it("does not strip when only the closing quote matches -- no opening quote at all", () => { + expect( + extractFrontMatter('---\ntitle: abc"\n---\n').metadata, + ).toStrictEqual({ + title: 'abc"', + }); + }); + + it("does not strip mismatched quote kinds (opens single, closes double)", () => { + expect( + extractFrontMatter(`---\ntitle: 'abc"\n---\n`).metadata, + ).toStrictEqual({ + title: `'abc"`, + }); + }); + + it("leaves an unquoted value untouched", () => { + expect(extractFrontMatter("---\ntitle: abc\n---\n").metadata).toStrictEqual( + { + title: "abc", + }, + ); + }); + + // The single-quote checks mirror the double-quote ones above exactly -- isDoubleQuoted short-circuits on startsWith('"') before ever reaching endsWith for a single-quoted value, so only a value that itself exercises isSingleQuoted's own length/startsWith/endsWith checks at each boundary can kill a mutant in it. + it("does not strip a lone single-quote character (length 1, below the boundary)", () => { + expect(extractFrontMatter("---\ntitle: '\n---\n").metadata).toStrictEqual({ + title: "'", + }); + }); + + it("strips an empty single-quoted value (length exactly 2)", () => { + expect(extractFrontMatter("---\ntitle: ''\n---\n").metadata).toStrictEqual({ + title: "", + }); + }); + + it("does not strip when only the opening single quote matches -- no closing quote at all", () => { + expect( + extractFrontMatter("---\ntitle: 'abc\n---\n").metadata, + ).toStrictEqual({ + title: "'abc", + }); + }); + + it("does not strip when only the closing single quote matches -- no opening quote at all", () => { + expect( + extractFrontMatter("---\ntitle: abc'\n---\n").metadata, + ).toStrictEqual({ + title: "abc'", + }); + }); +}); + +describe("extractFrontMatter: keywords, both the bracketed and the bare comma-separated shape", () => { + it("parses a bracketed flow-sequence list", () => { + expect( + extractFrontMatter("---\nkeywords: [a, b, c]\n---\n").metadata.keywords, + ).toStrictEqual(["a", "b", "c"]); + }); + + it("parses a bare comma-separated fallback with no brackets at all", () => { + expect( + extractFrontMatter("---\nkeywords: a, b, c\n---\n").metadata.keywords, + ).toStrictEqual(["a", "b", "c"]); + }); + + it("trims outer whitespace around a bracketed list before checking for the brackets", () => { + expect( + extractFrontMatter("---\nkeywords: [a, b] \n---\n").metadata.keywords, + ).toStrictEqual(["a", "b"]); + }); + + it("does not treat a value as bracketed when only the opening bracket is present", () => { + // Malformed: starts with "[" but never closes -- read as one bare comma-separated line instead, exactly as this module's own "not a real YAML parser" scope promises. The unstripped leading "[" survives on the first item. + expect( + extractFrontMatter("---\nkeywords: [a, b\n---\n").metadata.keywords, + ).toStrictEqual(["[a", "b"]); + }); + + it("filters out an empty item from a trailing comma", () => { + expect( + extractFrontMatter("---\nkeywords: a, b,\n---\n").metadata.keywords, + ).toStrictEqual(["a", "b"]); + }); + + it("keeps a genuinely single-character item, right at the length-0 filter boundary", () => { + expect( + extractFrontMatter("---\nkeywords: a,,b\n---\n").metadata.keywords, + ).toStrictEqual(["a", "b"]); + }); + + it("does not treat a value as bracketed when only the closing bracket is present", () => { + // Malformed the other way round: ends with "]" but never opens -- still read as one bare comma-separated line, since both the opening AND closing bracket are required together. The unstripped trailing "]" survives on the last item. + expect( + extractFrontMatter("---\nkeywords: a, b]\n---\n").metadata.keywords, + ).toStrictEqual(["a", "b]"]); + }); +}); + +describe("extractFrontMatter: direction, a two-member enum that silently drops any other value", () => { + it("maps both recognised direction values", () => { + expect( + extractFrontMatter("---\ndirection: rtl\n---\n").metadata, + ).toStrictEqual({ + direction: "rtl", + }); + expect( + extractFrontMatter("---\ndirection: ltr\n---\n").metadata, + ).toStrictEqual({ + direction: "ltr", + }); + }); + + it("silently drops an unrecognised direction value -- no FRONT_MATTER_KEY_UNMAPPED, since the key itself is recognised", () => { + const collector = createDiagnosticCollector(); + const result = extractFrontMatter( + "---\ndirection: sideways\n---\n", + collector.sink, + ); + expect(result.metadata).toStrictEqual({}); + expect(collector.diagnostics).toHaveLength(0); + }); +}); + +describe("extractFrontMatter: an unrecognised key reports FRONT_MATTER_KEY_UNMAPPED with its own exact message and 1-based line number", () => { + it("fires with the key name and the line it appeared on", () => { + const collector = createDiagnosticCollector(); + const result = extractFrontMatter( + "---\ntitle: x\ncustomField: y\n---\n", + collector.sink, + ); + expect(result.metadata).toStrictEqual({ title: "x" }); + expect(collector.diagnostics).toHaveLength(1); + expect(collector.diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.FRONT_MATTER_KEY_UNMAPPED, + message: + 'front matter key "customField" has no LayoutMetadata equivalent and was dropped from the metadata; its original spelling survives in the verbatim front-matter block this package\'s own writer can re-emit', + line: 3, + }); + }); +}); + +describe("extractFrontMatter: rest and source split exactly at the closing delimiter", () => { + it("carries the verbatim block (delimiters included) as source and everything after as rest", () => { + const result = extractFrontMatter("---\ntitle: x\n---\nbody\nmore\n"); + expect(result.source).toBe("---\ntitle: x\n---"); + expect(result.rest).toBe("body\nmore\n"); + }); +}); diff --git a/packages/markdown-codec/src/lower/front-matter.ts b/packages/markdown-codec/src/lower/front-matter.ts index f25bac1c4..ce9eb8744 100644 --- a/packages/markdown-codec/src/lower/front-matter.ts +++ b/packages/markdown-codec/src/lower/front-matter.ts @@ -114,23 +114,19 @@ export function extractFrontMatter( return { metadata: {}, rest: source, source: undefined }; } - let closingIndex = -1; - for (let index = 1; index < lines.length; index += 1) { - if (CLOSING_DELIMITER_PATTERN.test(lines[index] ?? "")) { - closingIndex = index; - break; - } - } + // Both loops below scan a slice (never a manually-bounded index/length comparison against the full array) and derive the 1-based line number from the slice's own offset -- lines[index] within either slice's real bounds is always a defined string (split() never produces a sparse array), so there is no further "missing element" fallback to write either. + const closingOffset = lines + .slice(1) + .findIndex((line) => CLOSING_DELIMITER_PATTERN.test(line)); + const closingIndex = closingOffset === -1 ? -1 : closingOffset + 1; if (closingIndex === -1) { return { metadata: {}, rest: source, source: undefined }; } const metadata: MutableLayoutMetadata = {}; - for (let index = 1; index < closingIndex; index += 1) { - const line = lines[index] ?? ""; - if (line.trim().length === 0) { - continue; - } + for (const [offset, line] of lines.slice(1, closingIndex).entries()) { + const index = offset + 1; + // No separate blank-line skip: a blank (or all-whitespace) line never matches KEY_VALUE_LINE_PATTERN either (it requires a leading identifier character), so it already falls through to the "not key: value shaped" skip below -- a dedicated check here would only ever repeat a skip the match failure already produces on its own. const match = KEY_VALUE_LINE_PATTERN.exec(line); const key = match?.[1]; const value = match?.[2]; diff --git a/packages/markdown-codec/src/lower/image.test.ts b/packages/markdown-codec/src/lower/image.test.ts new file mode 100644 index 000000000..f38da9600 --- /dev/null +++ b/packages/markdown-codec/src/lower/image.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { bytesToBase64 } from "../image/image"; +import { resolveMarkdownImage } from "./image"; + +// A minimal, otherwise-valid PNG signature + IHDR chunk with an asymmetric width/height (300x100) so a widthPt/heightPt swap or a wrong operator on either axis produces a value distinct from the other, rather than two coincidentally-equal numbers. +function pngBytes(widthPx: number, heightPx: number): Uint8Array { + const bytes = new Uint8Array(29); + bytes.set([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a], 0); + bytes.set([0x00, 0x00, 0x00, 0x0d], 8); // IHDR chunk length + bytes.set([0x49, 0x48, 0x44, 0x52], 12); // 'IHDR' + const view = new DataView(bytes.buffer); + view.setUint32(16, widthPx, false); + view.setUint32(20, heightPx, false); + return bytes; +} + +const CSS_PIXELS_PER_INCH = 96; +const POINTS_PER_INCH = 72; +const POINTS_PER_PIXEL = POINTS_PER_INCH / CSS_PIXELS_PER_INCH; + +describe("resolveMarkdownImage", () => { + it("converts an asymmetric PNG's own width/height in pixels to points independently, on the correct axis", () => { + const png = pngBytes(300, 100); + const destination = `data:image/png;base64,${bytesToBase64(png)}`; + const resolved = resolveMarkdownImage(destination, { alt: "" }, undefined); + expect(resolved?.widthPt).toBeCloseTo(300 * POINTS_PER_PIXEL); + expect(resolved?.heightPt).toBeCloseTo(100 * POINTS_PER_PIXEL); + }); +}); diff --git a/packages/markdown-codec/src/lower/inline.test.ts b/packages/markdown-codec/src/lower/inline.test.ts new file mode 100644 index 000000000..708b12d1f --- /dev/null +++ b/packages/markdown-codec/src/lower/inline.test.ts @@ -0,0 +1,244 @@ +// Direct unit tests for lowerInlineNodes' own leaf-by-leaf construction, isolated from the parser -- the round-trip suites in lower.test.ts exercise this module only through whatever shapes the real CommonMark parser happens to produce, which never reaches several of its own branches (an empty inline rawHtml literal, the rawHtml: "drop" branch for an INLINE tag specifically, an empty text/entity node, a nested bold-in-bold or strike-in-strike pair, an untitled image). Building MarkdownInlineNode trees by hand here reaches those directly and pins the exact diagnostic message text lower.test.ts's own `collector.has(code)` checks never inspect. + +import { describe, expect, it } from "vitest"; +import type { MarkdownInlineNode } from "../ast/ast"; +import { MarkdownDiagnosticCodes } from "../diagnostics/diagnostics"; +import { + MATH_INLINE_FONT_MARKER, + MONOSPACE_FONT_FAMILY, +} from "../shared/style-constants"; +import { createDiagnosticCollector } from "../test-support/diagnostics"; +import { lowerCodeBlockRun, lowerInlineNodes } from "./inline"; + +function lower( + nodes: MarkdownInlineNode[], + rawHtml: "preserve" | "drop" = "preserve", +) { + const collector = createDiagnosticCollector(); + const result = lowerInlineNodes(nodes, { sink: collector.sink, rawHtml }); + return { ...result, diagnostics: collector.diagnostics }; +} + +describe("lowerInlineNodes: buildRun's own conditional fields", () => { + it("a run with no active style carries only its own text -- no bold/italic/strike/hyperlink/fontFamily key at all, not even set to undefined", () => { + const { runs } = lower([{ type: "text", value: "plain" }]); + expect(runs).toHaveLength(1); + expect(Object.keys(runs[0]!).sort()).toStrictEqual(["text"]); + }); +}); + +describe("lowerInlineNodes: text and entity leaves drop entirely when empty", () => { + it("an empty text node produces no run at all", () => { + const { runs } = lower([{ type: "text", value: "" }]); + expect(runs).toHaveLength(0); + }); + + it("a non-empty text node produces exactly one run", () => { + const { runs } = lower([{ type: "text", value: "x" }]); + expect(runs).toHaveLength(1); + expect(runs[0]!.text).toBe("x"); + }); + + it("an empty entity node produces no run at all", () => { + const { runs } = lower([{ type: "entity", raw: "�", value: "" }]); + expect(runs).toHaveLength(0); + }); + + it("a non-empty entity node produces exactly one run carrying its resolved value", () => { + const { runs } = lower([{ type: "entity", raw: "&", value: "&" }]); + expect(runs).toHaveLength(1); + expect(runs[0]!.text).toBe("&"); + }); +}); + +describe("lowerInlineNodes: NESTED_EMPHASIS_FLATTENED fires per kind with its own precise message, only when genuinely nested", () => { + it("a single (non-nested) emphasis span fires no diagnostic", () => { + const { diagnostics } = lower([ + { + type: "emphasis", + marker: "_", + children: [{ type: "text", value: "a" }], + }, + ]); + expect(diagnostics).toHaveLength(0); + }); + + it("a single (non-nested) strong span fires no diagnostic", () => { + const { diagnostics } = lower([ + { type: "strong", marker: "*", children: [{ type: "text", value: "a" }] }, + ]); + expect(diagnostics).toHaveLength(0); + }); + + it("a single (non-nested) strikethrough span fires no diagnostic", () => { + const { diagnostics } = lower([ + { type: "strikethrough", children: [{ type: "text", value: "a" }] }, + ]); + expect(diagnostics).toHaveLength(0); + }); + + it("emphasis nested inside emphasis fires with the 'emphasis' wording", () => { + const { diagnostics, runs } = lower([ + { + type: "emphasis", + marker: "_", + children: [ + { + type: "emphasis", + marker: "*", + children: [{ type: "text", value: "a" }], + }, + ], + }, + ]); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.NESTED_EMPHASIS_FLATTENED, + message: + "a emphasis span is nested inside another span of the same kind; ContentRun has no nesting depth of its own, so both collapse to one flat run", + }); + expect(runs[0]).toMatchObject({ italic: true }); + }); + + it("strong nested inside strong fires with the 'strong emphasis' wording", () => { + const { diagnostics, runs } = lower([ + { + type: "strong", + marker: "*", + children: [ + { + type: "strong", + marker: "_", + children: [{ type: "text", value: "a" }], + }, + ], + }, + ]); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.NESTED_EMPHASIS_FLATTENED, + message: + "a strong emphasis span is nested inside another span of the same kind; ContentRun has no nesting depth of its own, so both collapse to one flat run", + }); + expect(runs[0]).toMatchObject({ bold: true }); + }); + + it("strikethrough nested inside strikethrough fires with the 'strikethrough' wording", () => { + const { diagnostics, runs } = lower([ + { + type: "strikethrough", + children: [ + { + type: "strikethrough", + children: [{ type: "text", value: "a" }], + }, + ], + }, + ]); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.NESTED_EMPHASIS_FLATTENED, + message: + "a strikethrough span is nested inside another span of the same kind; ContentRun has no nesting depth of its own, so both collapse to one flat run", + }); + expect(runs[0]).toMatchObject({ strike: true }); + }); +}); + +describe("lowerInlineNodes: inline rawHtml, both modes, both an empty and a non-empty literal", () => { + it('rawHtml: "drop" fires RAW_HTML_DROPPED with its own exact message and produces no run', () => { + const { runs, diagnostics } = lower( + [{ type: "rawHtml", literal: "" }], + "drop", + ); + expect(runs).toHaveLength(0); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.RAW_HTML_DROPPED, + message: 'inline raw HTML was dropped per the rawHtml: "drop" option', + }); + }); + + it('rawHtml: "preserve" fires RAW_HTML_PRESERVED_AS_TEXT with its own exact message even for an empty literal, and produces no run since there is no text to carry', () => { + const { runs, diagnostics } = lower( + [{ type: "rawHtml", literal: "" }], + "preserve", + ); + expect(runs).toHaveLength(0); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.RAW_HTML_PRESERVED_AS_TEXT, + message: + "inline raw HTML was preserved as literal text; it will not be rendered as HTML by any consumer of the resulting ContentDocument, and its verbatim original rides the run's own markdown residue for this package's writer to re-emit as-is", + }); + }); + + it('rawHtml: "preserve" with a non-empty literal produces one run carrying the literal as both text and markdown residue', () => { + const { runs, diagnostics } = lower( + [{ type: "rawHtml", literal: '' }], + "preserve", + ); + expect(diagnostics).toHaveLength(1); + expect(runs).toHaveLength(1); + expect(runs[0]).toMatchObject({ + text: '', + source: { format: "markdown", xml: '' }, + }); + }); +}); + +describe("lowerInlineNodes: mathInline preserves its own exact diagnostic message and marks the run", () => { + it("fires MATH_INLINE_PRESERVED_AS_TEXT with its own exact message and marks the run with MATH_INLINE_FONT_MARKER", () => { + const { runs, diagnostics } = lower([ + { type: "mathInline", literal: "x^2" }, + ]); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.MATH_INLINE_PRESERVED_AS_TEXT, + message: + "inline math (\\( \\)) was preserved as literal raw LaTeX text; it is not parsed as LaTeX or converted to MathML by this package", + }); + expect(runs).toHaveLength(1); + expect(runs[0]).toMatchObject({ + text: "x^2", + fontFamily: MATH_INLINE_FONT_MARKER, + }); + }); +}); + +describe("lowerInlineNodes: a nested image's title drops with its own exact message, only when a title is present", () => { + it("an untitled nested image fires no diagnostic at all", () => { + const { diagnostics, runs } = lower([ + { type: "image", destination: "/x.png", alt: "alt text" }, + ]); + expect(diagnostics).toHaveLength(0); + expect(runs[0]).toMatchObject({ text: "alt text", hyperlink: "/x.png" }); + }); + + it("a titled nested image fires LINK_TITLE_DROPPED with its own exact message naming the dropped title", () => { + const { diagnostics, runs } = lower([ + { + type: "image", + destination: "/x.png", + title: "a title", + alt: "alt text", + }, + ]); + expect(diagnostics).toHaveLength(1); + expect(diagnostics[0]).toMatchObject({ + code: MarkdownDiagnosticCodes.LINK_TITLE_DROPPED, + message: + 'image title "a title" has no ContentRun equivalent and was dropped', + }); + expect(runs[0]).toMatchObject({ text: "alt text", hyperlink: "/x.png" }); + }); +}); + +describe("lowerCodeBlockRun", () => { + it("wraps a code block's literal text in a single monospace run", () => { + expect(lowerCodeBlockRun("console.log(1);")).toStrictEqual({ + text: "console.log(1);", + fontFamily: MONOSPACE_FONT_FAMILY, + }); + }); +}); diff --git a/packages/markdown-codec/src/lower/inline.ts b/packages/markdown-codec/src/lower/inline.ts index 5a502d277..be797a2ff 100644 --- a/packages/markdown-codec/src/lower/inline.ts +++ b/packages/markdown-codec/src/lower/inline.ts @@ -75,9 +75,8 @@ function lowerInlineNodeInto( extents: RunConstructExtent[], ): void { switch (node.type) { + // text and entity both carry their materialised text in the same field, and are handled identically -- one shared body, rather than two separately-mutable cases whose bodies are textually forced to stay identical anyway. case "text": - if (node.value.length > 0) runs.push(buildRun(node.value, style)); - return; case "entity": if (node.value.length > 0) runs.push(buildRun(node.value, style)); return; diff --git a/packages/markdown-codec/src/lower/lower.test.ts b/packages/markdown-codec/src/lower/lower.test.ts index f3989dcb6..66fdddddb 100644 --- a/packages/markdown-codec/src/lower/lower.test.ts +++ b/packages/markdown-codec/src/lower/lower.test.ts @@ -359,6 +359,27 @@ describe("GFM tables", () => { runs: [{ text: "1" }], }); }); + + it("divides the section's own content width evenly across columns, not some other arithmetic on the same two numbers", () => { + const [table] = blocks("| a | b |\n| - | - |\n| 1 | 2 |", { + pageSize: { widthPt: 220, heightPt: 800 }, + margins: { topPt: 72, rightPt: 10, bottomPt: 72, leftPt: 10 }, + }); + if (table?.kind !== "table") throw new Error("expected a table block"); + expect(table.columnWidthsPt).toEqual([100, 100]); + }); + + it("carries no constructs key on a cell with no run-level constructs of its own", () => { + const [table] = blocks("| a |\n| - |\n| 1 |"); + if (table?.kind !== "table") throw new Error("expected a table block"); + expect(table.rows[0]?.cells[0]?.blocks[0]).not.toHaveProperty("constructs"); + }); + + it("carries no alignment key on a column the delimiter row leaves unaligned", () => { + const [table] = blocks("| a |\n| - |\n| 1 |"); + if (table?.kind !== "table") throw new Error("expected a table block"); + expect(table.rows[0]?.cells[0]?.blocks[0]).not.toHaveProperty("alignment"); + }); }); describe("images", () => { diff --git a/packages/markdown-codec/src/package.test.ts b/packages/markdown-codec/src/package.test.ts index 92ea0800e..89b93b05b 100644 --- a/packages/markdown-codec/src/package.test.ts +++ b/packages/markdown-codec/src/package.test.ts @@ -229,8 +229,19 @@ describe("writeMarkdown: DocumentTree -> markdown text", () => { sheets: [], }); - expect(() => writeMarkdown(spreadsheet)).toThrow( - MarkdownUnsupportedDocumentKindError, + let thrown: unknown; + try { + writeMarkdown(spreadsheet); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(MarkdownUnsupportedDocumentKindError); + const typed = thrown as MarkdownUnsupportedDocumentKindError; + expect(typed.name).toBe("MarkdownUnsupportedDocumentKindError"); + expect(typed.kind).toBe("spreadsheet"); + expect(typed.code).toBe("md/write-side-not-wordprocessing"); + expect(typed.message).toBe( + "writeMarkdown only supports a 'wordprocessing' ContentDocument, got 'spreadsheet'", ); }); @@ -242,8 +253,15 @@ describe("writeMarkdown: DocumentTree -> markdown text", () => { children: [], }; - expect(() => writeMarkdown(formula)).toThrow( - MarkdownUnsupportedDocumentKindError, + let thrown: unknown; + try { + writeMarkdown(formula); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(MarkdownUnsupportedDocumentKindError); + expect((thrown as MarkdownUnsupportedDocumentKindError).kind).toBe( + "formula", ); }); @@ -261,10 +279,17 @@ describe("writeMarkdown: DocumentTree -> markdown text", () => { readMarkdown(BLOCKQUOTED).documentPackage; expect(styles).toBeDefined(); - expect(() => writeMarkdown(packageWithoutStyles)).toThrow( - MarkdownPackageFlattenError, - ); - expect(() => writeMarkdown(packageWithoutStyles)).toThrow(/style ref/); + let thrown: unknown; + try { + writeMarkdown(packageWithoutStyles); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(MarkdownPackageFlattenError); + const typed = thrown as MarkdownPackageFlattenError; + expect(typed.name).toBe("MarkdownPackageFlattenError"); + expect(typed.code).toBe("md/package-flatten-failed"); + expect(typed.message).toMatch(/style ref/); }); it("reports a PACKAGE_TABLE_DROPPED diagnostic per non-empty package-level table flattenTree cannot carry into markdown", () => { @@ -305,6 +330,58 @@ describe("writeMarkdown: DocumentTree -> markdown text", () => { ).toHaveLength(0); expect(written).toBe(writeMarkdown(base)); }); + + it("reports nothing for a present but genuinely EMPTY table -- the guard is a real emptiness check, not merely 'is the key present'", () => { + const base = readMarkdown(SAMPLE).documentPackage; + const withEmptyTables = { + ...base, + definitions: {}, + layers: {}, + attachments: {}, + destinations: {}, + pages: [], + }; + const collector = createDiagnosticCollector(); + + writeMarkdown(withEmptyTables, { sink: collector.sink }); + + expect(collector.has(MarkdownDiagnosticCodes.PACKAGE_TABLE_DROPPED)).toBe( + false, + ); + }); + + it("names the specific table in each PACKAGE_TABLE_DROPPED diagnostic's own message", () => { + const base = readMarkdown(SAMPLE).documentPackage; + const withExtraTables = { + ...base, + definitions: { d1: { kind: "bookmark" } }, + layers: { l1: { kind: "layer" } }, + attachments: { a1: { kind: "file" } }, + destinations: { dest1: { kind: "anchor" } }, + pages: [{ widthPt: 100, heightPt: 100 }], + }; + const collector = createDiagnosticCollector(); + + writeMarkdown(withExtraTables, { sink: collector.sink }); + + const messages = collector.diagnostics + .filter( + (diagnostic) => + diagnostic.code === MarkdownDiagnosticCodes.PACKAGE_TABLE_DROPPED, + ) + .map((diagnostic) => diagnostic.message); + for (const name of [ + "definitions", + "layers", + "attachments", + "destinations", + "pages", + ]) { + expect(messages.some((message) => message.includes(`"${name}"`))).toBe( + true, + ); + } + }); }); describe("the construct-group path over footnote shapes beyond SAMPLE's single case", () => { @@ -379,6 +456,14 @@ describe("tree-only carries: reference definitions and front-matter residue", () }); }); + it("splices a titleless link reference definition with no title key at all, not an undefined one", () => { + const { documentPackage } = readMarkdown("[foo]: /url\n\n[foo]"); + expect(documentPackage.definitions).toEqual({ + FOO: { kind: "link", destination: "/url" }, + }); + expect(documentPackage.definitions?.FOO).not.toHaveProperty("title"); + }); + it("leaves definitions and the package source table absent for a document with neither, so the package is exactly assembleTree of the flat document", () => { const { documentPackage } = readMarkdown("plain body"); expect(documentPackage.definitions).toBeUndefined(); @@ -400,6 +485,27 @@ describe("tree-only carries: reference definitions and front-matter residue", () ); }); + it("renders a titleless link definition with no trailing title clause at all", () => { + const written = writeMarkdown( + readMarkdown("[foo]: /url\n\n[foo]").documentPackage, + ); + expect(written).toBe("[foo](/url)\n\n[FOO]: /url"); + }); + + it("joins two rendered link definitions with a real newline, one per line", () => { + const written = writeMarkdown( + readMarkdown("[foo]: /url1\n\n[bar]: /url2\n\n[foo] and [bar]") + .documentPackage, + ); + const definitionLines = written.split("\n\n").at(-1)?.split("\n"); + expect(definitionLines).toEqual(["[FOO]: /url1", "[BAR]: /url2"]); + }); + + it("renders bare definitions with no leading blank line when the document's own body is empty", () => { + const written = writeMarkdown(readMarkdown("[foo]: /url").documentPackage); + expect(written).toBe("[FOO]: /url"); + }); + it("round-trips text -> package -> text -> package to the identical package and text, definitions included", () => { const source = '[foo]: /url "the title"\n\nuse [foo] here.'; const first = readMarkdown(source).documentPackage; @@ -481,6 +587,65 @@ describe("tree-only carries: reference definitions and front-matter residue", () ).toEqual(metadata); }); + it("quotes and escapes a literal backslash inside a value that also needs quoting for its leading '-'", () => { + const base = readMarkdown("body").documentPackage; + const written = writeMarkdown( + { ...base, metadata: { ...base.metadata, title: "-\\" } }, + { frontMatter: true }, + ); + expect(written).toBe('---\ntitle: "-\\\\"\n---\n\nbody'); + }); + + it("quotes and escapes a literal double-quote inside a value that also needs quoting for its leading '-'", () => { + const base = readMarkdown("body").documentPackage; + const written = writeMarkdown( + { ...base, metadata: { ...base.metadata, title: '-"' } }, + { frontMatter: true }, + ); + expect(written).toBe('---\ntitle: "-\\""\n---\n\nbody'); + }); + + it("quotes a value that would otherwise be misread, for reasons NEEDS_QUOTING_PATTERN alone cannot catch: leading/trailing whitespace or an empty string", () => { + const base = readMarkdown("body").documentPackage; + expect( + writeMarkdown( + { ...base, metadata: { ...base.metadata, title: " leading space" } }, + { frontMatter: true }, + ), + ).toBe('---\ntitle: " leading space"\n---\n\nbody'); + expect( + writeMarkdown( + { ...base, metadata: { ...base.metadata, title: "trailing space " } }, + { frontMatter: true }, + ), + ).toBe('---\ntitle: "trailing space "\n---\n\nbody'); + expect( + writeMarkdown( + { ...base, metadata: { ...base.metadata, title: "" } }, + { frontMatter: true }, + ), + ).toBe('---\ntitle: ""\n---\n\nbody'); + }); + + it("omits the keywords line entirely for an empty (but defined) keywords array, rather than emitting an empty flow sequence", () => { + const base = readMarkdown("body").documentPackage; + const written = writeMarkdown( + { + ...base, + metadata: { ...base.metadata, title: "x", keywords: [] }, + }, + { frontMatter: true }, + ); + expect(written).toBe("---\ntitle: x\n---\n\nbody"); + expect(written).not.toContain("keywords"); + }); + + it("emits no front-matter block at all (returns the body untouched) when the metadata carries none of the fields it maps", () => { + const base = readMarkdown("body").documentPackage; + // frontMatter: true with a metadata object none of STRING_FIELD_ENTRIES/keywords/direction can read anything from -- emitFrontMatter's own lines array stays empty, so it must return undefined (no block at all) rather than an empty "---\n---" shell. + expect(writeMarkdown(base, { frontMatter: true })).toBe("body"); + }); + it("emits no front matter at all without the option, residue or not", () => { const { documentPackage } = readMarkdown("---\ntitle: x\n---\n\nbody", { frontMatter: true, diff --git a/packages/markdown-codec/src/read.ts b/packages/markdown-codec/src/read.ts index 49f491f86..aebe5b6c7 100644 --- a/packages/markdown-codec/src/read.ts +++ b/packages/markdown-codec/src/read.ts @@ -98,23 +98,18 @@ export function readMarkdown( detail.frontMatterSource === undefined ? undefined : { format: "markdown", xml: detail.frontMatterSource }; - const documentPackage: DocumentTree = - definitions === undefined && frontMatterResidue === undefined - ? assembled - : { - ...assembled, - ...(definitions !== undefined - ? { definitions: { ...assembled.definitions, ...definitions } } - : {}), - ...(frontMatterResidue !== undefined - ? { - source: { - ...(assembled.source ?? {}), - frontmatter: frontMatterResidue, - }, - } - : {}), - }; + // No shortcut returning `assembled` unchanged when neither splice applies: the spread below is already a no-op in that case (spreading `undefined`/an absent key adds nothing), so the shortcut bought only reference identity a DocumentTree's own contract never promises, at the cost of a branch no value-level test could ever tell apart from always spreading. + const documentPackage: DocumentTree = { + ...assembled, + ...(definitions !== undefined + ? { definitions: { ...assembled.definitions, ...definitions } } + : {}), + ...(frontMatterResidue !== undefined + ? { + source: { ...assembled.source, frontmatter: frontMatterResidue }, + } + : {}), + }; return { documentPackage, diagnostics: detail.diagnostics }; } diff --git a/packages/markdown-codec/src/scan/scan.test.ts b/packages/markdown-codec/src/scan/scan.test.ts index e7524659b..b4a86b961 100644 --- a/packages/markdown-codec/src/scan/scan.test.ts +++ b/packages/markdown-codec/src/scan/scan.test.ts @@ -11,6 +11,8 @@ describe("MarkdownScanCursor", () => { expect(cursor.position).toEqual({ offset: 2, line: 1, column: 2 }); expect(cursor.atEnd()).toBe(true); expect(cursor.next()).toBeUndefined(); + // Calling next() again once already at the exact end must not advance any further state -- offset/column stay put rather than ticking past source.length. + expect(cursor.position).toEqual({ offset: 2, line: 1, column: 2 }); }); it("expands a tab at column 0 to the next 4-column tab stop, one column at a time", () => { @@ -68,12 +70,28 @@ describe("MarkdownScanCursor", () => { expect(cursor.position.column).toBe(1); }); + it("peek() normalises a raw '\\r' to '\\n', matching next()'s own line-ending normalisation", () => { + expect(new MarkdownScanCursor("\rx").peek()).toBe("\n"); + }); + + it("peek() returns undefined at the true end of input, with no pending tab", () => { + expect(new MarkdownScanCursor("").peek()).toBeUndefined(); + }); + it("peekRaw() reads real source characters, ignoring pending tab-expansion state", () => { const cursor = new MarkdownScanCursor("\tfoo"); cursor.next(); // consume the first of the tab's expanded columns; rawOffset stays at the tab itself expect(cursor.peekRaw(4)).toBe("\tfoo"); }); + it("peekRaw() returns only the requested slice, not the whole remaining source", () => { + const cursor = new MarkdownScanCursor("abcdef"); + expect(cursor.peekRaw(2)).toBe("ab"); + cursor.next(); + cursor.next(); + expect(cursor.peekRaw(2)).toBe("cd"); + }); + it("treats LF, CRLF, and lone CR as a single logical newline, resetting column and advancing line", () => { for (const [source, label] of [ ["a\nb", "LF"], @@ -105,6 +123,16 @@ describe("MarkdownScanCursor", () => { expect(cursor.next()).toBe("f"); }); + it("next() past the end of input is idempotent -- it never advances rawOffset or column further", () => { + const cursor = new MarkdownScanCursor("a"); + cursor.next(); + expect(cursor.next()).toBeUndefined(); + const markAfterFirstPastEnd = cursor.mark(); + expect(cursor.next()).toBeUndefined(); + expect(cursor.next()).toBeUndefined(); + expect(cursor.mark()).toEqual(markAfterFirstPastEnd); + }); + it("atEnd() is false while a tab expansion is still pending, even past the raw source length", () => { const cursor = new MarkdownScanCursor("\t"); // The lone tab at column 0 expands to 4 columns in total. diff --git a/packages/markdown-codec/src/scan/scan.ts b/packages/markdown-codec/src/scan/scan.ts index e15e70823..272cbe147 100644 --- a/packages/markdown-codec/src/scan/scan.ts +++ b/packages/markdown-codec/src/scan/scan.ts @@ -41,18 +41,15 @@ export class MarkdownScanCursor { }; } + // No separate `pendingTabColumns === 0` half here: rawOffset only ever advances past a tab once every one of its own columns has been consumed (next()'s own tab branch below), so rawOffset can never reach source.length while pendingTabColumns is still nonzero -- the two conditions were never independent, and checking rawOffset alone already answers exactly when this class considers itself done. atEnd(): boolean { - return this.pendingTabColumns === 0 && this.rawOffset >= this.source.length; + return this.rawOffset >= this.source.length; } // The next effective character without consuming it: a real source character, or a synthetic single space while a tab's own expansion is only partially consumed. Never returns '\t' or '\r' -- a tab's columns come back as ' ' one at a time, and a line ending (LF, CRLF, or lone CR) comes back as a single '\n', matching next()'s own normalisation. + // + // No `pendingTabColumns > 0` branch of its own, and no `rawOffset >= source.length` guard either: while a tab's expansion is only partly consumed, rawOffset still points AT that same tab character (see atEnd's own note), so reading `this.source[this.rawOffset]` here already finds '\t' and the ordinary tab branch below already answers " " for it; and past the end of input, indexing a string out of range is itself already `undefined` in JS, which matches every one of the comparisons below and falls out the far end as `undefined` on its own -- both cases this method needs to handle are already handled by the plain read. peek(): string | undefined { - if (this.pendingTabColumns > 0) { - return " "; - } - if (this.rawOffset >= this.source.length) { - return undefined; - } const char = this.source[this.rawOffset]; if (char === "\t") { return " "; diff --git a/packages/markdown-codec/src/shared/list-id.test.ts b/packages/markdown-codec/src/shared/list-id.test.ts index d7fc02784..ea66a1dd0 100644 --- a/packages/markdown-codec/src/shared/list-id.test.ts +++ b/packages/markdown-codec/src/shared/list-id.test.ts @@ -91,4 +91,32 @@ describe("mintListNumId / parseListNumId", () => { it("mintedListType reads back just the type without the rest", () => { expect(mintedListType("md1:ordered@7+task")).toBe("ordered"); }); + + it("ignores a start value on a bullet mint -- the suffix is ordered-only", () => { + const state = createNumIdMintState(); + expect( + mintListNumId(state, { + type: "bullet", + start: 5, + task: false, + loose: false, + }), + ).toBe("md1:bullet"); + }); + + it("omits the start suffix on an ordered mint with no start at all", () => { + const state = createNumIdMintState(); + expect( + mintListNumId(state, { type: "ordered", task: false, loose: false }), + ).toBe("md1:ordered"); + }); + + it("ignores a numeric @N suffix on a bullet numId when parsing -- start is ordered-only", () => { + expect(parseListNumId("md1:bullet@3")).toEqual({ + type: "bullet", + start: undefined, + task: false, + loose: false, + }); + }); }); diff --git a/packages/markdown-codec/src/shared/list-id.ts b/packages/markdown-codec/src/shared/list-id.ts index 0dab7b45e..8faedeaca 100644 --- a/packages/markdown-codec/src/shared/list-id.ts +++ b/packages/markdown-codec/src/shared/list-id.ts @@ -63,7 +63,8 @@ export function parseListNumId(numId: string): ListNumIdInfo | undefined { return undefined; } const type = match[2]; - if (type === undefined || (type !== "bullet" && type !== "ordered")) { + // NUMID_PATTERN's own second capturing group is a mandatory (bullet|ordered) alternation with no `?` -- a successful overall match always populates it, so this comparison also catches the `undefined` case a plain regex capture-group index type otherwise admits (noUncheckedIndexedAccess), with no second, separately-testable branch for a case the pattern already rules out. + if (type !== "bullet" && type !== "ordered") { return undefined; } const startText = match[3]; diff --git a/packages/markdown-codec/src/shared/style-constants.test.ts b/packages/markdown-codec/src/shared/style-constants.test.ts new file mode 100644 index 000000000..f2c5e7647 --- /dev/null +++ b/packages/markdown-codec/src/shared/style-constants.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { headingStyleId, parseHeadingStyleId } from "./style-constants"; + +describe("headingStyleId / parseHeadingStyleId", () => { + it("mints and parses a heading styleId for an ordinary level", () => { + expect(headingStyleId(3)).toBe("Heading3"); + expect(parseHeadingStyleId("Heading3")).toBe(3); + }); + + it("parses a level past the markdown-reachable 1-6 ceiling, since ContentDocument is a shared cross-format pivot", () => { + expect(parseHeadingStyleId("Heading7")).toBe(7); + }); + + it("rejects a shape this exact pattern does not match", () => { + expect(parseHeadingStyleId("Heading")).toBeUndefined(); + expect(parseHeadingStyleId("heading1")).toBeUndefined(); + expect(parseHeadingStyleId("Quote")).toBeUndefined(); + }); + + it("rejects level 0 -- a heading style level is always a positive integer", () => { + expect(parseHeadingStyleId("Heading0")).toBeUndefined(); + }); + + it("rejects a digit run so long it parses to a non-integer (Infinity), rather than reporting a bogus level", () => { + expect(parseHeadingStyleId(`Heading${"9".repeat(400)}`)).toBeUndefined(); + }); +}); diff --git a/packages/markdown-codec/src/test-support/spec-corpus.test.ts b/packages/markdown-codec/src/test-support/spec-corpus.test.ts new file mode 100644 index 000000000..9cdd27023 --- /dev/null +++ b/packages/markdown-codec/src/test-support/spec-corpus.test.ts @@ -0,0 +1,103 @@ +// Direct tests for the corpus loader's own type guards, which a well-formed vendored spec.json never exercises the failure side of -- loadSpecExamples' own "not an array of {markdown, html, example, section} examples" throw only fires against malformed input, so pinning it means testing the guard functions themselves rather than the loader end to end. + +import type * as NodeFs from "node:fs"; +import { describe, expect, it, vi } from "vitest"; +import { + isSpecExample, + isSpecExampleArray, + loadSpecExamples, +} from "./spec-corpus"; + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readFileSync: vi.fn() }; +}); + +const VALID_EXAMPLE = { + markdown: "# hi\n", + html: "

      hi

      \n", + example: 1, + section: "Headings", +}; + +describe("isSpecExample", () => { + it("accepts a well-formed example", () => { + expect(isSpecExample(VALID_EXAMPLE)).toBe(true); + }); + + it("rejects a non-object", () => { + expect(isSpecExample("not an object")).toBe(false); + expect(isSpecExample(null)).toBe(false); + expect(isSpecExample(42)).toBe(false); + }); + + it('rejects a function even when it carries all four fields with the right types -- typeof a function is "function", never "object"', () => { + // Cast is unavoidable: TypeScript has no narrower type for "a function with these extra own properties attached" than a manual intersection, and Object.assign would build it unsoundly (banned by exadev/no-object-assign). + const fn = (() => {}) as (() => void) & typeof VALID_EXAMPLE; + fn.markdown = VALID_EXAMPLE.markdown; + fn.html = VALID_EXAMPLE.html; + fn.example = VALID_EXAMPLE.example; + fn.section = VALID_EXAMPLE.section; + expect(isSpecExample(fn)).toBe(false); + }); + + it("rejects an object missing any one of the four required fields", () => { + expect(isSpecExample({ html: "h", example: 1, section: "s" })).toBe(false); + expect(isSpecExample({ markdown: "m", example: 1, section: "s" })).toBe( + false, + ); + expect(isSpecExample({ markdown: "m", html: "h", section: "s" })).toBe( + false, + ); + expect(isSpecExample({ markdown: "m", html: "h", example: 1 })).toBe(false); + }); + + it("rejects an object whose fields are present but wrongly typed", () => { + expect(isSpecExample({ ...VALID_EXAMPLE, markdown: 1 })).toBe(false); + expect(isSpecExample({ ...VALID_EXAMPLE, html: 1 })).toBe(false); + expect(isSpecExample({ ...VALID_EXAMPLE, example: "1" })).toBe(false); + expect(isSpecExample({ ...VALID_EXAMPLE, section: 1 })).toBe(false); + }); +}); + +describe("isSpecExampleArray", () => { + it("accepts an array of well-formed examples, including the empty array", () => { + expect(isSpecExampleArray([VALID_EXAMPLE, VALID_EXAMPLE])).toBe(true); + expect(isSpecExampleArray([])).toBe(true); + }); + + it("rejects a non-array", () => { + expect(isSpecExampleArray(VALID_EXAMPLE)).toBe(false); + }); + + it("rejects an array containing even one malformed entry", () => { + expect(isSpecExampleArray([VALID_EXAMPLE, { not: "an example" }])).toBe( + false, + ); + }); +}); + +describe("loadSpecExamples", () => { + it("reads assets/commonmark/spec.json as utf8 and returns a well-formed corpus unchanged", async () => { + const { readFileSync } = await import("node:fs"); + vi.mocked(readFileSync).mockReturnValue(JSON.stringify([VALID_EXAMPLE])); + + expect(loadSpecExamples()).toEqual([VALID_EXAMPLE]); + + const call = vi.mocked(readFileSync).mock.calls[0]; + expect(call).toBeDefined(); + const [urlArgument, encodingArgument] = call!; + expect(String(urlArgument)).toContain("/assets/commonmark/spec.json"); + expect(encodingArgument).toBe("utf8"); + }); + + it("throws a specific message when the vendored corpus is not an array of well-formed examples", async () => { + const { readFileSync } = await import("node:fs"); + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify([VALID_EXAMPLE, { not: "an example" }]), + ); + expect(() => loadSpecExamples()).toThrow( + "assets/commonmark/spec.json is not an array of {markdown, html, example, section} examples", + ); + }); +}); diff --git a/packages/markdown-codec/src/test-support/spec-corpus.ts b/packages/markdown-codec/src/test-support/spec-corpus.ts index 2c61c7611..dc64b290e 100644 --- a/packages/markdown-codec/src/test-support/spec-corpus.ts +++ b/packages/markdown-codec/src/test-support/spec-corpus.ts @@ -13,16 +13,13 @@ export interface SpecExample { readonly section: string; } -function isSpecExample(value: unknown): value is SpecExample { - if (typeof value !== "object" || value === null) { - return false; - } - if ( - !("markdown" in value) || - !("html" in value) || - !("example" in value) || - !("section" in value) - ) { +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +// No separate "does every key exist" guard ahead of the type checks below: a genuinely absent key reads as undefined, whose typeof is never "string" or "number", so the four checks already reject a missing field exactly as they reject a present-but-wrongly-typed one. +export function isSpecExample(value: unknown): value is SpecExample { + if (!isRecord(value)) { return false; } return ( @@ -33,7 +30,7 @@ function isSpecExample(value: unknown): value is SpecExample { ); } -function isSpecExampleArray(value: unknown): value is SpecExample[] { +export function isSpecExampleArray(value: unknown): value is SpecExample[] { return Array.isArray(value) && value.every(isSpecExample); } @@ -72,10 +69,12 @@ export function loadGfmExtensionExamples(extension: string): SpecExample[] { let exampleNumber = 0; while (index < lines.length) { - const line = lines[index] ?? ""; + // index < lines.length just above already guarantees this index is in range. + const line = lines[index]!; const heading = GFM_SECTION_PATTERN.exec(line); if (heading !== null) { - section = heading[1] ?? ""; + // GFM_SECTION_PATTERN's own capturing group is not inside an alternation, so a successful match always populates it -- only TypeScript's own RegExpExecArray typing needs told. + section = heading[1]!; index += 1; continue; } @@ -89,16 +88,18 @@ export function loadGfmExtensionExamples(extension: string): SpecExample[] { index += 1; const markdown: string[] = []; while (index < lines.length && lines[index] !== ".") { - markdown.push(lines[index] ?? ""); + // index < lines.length in the while condition just above already guarantees this index is in range. + markdown.push(lines[index]!); index += 1; } index += 1; const html: string[] = []; + // Both reads below are guarded by the identical index < lines.length check, evaluated first in the while condition's own left-to-right && chain -- in range whenever reached. while ( index < lines.length && - !GFM_EXAMPLE_END_PATTERN.test(lines[index] ?? "") + !GFM_EXAMPLE_END_PATTERN.test(lines[index]!) ) { - html.push(lines[index] ?? ""); + html.push(lines[index]!); index += 1; } index += 1;