Skip to content

Commit 4d9cfbd

Browse files
committed
build(scripts): guard the generated markdown tables with a round trip instead of a regex
The sections-table and policy-defaults-table regions admitted authored text their renderers never write, so a hand edit inside the markers was erased on the next regeneration instead of refused. The sections-table shape used `[^\n]*` cells and admitted a pipe, a carriage return, a sixth cell, a blank cell, and an Undeclared default outside the three displays. The policy-defaults-table shape admitted an override naming the same policy as the default and a keep row above a delete row. Both shapes are now one shared round trip, `tableRoundTrip` in lib/markdown-table.ts: the body splits into rows and cells by the inverse of `tableRow()`, every cell passes the rule `tableCell()` writes by, the column count is the header's, every row opens with a key span no earlier row repeats, a per-table parser turns the cells into the record the table renders from, and the records re-rendered must reproduce the body byte for byte. The refusal names the first differing line through `renderedMismatch`, now shared with the action.yml round trip. The Defaults table re-renders through its real renderer, so the opposite-policy binding and the delete-first order come from the renderer rather than a grammar. The Sections table re-renders through the table helper with the prose cells opaque, since its PAT cell paraphrases prose no parser inverts. `POLICY_ROW_PROSE` is minted through `policyRowProse()`, whose `const T & NoBlankProse<T>` parameter makes an empty caveat or override a compile error. The old regex refused `delete ()` and a bare policy span while the old type let the renderer write them. A rule test in each registry's suite refuses a RegExp body on any region whose rendering is a table, so a third table cannot register a hand-written grammar. Refusal tables for both regions pin each input above: seven were admitted by the regexes on main, the rest were refused with the generic message and now name the cell or line.
1 parent 2b5d2ff commit 4d9cfbd

6 files changed

Lines changed: 351 additions & 55 deletions

File tree

‎.github/scripts/gen-action-docs.ts‎

Lines changed: 64 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,14 @@ import {
3030
} from "../../src/sections/contract/permissions.js";
3131
import { SECTIONS } from "../../src/sections/registry.js";
3232
import { agree } from "../../src/text.js";
33+
import type { UndeclaredPolicy } from "../../src/types.js";
3334
import { countWord } from "./lib/count-word.js";
34-
import { type GeneratedRegion, regenerateRegions } from "./lib/generated-regions.js";
35-
import { tableCell } from "./lib/markdown-table.js";
35+
import {
36+
type GeneratedRegion,
37+
regenerateRegions,
38+
renderedMismatch,
39+
} from "./lib/generated-regions.js";
40+
import { renderTable, tableCell, tableRoundTrip } from "./lib/markdown-table.js";
3641

3742
const ROOT = join(import.meta.dir, "..", "..");
3843

@@ -174,6 +179,28 @@ export interface PolicyRowProse {
174179
readonly override: string;
175180
}
176181

182+
type Prose<S> = S extends "" ? never : S;
183+
184+
// A blank caveat or override would render `delete ()` or a bare policy span, both cells tableCell() admits, so
185+
// the table's literals prove themselves non-empty at the type instead.
186+
type NoBlankProse<T extends Record<string, PolicyRowProse>> = {
187+
readonly [K in keyof T]: {
188+
readonly caveat?: Prose<T[K]["caveat"]>;
189+
readonly override: Prose<T[K]["override"]>;
190+
};
191+
};
192+
193+
function policyRowProse<const T extends Record<UndeclaredPolicySection, PolicyRowProse>>(
194+
table: T & NoBlankProse<T>,
195+
): T {
196+
return table;
197+
}
198+
199+
const OPPOSITE: Readonly<Record<UndeclaredPolicy, UndeclaredPolicy>> = {
200+
delete: "keep",
201+
keep: "delete",
202+
};
203+
177204
const DEFAULTS_TABLE_HEADER = "| Section | Default | The override buys you |\n|---|---|---|";
178205

