Skip to content

Commit 539e1ab

Browse files
committed
docs(generator): name the load-bearing newline rule and report an unscannable source
- Record why '\\n' is in REGEX_ALLOWED_AFTER: formatters emit a binary '/' at end-of-line, so every line-leading '/' in blocks/*.ts is a real regex, including the ones in table.ts and table_v2.ts. Removing it silently mis-scans those two. - Split a scanner failure out of the spread-only 'ids: null' case. Both scans come back empty for the same reason when blankStringsAndComments bails, so the mapper's renames were dropped with no warning; it now reports a parseError and warns. The spread case is unchanged, and its TSDoc no longer claims a cause that was false. - Route every catalog sort through an exported compareCatalogNames so the ordering test exercises the generator's comparator instead of re-deriving it, and match localeCompare arguments whole so localeCompare() and a variable locale are caught. - Note that downloadServableFileFromStorage guarantees a non-empty content type, so the Vanta mimeType fallback chain reads as deliberately defensive. Artifacts regenerate byte-identically and the generator warning set is unchanged.
1 parent 0e790bf commit 539e1ab

3 files changed

Lines changed: 104 additions & 15 deletions

File tree

apps/sim/lib/internal/vanta/file-input.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ export async function resolveVantaUploadFile(
7070
signal: context.signal,
7171
})
7272
context.signal?.throwIfAborted()
73+
/**
74+
* Every return path of `downloadServableFileFromStorage` yields a non-empty content
75+
* type, so `resolved.contentType` always wins. The remaining operands are defensive
76+
* fallbacks kept in place in case that guarantee is ever relaxed.
77+
*/
7378
return {
7479
buffer: resolved.buffer,
7580
fileName: input.fileName || userFile.name,

scripts/generate-docs.test.ts

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from 'fs'
22
import path from 'path'
33
import { describe, expect, it } from 'vitest'
44
import {
5+
compareCatalogNames,
56
extractAllBlockConfigs,
67
extractBlockSuppliedParamIds,
78
extractToolInfo,
@@ -698,27 +699,72 @@ describe('mapper param shapes', () => {
698699
})
699700
})
700701

702+
describe('a source the scanner cannot get through is reported, not swallowed', () => {
703+
/**
704+
* When `blankStringsAndComments` bails, both the `subBlocks` scan and the mapper scan come
705+
* back empty for the same reason. Reported as a plain `ids: null` that is indistinguishable
706+
* from a spread-only `subBlocks` array, the block's mapper renames are dropped in silence.
707+
*/
708+
const unterminated = `subBlocks: [{ id: 'a' }],
709+
tools: { config: { params: (p) => ({ renamedByMapper: p.a }) } },
710+
longDescription: 'never closed`
711+
712+
it('sets parseError so the caller warns', () => {
713+
const supplied = extractBlockSuppliedParamIds(unterminated, 'GhostBlock')
714+
715+
expect(supplied.parseError).not.toBeNull()
716+
expect(supplied.parseError).toMatch(/GhostBlock: source ends inside an unterminated/)
717+
expect(supplied.ids).toBeNull()
718+
})
719+
720+
it('still reports null with no parseError for a spread-only subBlocks array', () => {
721+
const supplied = extractBlockSuppliedParamIds(
722+
"subBlocks: [...NotionBlock.subBlocks], tools: { config: { params: (p) => ({ renamedByMapper: p.a }) } },",
723+
'SpreadBlock'
724+
)
725+
726+
expect(supplied.parseError).toBeNull()
727+
expect(supplied.ids).toBeNull()
728+
expect(supplied.mapperIds).toContain('renamedByMapper')
729+
})
730+
})
731+
701732
describe('the generated catalog ordering is locale-independent', () => {
702733
/**
703734
* `localeCompare` with no locale argument uses the runtime default, which varies with `LANG`
704735
* and the ICU build. Against the real catalog names, `tr-TR` (dotted/dotless I), `lt-LT`,
705736
* `cs-CZ` (the `ch` digraph) and `et-EE` each reorder the array, so a contributor on one of
706737
* those locales would regenerate a different `integrations.json` and fail CI with no obvious
707-
* cause. The generator pins `en-US`; this asserts the committed artifact matches that order.
738+
* cause.
708739
*/
709-
it('sorts integrations.json the way an explicit en-US comparator would', () => {
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', () => {
710748
const catalogPath = path.join(__dirname, '../packages/deployment-config/src/integrations.json')
711749
const names = (
712750
JSON.parse(fs.readFileSync(catalogPath, 'utf-8')).integrations as Array<{ name: string }>
713751
).map(({ name }) => name)
714752

715-
expect(names).toEqual([...names].sort((a, b) => a.localeCompare(b, 'en-US')))
753+
expect(names).toEqual([...names].sort(compareCatalogNames))
716754
})
717755

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.
760+
*/
718761
it('leaves no unpinned localeCompare in the generator', () => {
719762
const source = fs.readFileSync(path.join(__dirname, 'generate-docs.ts'), 'utf-8')
720763

721-
expect(source).not.toMatch(/localeCompare\(\s*[A-Za-z_$][\w$.]*\s*\)/)
764+
const calls = [...source.matchAll(/\blocaleCompare\(([^)]*)\)/g)].map(([, args]) => args)
765+
766+
expect(calls.length).toBeGreaterThan(0)
767+
for (const args of calls) expect(args).toMatch(/,\s*'[a-zA-Z-]+'\s*$/)
722768
})
723769
})
724770

