Skip to content

Commit 36d0488

Browse files
committed
fix(docs-gen): close a regex-vs-division gap and make three guards testable
`REGEX_ALLOWED_AFTER` was missing `'/'`, so a regex directly after a division operator lexed as a second division: in `x / y / /[}]/` the character class was left in the structural view and its `}` closed the enclosing object early, truncating the block's subBlock ids with no warning. Add `'/'`, and guard the `'+'`/`'-'` entries with a `++`/`--` lookbehind so a postfix update still reads as a value and `i++ / 2` stays a division rather than a phantom regex that runs to end-of-input and reports the block unreadable. The `.`/`#` property guard and the `'\n'` entry both survived their mutants. `counts.in / 2, m: preturn / 2` is self-cancelling — the mis-lexed regex closes on the second slash and blanks nothing structural — so the fixtures now leave an odd number of slashes on the line. The `'\n'` entry had no coverage at all: its fixture is now the wrapped `.match(` newline `/re/` shape that `blocks/table.ts` and `blocks/table_v2.ts` produce, which is what that entry (not the preceding `(`, which the newline overwrites) actually decides. Its comment claimed a count of line-leading regexes that drifts with the sources; restate it without one. Drop the two locale tests that could not fail. CI runs under an `en-US` default, where an unpinned `localeCompare` returns exactly what the pinned one does, so no behavioural comparison discriminates; and asserting the committed `integrations.json` against the comparator that produced it agrees by construction. The source grep for a literal locale argument is the real guard. Generated output is byte-identical and the extraction differential over `apps/sim/blocks/blocks/` is empty.
1 parent ec43e24 commit 36d0488

2 files changed

Lines changed: 92 additions & 35 deletions

File tree

scripts/generate-docs.test.ts