179206
export function renderPolicyDefaultsTable(
@@ -186,26 +213,46 @@ export function renderPolicyDefaultsTable(
186213
throw new Error(`no Defaults-per-section prose for the "${section.key}" section`);
187214
}
188215
const caveat = text.caveat === undefined ? "" : ` (${text.caveat})`;
189-
const opposite = section.undeclaredDefault === "delete" ? "keep" : "delete";
190-
const cells = [
216+
return [
191217
`\`${section.key}\``,
192218
tableCell(`${section.undeclaredDefault}${caveat}`, `the ${section.key} Default cell`),
193219
tableCell(
194-
`\`${opposite}\`: ${text.override}`,
220+
`\`${OPPOSITE[section.undeclaredDefault]}\`: ${text.override}`,
195221
`the ${section.key} "The override buys you" cell`,
196222
),
197223
];
198-
return `| ${cells.join(" | ")} |`;
199224
});
200-
return [DEFAULTS_TABLE_HEADER, ...rows].join("\n");
225+
return renderTable(DEFAULTS_TABLE_HEADER, rows);
226+
}
227+
228+
/** A Defaults row read back to the section and prose it renders from; the opposite-policy binding and the
229+
* delete-first order are then the renderer's to check, through the byte compare. The dotAll flag keeps the
230+
* two Unicode line separators tableCell() admits inside the prose. */
231+
function policyRow(
232+
cells: readonly string[],
233+
key: string,
234+
): { readonly section: KnobbedSection; readonly prose: PolicyRowProse } | string {
235+
const policy = /^(delete|keep)(?: \((.+)\))?$/s.exec(cells[1] ?? "");
236+
const override = /^`(?:delete|keep)`: (.+)$/s.exec(cells[2] ?? "")?.[1];
237+
if (policy === null) {
238+
return "states no delete or keep default";
239+
}
240+
if (override === undefined) {
241+
return "names no policy span before its override prose";
242+
}
243+
const caveat = policy[2];
244+
return {
245+
section: { key, undeclaredDefault: policy[1] === "delete" ? "delete" : "keep" },
246+
prose: caveat === undefined ? { override } : { caveat, override },
247+
};
201248
}
202249

203250
/** The row every secret family shares: the value is write-only, so a wrong delete is a loss, not a drift. */
204-
const SECRET_ROW_PROSE: PolicyRowProse = {
251+
const SECRET_ROW_PROSE = {
205252
override: "prune stale secrets - a deleted secret's value is unrecoverable",
206-
};
253+
} as const;
207254

208-
export const POLICY_ROW_PROSE: Record<UndeclaredPolicySection, PolicyRowProse> = {
255+
export const POLICY_ROW_PROSE = policyRowProse({
209256
labels: {
210257
caveat: "Probot parity",
211258
override: "manage a core set without deleting ad-hoc labels",
@@ -244,7 +291,7 @@ export const POLICY_ROW_PROSE: Record<UndeclaredPolicySection, PolicyRowProse> =
244291
override:
245292
"prune stale patterns - the pattern's alerts are resolved (never deleted), keeping the audit trail",
246293
},
247-
};
294+
});
248295

249296
/** The token-UI label of a permission's primary resource (ANY one grants access; the first is the one to ask for). */
250297
function primaryLabel(permission: SectionPermission): string {
@@ -347,10 +394,6 @@ function blockShape(lines: string): RegExp {
347394
return new RegExp(String.raw`^\n(?:${lines}|\n)?$`);
348395
}
349396

350-
function tableShape(header: string, cells: string): RegExp {
351-
return blockShape(String.raw`${RegExp.escape(header)}\n(?:\| ${cells} \|\n)*`);
352-
}
353-
354397
/** The emitter is its own grammar: a body is admitted when re-rendering the declarations it parses to reproduces it
355398
* byte for byte, so no spelling the emitter does not write gets through. A freshly placed region holds "\n" and
356399
* renders next. */
@@ -386,15 +429,7 @@ function roundTrip<T extends { readonly description: string }>(
386429
if (typeof declared === "string") {
387430
return `it is not a set of ${key} declarations (${declared})`;
388431
}
389-
const rendered = `\n${render(declared)}\n`;
390-
if (rendered === body) {
391-
return undefined;
392-
}
393-
const authored = body.split("\n");
394-
const expected = rendered.split("\n");
395-
const differing = authored.findIndex((line, i) => line !== expected[i]);
396-
const at = differing === -1 ? authored.length : differing;
397-
return `line ${at} reads ${JSON.stringify(authored[at] ?? "")} where the generator writes ${JSON.stringify(expected[at] ?? "")}`;
432+
return renderedMismatch(body, `\n${render(declared)}\n`);
398433
};
399434
}
400435