scripts/generate-docs.ts

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@ function writeIconMapping(iconMapping: Record<string, IconRef>): void {
652652

653653
// Generate mapping with direct references (no dynamic access for tree shaking)
654654
const mappingEntries = Object.entries(withAliases)
655-
.sort(([a], [b]) => a.localeCompare(b, 'en-US'))
655+
.sort(([a], [b]) => compareCatalogNames(a, b))
656656
.map(([blockType, iconRef]) => ` ${formatIconMapKey(blockType)}: ${iconRef.name},`)
657657
.join('\n')
658658

@@ -1078,6 +1078,21 @@ export function extractBlockSuppliedParamIds(
10781078
blockContent: string,
10791079
blockName = 'block'
10801080
): BlockSuppliedParams {
1081+
/**
1082+
* A source the blanking scanner cannot get through — it ends inside an unterminated string,
1083+
* template literal or comment — makes both scans below report "nothing found" for the same
1084+
* reason. It is caught here so it is reported as a parse failure. Left to the branches below
1085+
* it would be indistinguishable from a spread-only `subBlocks` array whose block has no
1086+
* mapper, and the block's renames would be dropped without a word.
1087+
*/
1088+
if (blankStringsAndComments(blockContent) === null) {
1089+
return {
1090+
ids: null,
1091+
mapperIds: [],
1092+
parseError: `${blockName}: source ends inside an unterminated string, template literal or comment, so neither its subBlocks array nor its params mapper could be read`,
1093+
}
1094+
}
1095+
10811096
const mapperIds = extractMapperWrittenParamIds(blockContent)
10821097

10831098
try {
@@ -1255,7 +1270,29 @@ function extractAuthType(blockContent: string): 'oauth' | 'api-key' | 'none' {
12551270
return 'none'
12561271
}
12571272

1258-
/** Characters after which a `/` begins a regex literal rather than a division. */
1273+
/**
1274+
* The catalog and every generated mapping are sorted with an explicit `en-US` collation.
1275+
* `localeCompare` with no locale uses the runtime default, which varies with `LANG` and the
1276+
* ICU build: `tr-TR`, `lt-LT`, `cs-CZ` and `et-EE` each reorder the real integration names, so
1277+
* a contributor on one of those locales would regenerate a different artifact and fail CI with
1278+
* no obvious cause.
1279+
*/
1280+
export function compareCatalogNames(a: string, b: string): number {
1281+
return a.localeCompare(b, 'en-US')
1282+
}
1283+
1284+
/**
1285+
* Characters after which a `/` begins a regex literal rather than a division.
1286+
*
1287+
* `'\n'` is deliberate and load-bearing: a line-leading `/` is treated as opening a regex.
1288+
* 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.
1295+
*/
12591296
const REGEX_ALLOWED_AFTER = new Set([
12601297
'(',
12611298
',',
@@ -1565,7 +1602,7 @@ function writeIntegrationsIconMapping(iconMapping: Record<string, IconRef>): voi
15651602

15661603
const imports = renderIconImports(Object.values(iconMapping))
15671604
const mappingEntries = Object.entries(iconMapping)
1568-
.sort(([a], [b]) => a.localeCompare(b, 'en-US'))
1605+
.sort(([a], [b]) => compareCatalogNames(a, b))
15691606
.map(([blockType, iconRef]) => ` ${formatIconMapKey(blockType)}: ${iconRef.name},`)
15701607
.join('\n')
15711608

@@ -1740,7 +1777,7 @@ async function writeIntegrationsJson(iconMapping: Record<string, IconRef>): Prom
17401777
}
17411778
}
17421779

1743-
integrations.sort((a, b) => a.name.localeCompare(b.name, 'en-US'))
1780+
integrations.sort((a, b) => compareCatalogNames(a.name, b.name))
17441781

17451782
const jsonPath = path.join(INTEGRATIONS_CATALOG_PATH, 'integrations.json')
17461783
// `JSON.stringify` always expands every array across multiple lines, but Biome's
@@ -1961,12 +1998,13 @@ function extractBlockConfigFromContent(
19611998
userSettableParamIds = null
19621999
} else if (supplied.ids === null) {
19632000
/**
1964-
* The block's `subBlocks` array holds only spreads of fields arrays this scanner cannot
1965-
* follow. A config-level spread base still contributes its readable fields, so the filter
1966-
* stays on against those plus the mapper's renames; with no base there is nothing to
1967-
* filter against and the filter is switched off. No warning: unlike the `parseError`
1968-
* cases the array itself parsed fine, and every field it names is documented through the
1969-
* spread source's own page.
2001+
* With `parseError` null, the only remaining cause is a `subBlocks` array holding just
2002+
* spreads of fields arrays this scanner cannot follow — a source the scanner could not
2003+
* get through at all is reported as a `parseError` by `extractBlockSuppliedParamIds` and
2004+
* handled above. A config-level spread base still contributes its readable fields, so the
2005+
* filter stays on against those plus the mapper's renames; with no base there is nothing
2006+
* to filter against and the filter is switched off. No warning: the array itself parsed
2007+
* fine, and every field it names is documented through the spread source's own page.
19702008
*/
19712009
userSettableParamIds =
19722010
baseSettableParamIds.length > 0
@@ -4454,7 +4492,7 @@ function groupTriggersByProvider(
44544492
}
44554493
groups.set(
44564494
provider,
4457-
[...byName.values()].sort((a, b) => a.name.localeCompare(b.name, 'en-US'))
4495+
[...byName.values()].sort((a, b) => compareCatalogNames(a.name, b.name))
44584496
)
44594497
}
44604498
return groups

0 commit comments

Comments
 (0)