Lines changed: 65 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import fs from 'fs'
22
import path from 'path'
33
import { describe, expect, it } from 'vitest'
44
import {
5-
compareCatalogNames,
65
extractAllBlockConfigs,
76
extractBlockSuppliedParamIds,
87
extractToolInfo,
@@ -735,28 +734,16 @@ describe('the generated catalog ordering is locale-independent', () => {
735734
* and the ICU build. Against the real catalog names, `tr-TR` (dotted/dotless I), `lt-LT`,
736735
* `cs-CZ` (the `ch` digraph) and `et-EE` each reorder the array, so a contributor on one of
737736
* those locales would regenerate a different `integrations.json` and fail CI with no obvious
738-
* cause.
739-
*/
740-
it('pins the generator comparator to en-US regardless of the runtime default', () => {
741-
/** `I` sorts after `i` in `en-US` but before it in `tr-TR`, which has a dotless `ı`. */
742-
expect(compareCatalogNames('Intercom', 'incident.io')).toBeGreaterThan(0)
743-
/** `ch` is a single letter after `h` in `cs-CZ`; in `en-US` it stays under `c`. */
744-
expect(compareCatalogNames('Chargebee', 'HubSpot')).toBeLessThan(0)
745-
})
746-
747-
it('sorts integrations.json with the generator comparator', () => {
748-
const catalogPath = path.join(__dirname, '../packages/deployment-config/src/integrations.json')
749-
const names = (
750-
JSON.parse(fs.readFileSync(catalogPath, 'utf-8')).integrations as Array<{ name: string }>
751-
).map(({ name }) => name)
752-
753-
expect(names).toEqual([...names].sort(compareCatalogNames))
754-
})
755-
756-
/**
757-
* Every `localeCompare` in the generator must name its locale as a literal. A bare
758-
* `localeCompare()`, `localeCompare(b)` or a locale read from a variable all fall back to
759-
* the runtime default, so the arguments are matched whole rather than pattern-matched.
737+
* cause. Every `localeCompare` in the generator must therefore name its locale as a literal;
738+
* a bare `localeCompare()`, `localeCompare(b)` or a locale read from a variable all fall
739+
* back to the default, so the arguments are matched whole rather than pattern-matched.
740+
*
741+
* A source grep is the only assertion that can catch an unpinned comparator. CI runs under
742+
* an `en-US` default, where an unpinned `localeCompare` returns exactly what the pinned one
743+
* does, so no behavioural comparison against real catalog names discriminates there; and
744+
* comparing the committed `integrations.json` against the comparator that produced it agrees
745+
* by construction whatever the comparator does. Both of those were asserted here and were
746+
* removed for claiming a guarantee they did not hold.
760747
*/
761748
it('leaves no unpinned localeCompare in the generator', () => {
762749
const source = fs.readFileSync(path.join(__dirname, 'generate-docs.ts'), 'utf-8')
@@ -806,6 +793,49 @@ describe('the scanner survives regex literals in a block config', () => {
806793
it('reports UNKNOWN rather than guessing when a literal never terminates', () => {
807794
expect(extractUserSettableParamIds("subBlocks: [{ id: 'a }],")).toBeNull()
808795
})
796+
797+
/**
798+
* A `/` directly after a division operator is an operand position, so it opens a regex.
799+
* Without `'/'` in `REGEX_ALLOWED_AFTER` the third slash of `x / y / /re/` lexes as a
800+
* second division, the character class is left in the structural view and its `}` closes
801+
* the object early — a short list with no warning.
802+
*/
803+
it('reads a regex that follows a division operator', () => {
804+
const ids = extractUserSettableParamIds(
805+
"subBlocks: [{ id: 'a', v: x / y / /[}]/.source }, { id: 'b' }],"
806+
)
807+
808+
expect(ids).toEqual(['a', 'b'])
809+
})
810+
811+
/**
812+
* `'+'` and `'-'` are in `REGEX_ALLOWED_AFTER` for the binary operators, so the previous
813+
* significant character alone reads the `/` after a postfix `i++` as opening a regex. The
814+
* phantom regex then runs to the end of the input and the scan reports the block unreadable.
815+
*/
816+
it('still reads a division after a postfix increment or decrement', () => {
817+
for (const op of ['++', '--']) {
818+
const ids = extractUserSettableParamIds(
819+
`subBlocks: [{ id: 'a', n: (i) => i${op} / 2 }, { id: 'b' }],`
820+
)
821+
822+
expect(ids, op).toEqual(['a', 'b'])
823+
}
824+
})
825+
826+
/**
827+
* The shape Prettier produces when a `.match()` argument does not fit on one line, as in
828+
* `blocks/table.ts` and `blocks/table_v2.ts`. A newline is recorded as the previous
829+
* significant character rather than skipped, so the `(` does not carry the decision — only
830+
* the `'\n'` entry in `REGEX_ALLOWED_AFTER` keeps this lexing as a regex.
831+
*/
832+
it('reads a regex that a formatter has wrapped onto its own line', () => {
833+
const ids = extractUserSettableParamIds(
834+
["subBlocks: [{ id: 'a', v: (s) => s.match(", ' /[}]/', ") }, { id: 'b' }],"].join('\n')
835+
)
836+
837+
expect(ids).toEqual(['a', 'b'])
838+
})
809839
})
810840

811841
describe('the scanner reads a regex that opens in keyword position', () => {
@@ -848,12 +878,19 @@ describe('the scanner reads a regex that opens in keyword position', () => {
848878
}
849879
})
850880

881+
/**
882+
* The fixture leaves an odd number of `/` on the line, so a mis-lexed regex runs on to the
883+
* end of the input rather than closing on a second slash. A self-cancelling pair like
884+
* `counts.in / 2, m: preturn / 2` passes with the guard removed, because the phantom regex
885+
* spans only `2, m: preturn ` and blanks nothing structural.
886+
*/
851887
it('still reads a division after a property or an identifier that merely ends in a keyword', () => {
852-
const ids = extractUserSettableParamIds(
853-
"subBlocks: [{ id: 'a', n: counts.in / 2, m: preturn / 2 }, { id: 'b' }],"
854-
)
855-
856-
expect(ids).toEqual(['a', 'b'])
888+
expect(
889+
extractUserSettableParamIds("subBlocks: [{ id: 'a', n: counts.in / 2 }, { id: 'b' }],")
890+
).toEqual(['a', 'b'])
891+
expect(
892+
extractUserSettableParamIds("subBlocks: [{ id: 'a', n: preturn / 2 }, { id: 'b' }],")
893+
).toEqual(['a', 'b'])
857894
})
858895
})
859896

scripts/generate-docs.ts

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,21 +1277,22 @@ function extractAuthType(blockContent: string): 'oauth' | 'api-key' | 'none' {
12771277
* a contributor on one of those locales would regenerate a different artifact and fail CI with
12781278
* no obvious cause.
12791279
*/
1280-
export function compareCatalogNames(a: string, b: string): number {
1280+
function compareCatalogNames(a: string, b: string): number {
12811281
return a.localeCompare(b, 'en-US')
12821282
}
12831283

12841284
/**
12851285
* Characters after which a `/` begins a regex literal rather than a division.
12861286
*
12871287
* `'\n'` is deliberate and load-bearing: a line-leading `/` is treated as opening a regex.
1288+
* A newline is recorded as the previous significant character rather than skipped, so this
1289+
* entry — not the `(` before it — is what decides a wrapped `value.match(` newline `/re/`.
12881290
* Prettier and Biome both emit a binary `/` at end-of-line, never at the start of the next
1289-
* one, so in this repo's formatted sources every line-leading `/` really is a regex — all 17
1290-
* occurrences across `apps/sim/blocks/blocks/*.ts` are, including the ones at
1291-
* `blocks/table.ts:31` and `blocks/table_v2.ts:37` that this set exists to get right. Removing
1292-
* `'\n'` makes those two blocks lex as division and silently mis-scan. It is a deliberate
1293-
* trade: a hand-wrapped `b` newline `/ c / d` would be blanked as a regex body, which no
1294-
* formatted file in this repo produces.
1291+
* one, so in this repo's formatted sources every line-leading `/` really is a regex, without
1292+
* exception — `blocks/table.ts` and `blocks/table_v2.ts` both wrap a `.match(` argument this
1293+
* way, and removing `'\n'` makes them lex as division and silently mis-scan. It is a
1294+
* deliberate trade: a hand-wrapped `b` newline `/ c / d` would be blanked as a regex body,
1295+
* which no formatted file in this repo produces.
12951296
*/
12961297
const REGEX_ALLOWED_AFTER = new Set([
12971298
'(',
@@ -1309,6 +1310,7 @@ const REGEX_ALLOWED_AFTER = new Set([
13091310
'+',
13101311
'-',
13111312
'*',
1313+
'/',
13121314
'%',
13131315
'~',
13141316
'^',
@@ -1486,8 +1488,26 @@ function scanTemplateExpression(content: string, start: number): number | null {
14861488
return null
14871489
}
14881490

1491+
/**
1492+
* Whether the word immediately before `index` ends in a postfix `++` or `--`.
1493+
* {@link REGEX_ALLOWED_AFTER} holds `'+'` and `'-'` for the binary operators, but a postfix
1494+
* increment produces a value, so the `/` in `i++ / a` is a division. Only the two-character
1495+
* form is matched — a single `+`/`-` stays an operator position.
1496+
*/
1497+
function precededByPostfixUpdate(content: string, index: number): boolean {
1498+
let j = index - 1
1499+
while (j >= 0 && /\s/.test(content[j])) j--
1500+
const c = content[j]
1501+
return (c === '+' || c === '-') && content[j - 1] === c
1502+
}
1503+
14891504
/** Whether the `/` at `index` opens a regex literal rather than a division. */
14901505
function startsRegexLiteral(content: string, index: number, prevSignificant: string): boolean {
1506+
if (
1507+
(prevSignificant === '+' || prevSignificant === '-') &&
1508+
precededByPostfixUpdate(content, index)
1509+
)
1510+
return false
14911511
return (
14921512
prevSignificant === '' ||
14931513
REGEX_ALLOWED_AFTER.has(prevSignificant) ||

0 commit comments

Comments
 (0)