@@ -434,10 +469,11 @@ export const GENERATED_REGIONS: Readonly<Record<string, readonly GeneratedRegion
434469
{
435470
name: "policy-defaults-table",
436471
placement: { kind: "under-heading", heading: "## Defaults per section" },
437-
// A cell holds no pipe and no line break: tableCell() refuses both, so a row carrying one is authored, not stale output.
438-
body: tableShape(
439-
DEFAULTS_TABLE_HEADER,
440-
String.raw`\x60[a-z_]+\x60 \| (?:delete|keep)(?: \([^\r\n|]+\))? \| \x60(?:delete|keep)\x60: [^\r\n|]+`,
472+
body: tableRoundTrip(DEFAULTS_TABLE_HEADER, policyRow, (rows) =>
473+
renderPolicyDefaultsTable(
474+
rows.map((row) => row.section),
475+
Object.fromEntries(rows.map((row) => [row.section.key, row.prose])),
476+
),
441477
),
442478
render: block(() => renderPolicyDefaultsTable(knobbedSections(), POLICY_ROW_PROSE)),
443479
},

‎.github/scripts/gen-docs.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { readArchitecture, renderArchitectureMermaid } from "./arch-lint.js";
1616
import { COVERAGE_DATA, type CoverageData } from "./coverage-data.js";
1717
import { ENDPOINT_ANCHORS, type EndpointAnchors } from "./endpoint-docs.js";
1818
import { type GeneratedRegion, regenerateRegions, regionBounds } from "./lib/generated-regions.js";
19-
import { tableCell } from "./lib/markdown-table.js";
19+
import { renderTable, tableCell, tableRoundTrip, tableRow } from "./lib/markdown-table.js";
2020

2121
const ROOT = join(import.meta.dir, "..", "..");
2222
export const COVERAGE_PATH = "docs/reference/coverage.md";
@@ -106,7 +106,7 @@ export function renderSectionsTable(
106106
tableCell(doc.sections_table.notes, `the ${section.key} Notes cell`),
107107
];
108108
});
109-
return [TABLE_HEADER, ...rows.map((cells) => `| ${cells.join(" | ")} |`)].join("\n");
109+
return renderTable(TABLE_HEADER, rows);
110110
}
111111

112112
/** Text for inside a markdown code span: a cell, and a backtick would close the span early. */
@@ -244,7 +244,7 @@ function renderSection(
244244
`[\`${section.key}\`](${SECTIONS_PAGE})${keys}`,
245245
tableCell(renderCalls(section, row, claimed, carrier, anchors), `${where}'s Endpoints cell`),
246246
];
247-
return `| ${cells.join(" | ")} |`;
247+
return tableRow(cells);
248248
});
249249
const declared = [...Object.keys(section.endpoints), ...Object.keys(section.graphql ?? {})];
250250
const unclaimed = declared.filter((role) => !claimed.has(role));
@@ -310,7 +310,7 @@ export function renderCoverage(
310310
tableCell(row.endpoints.join(", "), `${where}'s Endpoints cell`),
311311
tableCell(row.why, `${where}'s Why cell`),
312312
];
313-
return `| ${cells.join(" | ")} |`;
313+
return tableRow(cells);
314314
}),
315315
];
316316
return [
@@ -407,15 +407,20 @@ function patFormUrl(): string {
407407
);
408408
}
409409

410-
// The body shape is this generator's own output (or an empty body), built from the renderer constants, so a marker
411-
// moved over authored prose or another table fails instead of erasing it.
410+
// The three prose cells are opaque; a row is this table's when its Undeclared default cell is one of the displays,
411+
// the one cell past the key renderSectionsTable() writes without tableCell().
412+
function sectionsRow(cells: readonly string[]): readonly string[] | string {
413+
if (!Object.values(UNDECLARED_DEFAULT_DISPLAY).includes(cells[3] ?? "")) {
414+
return "shows an Undeclared default the renderer has no display for";
415+
}
416+
return cells;
417+
}
418+
412419
function sectionsTableRegion(name: string, heading: string): GeneratedRegion {
413420
return {
414421
name,
415422
placement: { kind: "under-heading", heading },
416-
body: new RegExp(
417-
String.raw`^\n(?:${RegExp.escape(TABLE_HEADER)}\n(?:\| \x60[a-z_]+\x60 \| [^\n]* \|\n)*)?$`,
418-
),
423+
body: tableRoundTrip(TABLE_HEADER, sectionsRow, (rows) => renderTable(TABLE_HEADER, rows)),
419424
render: () => `\n${renderSectionsTable(SECTIONS, DOCS)}\n`,
420425
};
421426
}

‎.github/scripts/lib/generated-regions.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,18 @@ export function bodyRefusal(shape: BodyShape, body: string): string | undefined
159159
return shape(body);
160160
}
161161

