chore(#2880): close ADR-2143 deployment misses — table-regex fingerprint + state-document seam migration (#2889)
* chore(#2880): close ADR-2143 seam misses — widen table-regex fingerprint, migrate state-document onto the seam The no-adhoc-markdown-parsing rule matched only a negated class whose sole member was a pipe ([^|]), so the stricter and more common [^|\n] spelling evaded it entirely -- src/state-document.cts hand-rolled exactly that shape and linted clean. Widen the fingerprint to any negated class excluding a pipe, which is the ADR-2143 section 7 prohibition as written. With the rule fixed, state-document.cts goes red. Replace tableRowPattern with locateFieldRow: a line scan using the markdown-table seam's splitTableRow for cell semantics, returning the value cell's byte range, and splice that range instead of running a whole-document content.replace. An edit now physically cannot cross a row boundary (section 4). Behavior is frozen -- stateReplaceField has 79 dependents across 5 command processes. Characterization tests lock all 14 table-branch rows plus CRLF, extract round-trip and the withFallback caller shape; a fast-check property asserts every non-target line stays byte-identical. Refs #2880, epic #2143 * fix(#2880): address adversarial review — lone-CR rows, field-name padding, quadratic scan, over-broad fingerprint Isolated adversarial review found four defects in the first commit. 1. locateFieldRow split lines on \n only. JS treats a lone \r as a line terminator, so the regex it replaced matched rows separated by bare CR. "| Phase | 3 |\r| Other | 9 |" returned 3 before and null after. Now CR, LF and CRLF are all terminators, byte offsets unchanged. 2. The field name was normalised with trim().toLowerCase(). The old regex embedded it verbatim, so its whitespace had to be absorbed by the row's own padding -- and because the group is a literal-character match rather than a whitespace class, a tab-padded cell does not accept a space-padded name. Replaced with an offset-aligned search reproducing the original backtracking exactly. 3. The widened fingerprint regex had two unbounded [^\]]* around an optional and ran quadratically over every regex source in every linted file: 256000 chars took 23 seconds. Replaced with a single-pass scanner that never rescans; the same input is now ~1ms. 4. The fingerprint also matched non-table idioms such as [^\s|] and [^"|]. Narrowed to a class excluding the pipe plus only \n, \r or \t. Differential fuzz against origin/next: 20000 cases, 0 mismatches. Refs #2880, epic #2143 * test(#2880): drop wall-clock assertion from the ReDoS regression guard local/no-elapsed-assertion flagged the elapsed-time check, and CLAUDE.md bans timing assertions outright as flaky. The 256000-char input stays as the regression guard for the quadratic scan; correctness of the verdict is what is asserted. If the quadratic path returns, the test stops completing and surfaces as a suite timeout rather than a silent pass. Also adds the changeset fragment for #2880. Refs #2880 * fix(#2880): spec-correct case folding, property tests, naming Code-review findings. The field-name comparison used toLowerCase(). The regex it replaced used /i WITHOUT /u, and ECMAScript Canonicalize deliberately does not fold a non-ASCII character onto an ASCII one -- KELVIN SIGN U+212A matched ASCII K where the old code returned null. Replaced with spec-correct Canonicalize, including the multi-character uppercase case (eszett -> SS), which a naive uppercase comparison also gets wrong. Added the fast-check property tests CLAUDE.md requires for parsers: one for the negated-class scanner, one for the field-name fold semantics, each against an independent reference implementation. Both reference impls failed on first run against real bugs, so neither property is vacuous. Renamed p2/p3 to name the exactly-three-pipes invariant, and reduced a duplicated comment to a cross-reference. Differential fuzz vs origin/next: 20000 runs, 0 mismatches, with the harness proven to discriminate the KELVIN case. Refs #2880 * chore(#2880): backfill changeset PR number (#2889) * docs(#2890): correct the local ESLint plugin path in CONTEXT.md CONTEXT.md named the local AST-rule plugin directory as scripts/eslint-rules/, which does not exist. The real location is eslint-rules/ at the repo root -- what eslint.config.mjs actually imports -- and CONTEXT.md's own later entry already says so explicitly, so the file disagreed with itself. Found by a line-by-line audit of all 1036 lines against the live graph; this was the only confirmed inaccuracy. Closes #2890 --------- Co-authored-by: Test <test@example.com>
This commit is contained in:
Tom Boucher
@@ -7,6 +7,8 @@
|
||||
* from the prior hand-written .cjs; only types are added.
|
||||
*/
|
||||
|
||||
import { splitTableRow } from './markdown-table.cjs';
|
||||
|
||||
// Internal helpers
|
||||
function escapeRegex(str: string): string {
|
||||
return str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
@@ -42,25 +44,171 @@ function isTableSeparatorRow(firstCell: string): boolean {
|
||||
return /^[\s\-:]+$/.test(firstCell.trim());
|
||||
}
|
||||
|
||||
function countLeading(str: string): number {
|
||||
const match = /^[ \t]*/.exec(str);
|
||||
return match ? match[0].length : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a regex that matches a pipe-table row `| FieldName | value |` for the
|
||||
* given (already-escaped) field name. The match is case-insensitive and
|
||||
* tolerates variable amounts of whitespace around the cell contents.
|
||||
*
|
||||
* Capture group 1: leading pipe + whitespace before the field cell
|
||||
* Capture group 2: the field name cell text (trimmed)
|
||||
* Capture group 3: whitespace between field cell and separator pipe
|
||||
* Capture group 4: the value cell text (trimmed)
|
||||
* Capture group 5: trailing whitespace + closing pipe(s)
|
||||
*
|
||||
* We use a single-line match (`m` flag so ^ anchors work on each line) to
|
||||
* avoid cross-row replacement.
|
||||
* Canonicalize one UTF-16 code unit per the ECMAScript non-unicode
|
||||
* `Canonicalize` abstract operation, which governs how a case-insensitive
|
||||
* (`/i`, no `u` flag) RegExp compares characters: take `ch.toUpperCase()`.
|
||||
* The uppercasing is REJECTED (the original character is kept as-is) in
|
||||
* either of two cases: (1) `ch.toUpperCase()` does not produce exactly one
|
||||
* character (e.g. "ß" -> "SS" — a multi-character case-fold can never be a
|
||||
* per-character regex match, so Canonicalize leaves it alone), or (2) it
|
||||
* produces exactly one character but the original character's code point is
|
||||
* >= 128 while the uppercased character's code point is < 128 (this is what
|
||||
* stops a non-ASCII character from folding onto an ASCII one under `/i` —
|
||||
* e.g. KELVIN SIGN U+212A uppercases to ASCII "K" (U+004B), so this rule
|
||||
* rejects the fold and keeps U+212A, meaning `/k/i`/`/K/i` do NOT match
|
||||
* U+212A). Otherwise, the uppercased character is used. Plain
|
||||
* `.toLowerCase()`/`.toUpperCase()` folds both of these cases, which is
|
||||
* exactly why they diverge from real regex `/i` semantics.
|
||||
*/
|
||||
function tableRowPattern(escapedFieldName: string): RegExp {
|
||||
return new RegExp(
|
||||
`^(\\|[ \\t]*)(${escapedFieldName})([ \\t]*\\|[ \\t]*)([^|\\n]*?)([ \\t]*\\|[ \\t]*)$`,
|
||||
'im',
|
||||
);
|
||||
function canonicalizeCharForCaselessCompare(ch: string): string {
|
||||
const upper = ch.toUpperCase();
|
||||
if (upper.length !== 1) {
|
||||
return ch;
|
||||
}
|
||||
if (ch.charCodeAt(0) >= 128 && upper.charCodeAt(0) < 128) {
|
||||
return ch;
|
||||
}
|
||||
return upper;
|
||||
}
|
||||
|
||||
/**
|
||||
* Canonicalize a whole string, one UTF-16 code unit at a time, per the
|
||||
* ECMAScript non-unicode `Canonicalize` rule (see
|
||||
* canonicalizeCharForCaselessCompare) so that two strings compare equal
|
||||
* under this function iff a non-`u`-flag `/i` RegExp would treat them as
|
||||
* the same literal text. This is the correct replacement for
|
||||
* `.toLowerCase()` when replicating a non-`u` `/i` regex: `.toLowerCase()`
|
||||
* folds some non-ASCII characters (e.g. KELVIN SIGN U+212A) onto their
|
||||
* ASCII counterparts, which real `/i` regex semantics do not. Iteration is
|
||||
* by UTF-16 code unit (not code point) to match how a non-`u` regex engine
|
||||
* itself operates on surrogate halves individually.
|
||||
*/
|
||||
function canonicalizeForCaselessCompare(str: string): string {
|
||||
let result = '';
|
||||
for (let i = 0; i < str.length; i++) {
|
||||
result += canonicalizeCharForCaselessCompare(str[i]);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true when the caller's raw (untrimmed) `fieldName` may be considered
|
||||
* to match a row's raw (untrimmed) field cell text. Faithfully replicates the
|
||||
* backtracking of the regex this function replaced: `^(\|[ \t]*)(FieldName)
|
||||
* ([ \t]*\|...)`. Group 1 (`\|[ \t]*`, greedy but backtrackable) can hand any
|
||||
* PREFIX of the cell's leading `[ \t]` run over to group 2 (the literal,
|
||||
* case-insensitive `fieldName` text) — so `fieldName` is tried at every offset
|
||||
* `j` from 0 up to the length of that leading run. For a given `j` to be a
|
||||
* genuine match, two things must hold: `rawCell.slice(j, j + fieldName.length)`
|
||||
* must equal `fieldName` case-insensitively (group 2), AND everything left
|
||||
* over after it — `rawCell.slice(j + fieldName.length)` — must be entirely
|
||||
* `[ \t]` characters, because group 3 (`[ \t]*\|`) must consume that leftover
|
||||
* as whitespace before it can reach the delimiter pipe.
|
||||
*
|
||||
* A simple count-of-leading/trailing-whitespace comparison is NOT equivalent:
|
||||
* it ignores that group 2 is a literal-character match, not a whitespace-
|
||||
* class match, so it can produce false positives whenever `fieldName`'s own
|
||||
* padding is a different run of `[ \t]` characters than the cell's (e.g.
|
||||
* `fieldName` padded with spaces against a cell padded with tabs) — caught by
|
||||
* differential fuzzing against the regex this replaces.
|
||||
*
|
||||
* The case-insensitive comparison itself is done via
|
||||
* canonicalizeForCaselessCompare, NOT `.toLowerCase()`: the replaced regex
|
||||
* used `/i` WITHOUT the `u` flag, whose case-folding is the ECMAScript
|
||||
* non-unicode `Canonicalize` operation. `.toLowerCase()` folds some non-ASCII
|
||||
* characters onto ASCII ones (e.g. KELVIN SIGN U+212A -> "k") that `/i`
|
||||
* (no `u`) does NOT fold, so `.toLowerCase()` alone would NOT faithfully
|
||||
* replicate the old regex's semantics; canonicalizeForCaselessCompare does.
|
||||
*/
|
||||
function fieldNameMatchesRawCell(fieldName: string, rawCell: string): boolean {
|
||||
const n = fieldName.length;
|
||||
const cellLength = rawCell.length;
|
||||
if (n > cellLength)
|
||||
return false;
|
||||
const leadingRun = countLeading(rawCell);
|
||||
const maxOffset = Math.min(leadingRun, cellLength - n);
|
||||
const canonicalFieldName = canonicalizeForCaselessCompare(fieldName);
|
||||
for (let j = 0; j <= maxOffset; j++) {
|
||||
if (canonicalizeForCaselessCompare(rawCell.slice(j, j + n)) !== canonicalFieldName)
|
||||
continue;
|
||||
if (/^[ \t]*$/.test(rawCell.slice(j + n)))
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate the value cell of a pipe-table row `| FieldName | value |` for the
|
||||
* given field name, by scanning `content` line by line (no whole-document
|
||||
* regex). Only a strict two-column row (exactly 3 `|` chars, starting the
|
||||
* line, ending the line after trailing space/tab is stripped) is considered;
|
||||
* this is what makes a 3-column row or an unescaped-pipe-bearing value cell
|
||||
* fail to match, mirroring the previous regex's behaviour. Separator rows
|
||||
* (`| --- | --- |`) are skipped, not matched. The match is case-insensitive.
|
||||
* A line terminator is `\r\n`, a lone `\r`, or a lone `\n` — matching the `m`
|
||||
* flag semantics of the regex this function replaced. Returns the byte range
|
||||
* of the value cell (after trimming surrounding space/tab) so the caller can
|
||||
* splice it directly.
|
||||
*/
|
||||
function locateFieldRow(content: string, fieldName: string): { valueStart: number; valueEnd: number; rawValue: string } | null {
|
||||
let lineStart = 0;
|
||||
while (lineStart <= content.length) {
|
||||
// A line terminator is `\r\n`, a lone `\r`, or a lone `\n` (JS treats a
|
||||
// bare `\r` as a line terminator too — the regex this replaced used the
|
||||
// `m` flag, which honors all three). Scan for whichever of `\r`/`\n`
|
||||
// occurs first; if it's `\r` immediately followed by `\n`, the terminator
|
||||
// is 2 chars wide, otherwise 1.
|
||||
let terminatorIndex = -1;
|
||||
let terminatorLength = 0;
|
||||
for (let i = lineStart; i < content.length; i++) {
|
||||
const ch = content[i];
|
||||
if (ch === '\n') {
|
||||
terminatorIndex = i;
|
||||
terminatorLength = 1;
|
||||
break;
|
||||
}
|
||||
if (ch === '\r') {
|
||||
terminatorIndex = i;
|
||||
terminatorLength = content[i + 1] === '\n' ? 2 : 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
const lineEnd = terminatorIndex === -1 ? content.length : terminatorIndex;
|
||||
const line = content.slice(lineStart, lineEnd);
|
||||
if (line.startsWith('|')) {
|
||||
const pipeCount = (line.match(/\|/g) || []).length;
|
||||
const trimmedEnd = line.replace(/[ \t]+$/, '');
|
||||
if (pipeCount === 3 && trimmedEnd.endsWith('|')) {
|
||||
const cells = splitTableRow(line);
|
||||
if (cells.length === 2 && !isTableSeparatorRow(cells[0])) {
|
||||
// Line has exactly 3 pipes (enforced above): opening pipe, the
|
||||
// field/value separator pipe, and the row-closing pipe.
|
||||
const fieldValueSeparatorPipe = line.indexOf('|', line.indexOf('|') + 1);
|
||||
const rawCell = line.slice(1, fieldValueSeparatorPipe);
|
||||
if (fieldNameMatchesRawCell(fieldName, rawCell)) {
|
||||
const rowClosingPipe = line.indexOf('|', fieldValueSeparatorPipe + 1);
|
||||
let valueStart = lineStart + fieldValueSeparatorPipe + 1;
|
||||
while (content[valueStart] === ' ' || content[valueStart] === '\t')
|
||||
valueStart++;
|
||||
let valueEnd = lineStart + rowClosingPipe;
|
||||
while (valueEnd - 1 >= valueStart && (content[valueEnd - 1] === ' ' || content[valueEnd - 1] === '\t'))
|
||||
valueEnd--;
|
||||
return { valueStart, valueEnd, rawValue: content.slice(valueStart, valueEnd) };
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (terminatorIndex === -1)
|
||||
break;
|
||||
lineStart = terminatorIndex + terminatorLength;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
export function stateExtractField(content: string, fieldName: string): string | null {
|
||||
@@ -77,9 +225,9 @@ export function stateExtractField(content: string, fieldName: string): string |
|
||||
return plainMatch[1].trim();
|
||||
// Pipe-table format: | FieldName | value |
|
||||
// (Separator rows such as `| --- | --- |` are excluded.)
|
||||
const tableMatch = content.match(tableRowPattern(escaped));
|
||||
if (tableMatch && !isTableSeparatorRow(tableMatch[2]))
|
||||
return tableMatch[4].trim();
|
||||
const hit = locateFieldRow(content, fieldName);
|
||||
if (hit)
|
||||
return hit.rawValue.trim();
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -97,13 +245,9 @@ export function stateReplaceField(content: string, fieldName: string, newValue:
|
||||
}
|
||||
// Pipe-table format: | FieldName | value |
|
||||
// Preserve the surrounding pipe/whitespace structure; only swap the value cell.
|
||||
const tblPat = tableRowPattern(escaped);
|
||||
const tblMatch = content.match(tblPat);
|
||||
if (tblMatch && !isTableSeparatorRow(tblMatch[2])) {
|
||||
// Reconstruct the row, preserving the original surrounding whitespace/pipes.
|
||||
return content.replace(tblPat, (_m, leadPipe: string, fieldCell: string, midPipe: string, _oldVal: string, trailPipe: string) =>
|
||||
`${leadPipe}${fieldCell}${midPipe}${newValue}${trailPipe}`,
|
||||
);
|
||||
const hit = locateFieldRow(content, fieldName);
|
||||
if (hit) {
|
||||
return content.slice(0, hit.valueStart) + newValue + content.slice(hit.valueEnd);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user