162+
/** A round-trip guard's verdict: undefined when `body` is `rendered` byte for byte, else the first differing line. */
163+
export function renderedMismatch(body: string, rendered: string): string | undefined {
164+
if (rendered === body) {
165+
return undefined;
166+
}
167+
const authored = body.split("\n");
168+
const expected = rendered.split("\n");
169+
const differing = authored.findIndex((line, i) => line !== expected[i]);
170+
const at = differing === -1 ? authored.length : differing;
171+
return `line ${at} reads ${JSON.stringify(authored[at] ?? "")} where the generator writes ${JSON.stringify(expected[at] ?? "")}`;
172+
}
173+
162174
interface MarkdownScan {
163175
/** Fenced code blocks, an unclosed one running to the end of the text. */
164176
readonly fenced: readonly MarkerSpan[];
Lines changed: 87 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,93 @@
1+
// The markdown tables the generators write: a header constant over one `| a | b |` row per record. The cell rule
2+
// and the row joiner are the whole grammar, so tableRoundTrip() reads a region back by the same rules and admits
3+
// only what renderTable() reproduces byte for byte.
4+
5+
import { renderedMismatch } from "./generated-regions.js";
6+
7+
/** Why `text` cannot be a cell: a pipe or a line break would split its row, a blank would leave its column empty. */
8+
function cellFault(text: string): string | undefined {
9+
return text.trim() === "" || /[|\r\n]/.test(text)
10+
? 'is blank or contains "|" or a line break'
11+
: undefined;
12+
}
13+
114
/**
2-
* A markdown table cell of authored prose, refused rather than escaped: a pipe or a line break would split its row,
3-
* a blank one would leave its column empty, and an escape would silently change the rendered text. `where` names
4-
* the cell as its author knows it.
15+
* `text` as a cell of authored prose, refused rather than escaped: an escape would silently change the rendered text.
16+
* `where` names the cell as its author knows it.
517
*/
618
export function tableCell(text: string, where: string): string {
7-
if (text.trim() === "" || /[|\r\n]/.test(text)) {
8-
throw new Error(
9-
`${where} is blank or contains "|" or a line break, which would break its table row: "${text}"`,
10-
);
19+
const fault = cellFault(text);
20+
if (fault !== undefined) {
21+
throw new Error(`${where} ${fault}, which would break its table row: "${text}"`);
1122
}
1223
return text;
1324
}
25+
26+
/** A settings key in a code span: the cell every generated table opens its rows with, written without tableCell(). */
27+
const KEY_SPAN = /^`([a-z_]+)`$/;
28+
29+
export function tableRow(cells: readonly string[]): string {
30+
return `| ${cells.join(" | ")} |`;
31+
}
32+
33+
export function renderTable(header: string, rows: ReadonlyArray<readonly string[]>): string {
34+
return [header, ...rows.map(tableRow)].join("\n");
35+
}
36+
37+
/** The cells of one tableRow() line; a line of another shape yields the wrong count or fails the byte compare. */
38+
function rowCells(line: string): string[] {
39+
return line
40+
.split("|")
41+
.slice(1, -1)
42+
.map((cell) => cell.slice(1, -1));
43+
}
44+
45+
/**
46+
* The body guard of a generated table region. The body splits into rows and cells the way renderTable() joined
47+
* them, every cell passes cellFault(), every row opens with a key span no other row repeats (the renderers walk a
48+
* registry), `parseRow` turns a row's cells into the record it renders from (a string is the refusal), and
49+
* `renderRows` must reproduce the body byte for byte, so an authored spelling the renderer never writes is refused
50+
* rather than erased. A freshly placed region holds "\n" and renders next.
51+
*/
52+
export function tableRoundTrip<Row>(
53+
header: string,
54+
parseRow: (cells: readonly string[], key: string) => Row | string,
55+
renderRows: (rows: readonly Row[]) => string,
56+
): (body: string) => string | undefined {
57+
const headerLines = header.split("\n");
58+
const columns = rowCells(headerLines[0] ?? "").length;
59+
return (body) => {
60+
if (body === "\n") {
61+
return undefined;
62+
}
63+
const rows: Row[] = [];
64+
const keys = new Set<string>();
65+
if (body.startsWith(`\n${header}\n`) && body.endsWith("\n")) {
66+
const lines = body.split("\n");
67+
for (let at = headerLines.length + 1; at < lines.length - 1; at += 1) {
68+
const cells = rowCells(lines[at] ?? "");
69+
if (cells.length !== columns) {
70+
return `line ${at} has ${cells.length} cells where the table has ${columns}`;
71+
}
72+
const faulty = cells.findIndex((cell) => cellFault(cell) !== undefined);
73+
if (faulty !== -1) {
74+
return `cell ${faulty + 1} of line ${at} ${cellFault(cells[faulty] ?? "")}`;
75+
}
76+
const key = KEY_SPAN.exec(cells[0] ?? "")?.[1];
77+
if (key === undefined) {
78+
return `line ${at} does not open with a section key in a code span`;
79+
}
80+
if (keys.has(key)) {
81+
return `line ${at} repeats the \`${key}\` row`;
82+
}
83+
keys.add(key);
84+
const row = parseRow(cells, key);
85+
if (typeof row === "string") {
86+
return `line ${at} ${row}`;
87+
}
88+
rows.push(row);
89+
}
90+
}
91+
return renderedMismatch(body, `\n${renderRows(rows)}\n`);
92+
};
93+
}

0 commit comments

Comments
 (0)