Skip to content

Commit b4645ea

Browse files
committed
fix(audits): detect partialize and .with by AST instead of regex
1 parent b53e83a commit b4645ea

5 files changed

Lines changed: 236 additions & 47 deletions

File tree

‎.claude/rules/sim-react-performance.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ return items.sort(compare)
7777
return [...items].sort(compare)
7878
```
7979

80-
**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and any `.with(index, value)` call whose second argument is not a function literal; OpenTelemetry's `context.with(ctx, () => …)` passes, and one handed its callback as an identifier needs `// utils-lint-allow: <reason>`.
80+
**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and every two-argument `.with(index, value)` call on any receiver except an OpenTelemetry context object (a receiver named like `context`, `otelContext`, `otelContextApi`); a deliberate exception carries `// utils-lint-allow: <reason>`.
8181

8282
## Run independent awaits in parallel
8383

‎.cursor/rules/sim-react-performance.mdc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ return items.sort(compare)
8080
return [...items].sort(compare)
8181
```
8282

83-
**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and any `.with(index, value)` call whose second argument is not a function literal; OpenTelemetry's `context.with(ctx, () => …)` passes, and one handed its callback as an identifier needs `// utils-lint-allow: <reason>`.
83+
**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is used everywhere: whether a module reaches the browser is not visible from its path. `check:utils` flags `toSorted`/`toReversed`/`toSpliced` repo-wide and every two-argument `.with(index, value)` call on any receiver except an OpenTelemetry context object (a receiver named like `context`, `otelContext`, `otelContextApi`); a deliberate exception carries `// utils-lint-allow: <reason>`.
8484

8585
## Run independent awaits in parallel
8686

‎scripts/check-application-graph.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,11 +286,14 @@ export function findViolations({ root, forbidden }: GuardedRoot): GraphViolation
286286
/** A module a runtime import can resolve to: not a test, integration test, or declaration file. */
287287
const RUNTIME_MODULE = /(?<!\.(?:test|spec|integration|d))\.tsx?$/
288288

289+
/** Test-support directories: nothing in them satisfies a runtime import. */
290+
const TEST_SUPPORT_DIRS = new Set(['__fixtures__', '__mocks__', '__tests__', '__integration__'])
291+
289292
/** Whether `dir` holds a runtime module at any depth. */
290293
function containsRuntimeModule(dir: string): boolean {
291294
return readdirSync(dir, { withFileTypes: true }).some((entry) =>
292295
entry.isDirectory()
293-
? containsRuntimeModule(resolve(dir, entry.name))
296+
? !TEST_SUPPORT_DIRS.has(entry.name) && containsRuntimeModule(resolve(dir, entry.name))
294297
: RUNTIME_MODULE.test(entry.name)
295298
)
296299
}

‎scripts/check-utils-enforcement.ts‎

Lines changed: 86 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
*/
2020
import { readdir, readFile } from 'node:fs/promises'
2121
import path from 'node:path'
22+
import { parse } from '@babel/parser'
23+
import { getErrorMessage } from '@sim/utils/errors'
2224

2325
const ROOT = path.resolve(import.meta.dir, '..')
2426

@@ -51,11 +53,18 @@ const TRUNCATE_PREFILTER = /\.(?:slice|substring)\(\s*0\s*,[^)]*\)\s*(?:\}|\+)/
5153
/** Literal gate shared by the `filterUndefined` and `omit` patterns. */
5254
const FROM_ENTRIES = /Object\.fromEntries\(/
5355

56+
/** Shared by the toSorted/toReversed/toSpliced pattern and the AST-based `.with` check. */
57+
const ES2023_ARRAY_METHOD = {
58+
description:
59+
'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)',
60+
suggestion:
61+
'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice(), or [...arr] then next[i] = value',
62+
}
63+
5464
const BANNED_PATTERNS: Array<{
5565
pattern: RegExp
5666
description: string
5767
suggestion: string
58-
/** Restricts the pattern to matching files; unrestricted patterns apply everywhere. */
5968
/** Cheap literal test that skips the pattern on files that cannot match; memoized per file. */
6069
prefilter?: RegExp
6170
}> = [
@@ -146,13 +155,8 @@ const BANNED_PATTERNS: Array<{
146155
},
147156
// Render-path rules (.claude/rules/sim-react-performance.md, sim-styling.md)
148157
{
149-
/** `.with(i, value)`; a function second argument is OpenTelemetry's `context.with(ctx, fn)`. */
150-
pattern:
151-
/\.(?:toSorted|toReversed|toSpliced)\s*\(|\.with\(\s*[^,()]+,(?!\s*(?:async\s*)?(?:\([^)]*\)\s*=>|\w+\s*=>|function\b))/g,
152-
description:
153-
'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)',
154-
suggestion:
155-
'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice(), or [...arr] then next[i] = value',
158+
pattern: /\.(?:toSorted|toReversed|toSpliced)\s*\(/g,
159+
...ES2023_ARRAY_METHOD,
156160
},
157161
{
158162
pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g,
@@ -169,6 +173,75 @@ const BANNED_PATTERNS: Array<{
169173
},
170174
]
171175

176+
/** Cheap gate: only files that contain a `.with(` call are parsed. */
177+
const WITH_CALL = /\.with\s*\(/
178+
179+
/** OpenTelemetry's `context.with(ctx, fn)` shares the shape; its receivers are named for the context API. */
180+
const OTEL_CONTEXT_RECEIVER = /context/i
181+
182+
/** A Babel AST node, read structurally rather than through `@babel/types`. */
183+
interface SyntaxNode extends Record<string, unknown> {
184+
type: string
185+
start: number
186+
}
187+
188+
function isSyntaxNode(value: unknown): value is SyntaxNode {
189+
return (
190+
typeof value === 'object' && value !== null && 'type' in value && typeof value.type === 'string'
191+
)
192+
}
193+
194+
function walkNodes(node: SyntaxNode, visit: (node: SyntaxNode) => void): void {
195+
visit(node)
196+
for (const value of Object.values(node)) {
197+
if (isSyntaxNode(value)) walkNodes(value, visit)
198+
else if (Array.isArray(value))
199+
for (const item of value) if (isSyntaxNode(item)) walkNodes(item, visit)
200+
}
201+
}
202+
203+
/** The name a member call is made on: `arr` in `arr.with(…)`, `items` in `this.items.with(…)`. */
204+
function receiverName(receiver: unknown): string | undefined {
205+
if (!isSyntaxNode(receiver)) return undefined
206+
if (receiver.type === 'Identifier' && typeof receiver.name === 'string') return receiver.name
207+
const isMember =
208+
receiver.type === 'MemberExpression' || receiver.type === 'OptionalMemberExpression'
209+
if (!isMember || receiver.computed === true || !isSyntaxNode(receiver.property)) return undefined
210+
return typeof receiver.property.name === 'string' ? receiver.property.name : undefined
211+
}
212+
213+
/**
214+
* Offsets of every `Array.prototype.with(index, value)` call: a two-argument `.with` on any
215+
* receiver except an OpenTelemetry context object. Drizzle's one-argument `.with(cte)` and
216+
* `index().with({ … })` never match.
217+
*/
218+
function findArrayWithCalls(file: string, content: string): number[] {
219+
let program: unknown
220+
try {
221+
program = parse(content, {
222+
sourceType: 'module',
223+
plugins: ['typescript', ...(/\.[jt]sx$/.test(file) ? (['jsx'] as const) : [])],
224+
errorRecovery: true,
225+
}).program
226+
} catch (error) {
227+
throw new Error(`Cannot parse ${file} to check its .with calls: ${getErrorMessage(error)}`)
228+
}
229+
if (!isSyntaxNode(program)) return []
230+
231+
const offsets: number[] = []
232+
walkNodes(program, (node) => {
233+
if (node.type !== 'CallExpression' && node.type !== 'OptionalCallExpression') return
234+
if (!Array.isArray(node.arguments) || node.arguments.length !== 2) return
235+
const callee = node.callee
236+
if (!isSyntaxNode(callee) || callee.computed === true || !isSyntaxNode(callee.property)) return
237+
if (callee.type !== 'MemberExpression' && callee.type !== 'OptionalMemberExpression') return
238+
if (callee.property.name !== 'with') return
239+
if (OTEL_CONTEXT_RECEIVER.test(receiverName(callee.object) ?? '')) return
240+
offsets.push(callee.property.start)
241+
})
242+
return offsets
243+
}
244+
172245
async function walk(dir: string, results: string[] = []): Promise<string[]> {
173246
let entries
174247
try {
@@ -285,6 +358,11 @@ async function main() {
285358
matches.push({ index: match.index, description, suggestion })
286359
}
287360
}
361+
if (WITH_CALL.test(content)) {
362+
for (const index of findArrayWithCalls(rel, content)) {
363+
matches.push({ index, ...ES2023_ARRAY_METHOD })
364+
}
365+
}
288366
if (matches.length === 0) continue
289367

290368
const lines = content.split('\n')

‎scripts/check-zustand-v5-selectors.ts‎

Lines changed: 144 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#!/usr/bin/env bun
22
import { readdir, readFile } from 'node:fs/promises'
33
import path from 'node:path'
4+
import { parse } from '@babel/parser'
5+
import { getErrorMessage } from '@sim/utils/errors'
46

57
const ROOT = path.resolve(import.meta.dir, '..')
68
const APP_DIR = path.join(ROOT, 'apps/sim')
@@ -298,14 +300,121 @@ function auditFile(file: string, source: string): Violation[] {
298300
const PERSIST_IMPORT =
299301
/import\s*\{[^}]*\bpersist\b(?:\s+as\s+(\w+))?[^}]*\}\s*from\s*'zustand\/middleware'/
300302

301-
/** Removes comments while leaving string literals (which may contain `//`) intact. */
302-
function stripComments(code: string): string {
303-
return code.replace(
304-
/('(?:\\.|[^'\\])*'|"(?:\\.|[^"\\])*"|`(?:\\.|[^`\\])*`)|\/\*[\s\S]*?\*\/|\/\/[^\n]*/g,
305-
(_, literal: string | undefined) => literal ?? ''
303+
/** A Babel AST node, read structurally rather than through `@babel/types`. */
304+
interface SyntaxNode extends Record<string, unknown> {
305+
type: string
306+
start: number
307+
end: number
308+
}
309+
310+
function isSyntaxNode(value: unknown): value is SyntaxNode {
311+
return (
312+
typeof value === 'object' && value !== null && 'type' in value && typeof value.type === 'string'
306313
)
307314
}
308315

316+
/** Visits `node` and every node beneath it, depth first; `visit` returning false skips a subtree. */
317+
function walkNodes(node: SyntaxNode, visit: (node: SyntaxNode) => boolean | undefined): void {
318+
if (visit(node) === false) return
319+
for (const value of Object.values(node)) {
320+
if (isSyntaxNode(value)) walkNodes(value, visit)
321+
else if (Array.isArray(value))
322+
for (const item of value) if (isSyntaxNode(item)) walkNodes(item, visit)
323+
}
324+
}
325+
326+
/** Strips the type-only wrappers (`as`, `satisfies`, `!`) around an expression. */
327+
function unwrapExpression(node: unknown): unknown {
328+
let current = node
329+
while (
330+
isSyntaxNode(current) &&
331+
(current.type === 'TSAsExpression' ||
332+
current.type === 'TSSatisfiesExpression' ||
333+
current.type === 'TSNonNullExpression')
334+
) {
335+
current = current.expression
336+
}
337+
return current
338+
}
339+
340+
function isIdentifierNamed(node: unknown, name: string): boolean {
341+
const unwrapped = unwrapExpression(node)
342+
return isSyntaxNode(unwrapped) && unwrapped.type === 'Identifier' && unwrapped.name === name
343+
}
344+
345+
/** `name` itself, or an object literal that spreads `...name` (extra keys still carry everything). */
346+
function isWholeBinding(node: unknown, name: string): boolean {
347+
if (isIdentifierNamed(node, name)) return true
348+
const unwrapped = unwrapExpression(node)
349+
if (!isSyntaxNode(unwrapped) || unwrapped.type !== 'ObjectExpression') return false
350+
const properties = Array.isArray(unwrapped.properties) ? unwrapped.properties : []
351+
return properties.some(
352+
(property) =>
353+
isSyntaxNode(property) &&
354+
property.type === 'SpreadElement' &&
355+
isIdentifierNamed(property.argument, name)
356+
)
357+
}
358+
359+
/** The expressions a function returns: its expression body, or every `return` in its own block. */
360+
function returnedExpressions(fn: SyntaxNode): unknown[] {
361+
if (!isSyntaxNode(fn.body)) return []
362+
if (fn.body.type !== 'BlockStatement') return [fn.body]
363+
const returned: unknown[] = []
364+
walkNodes(fn.body, (node) => {
365+
if (node.type === 'ReturnStatement') returned.push(node.argument)
366+
return !/Function|ObjectMethod|ClassMethod/.test(node.type)
367+
})
368+
return returned
369+
}
370+
371+
/**
372+
* The parameter binding that holds every state field: `s` in `(s) => …`, or `rest` in
373+
* `({ a, ...rest }) => …`, which holds every field but the ones named (a deny-list).
374+
*/
375+
function wholeStateBinding(param: unknown): { name: string; reason: string } | null {
376+
if (!isSyntaxNode(param)) return null
377+
if (param.type === 'Identifier' && typeof param.name === 'string') {
378+
return { name: param.name, reason: 'persist partialize spreads the whole state' }
379+
}
380+
if (param.type !== 'ObjectPattern' || !Array.isArray(param.properties)) return null
381+
const rest = param.properties.find(
382+
(property): property is SyntaxNode => isSyntaxNode(property) && property.type === 'RestElement'
383+
)
384+
if (!rest || !isSyntaxNode(rest.argument) || typeof rest.argument.name !== 'string') return null
385+
return {
386+
name: rest.argument.name,
387+
reason: 'persist partialize persists everything but the fields it names (a deny-list)',
388+
}
389+
}
390+
391+
/**
392+
* Why an inline `partialize` persists more than a whitelist, or null when it returns one:
393+
* any return of the whole-state binding itself or of `{ ...binding }`.
394+
*/
395+
function partializeLeak(fn: SyntaxNode): string | null {
396+
const binding = wholeStateBinding(Array.isArray(fn.params) ? fn.params[0] : undefined)
397+
if (!binding) return null
398+
return returnedExpressions(fn).some((expression) => isWholeBinding(expression, binding.name))
399+
? `${binding.reason}; return an explicit whitelist of durable fields`
400+
: null
401+
}
402+
403+
/** The top-level `partialize` of a persist options object literal, or null when it has none. */
404+
function findPartialize(options: SyntaxNode): SyntaxNode | null {
405+
const properties = Array.isArray(options.properties) ? options.properties : []
406+
for (const property of properties) {
407+
if (!isSyntaxNode(property) || property.computed === true) continue
408+
if (property.type !== 'ObjectProperty' && property.type !== 'ObjectMethod') continue
409+
const key = property.key
410+
const keyName = isSyntaxNode(key) ? (key.type === 'Identifier' ? key.name : key.value) : null
411+
if (keyName !== 'partialize') continue
412+
if (property.type === 'ObjectMethod') return property
413+
return isSyntaxNode(property.value) ? property.value : null
414+
}
415+
return null
416+
}
417+
309418
/**
310419
* `.claude/rules/sim-stores.md`: every `persist` names its durable fields in `partialize`.
311420
* Without one, zustand writes the whole state — transient flags, drag state, `_hasHydrated` —
@@ -315,41 +424,40 @@ function auditPersist(file: string, source: string): Violation[] {
315424
const persistImport = PERSIST_IMPORT.exec(source)
316425
if (!persistImport) return []
317426
const local = persistImport[1] ?? 'persist'
318-
/** A call of the imported middleware; `.persist(` (an instance method) is excluded. */
319-
const persistCall = new RegExp(`(?<![.\\w$])${local}\\s*(?:<[^()]*?>)?\\s*\\(`, 'g')
427+
let program: unknown
428+
try {
429+
program = parse(source, {
430+
sourceType: 'module',
431+
plugins: ['typescript', ...(/\.[jt]sx$/.test(file) ? (['jsx'] as const) : [])],
432+
errorRecovery: true,
433+
}).program
434+
} catch (error) {
435+
throw new Error(`Cannot parse ${file} to audit its persist calls: ${getErrorMessage(error)}`)
436+
}
437+
if (!isSyntaxNode(program)) return []
438+
320439
const violations: Violation[] = []
321-
for (let match = persistCall.exec(source); match; match = persistCall.exec(source)) {
322-
if (hasSafeAnnotation(source, match.index)) continue
323-
const openParenIndex = match.index + match[0].length - 1
324-
const closeParenIndex = findMatchingParen(source, openParenIndex)
325-
if (closeParenIndex === -1) continue
326-
const args = splitTopLevelArguments(
327-
stripComments(source.slice(openParenIndex + 1, closeParenIndex))
328-
)
329-
const options = args.length > 1 ? args[args.length - 1] : ''
330-
const hasPartialize = /(?:^|[{,])\s*partialize\s*[:(,}]/.test(options)
331-
/**
332-
* `(s) => s`, `(s) => ({ ...s })`, or a block body returning either — with `s` also bound as
333-
* `({ ...s })` — persists the whole state.
334-
*/
335-
const spreadsState =
336-
/\bpartialize\s*:\s*\(?\s*(?:\{\s*\.\.\.)?(\w+)[^)]*\)?\s*=>\s*(?:\1\b(?!\s*[.[])|\(\s*\{\s*\.\.\.\1\b|\{[^}]*\breturn\s+(?:\1\b(?!\s*[.[])|\{\s*\.\.\.\1\b))/.test(
337-
options
338-
)
339-
if (hasPartialize && !spreadsState) continue
440+
walkNodes(program, (node) => {
441+
if (node.type !== 'CallExpression' || !isIdentifierNamed(node.callee, local)) return
442+
if (hasSafeAnnotation(source, node.start)) return
443+
const args = Array.isArray(node.arguments) ? node.arguments : []
444+
const options = args.length > 1 ? unwrapExpression(args[args.length - 1]) : null
445+
const partialize =
446+
isSyntaxNode(options) && options.type === 'ObjectExpression' ? findPartialize(options) : null
447+
const isInlineFunction =
448+
partialize !== null &&
449+
/^(?:ArrowFunctionExpression|FunctionExpression|ObjectMethod)$/.test(partialize.type)
450+
const leak = partialize && isInlineFunction ? partializeLeak(partialize) : null
451+
if (partialize && !leak) return
340452
violations.push({
341453
file,
342-
line: lineNumberAt(source, match.index),
343-
description: spreadsState
344-
? 'persist partialize spreads the whole state; return an explicit whitelist of durable fields'
345-
: `persist has no partialize; add \`partialize: (state) => ({ <durable fields> })\` (sim-stores.md). If the options object is hoisted into a variable that has one, mark the call // ${SAFE_ANNOTATION} <where>`,
346-
snippet: oneLineSnippet(
347-
source,
348-
match.index,
349-
Math.min(closeParenIndex + 1, match.index + 180)
350-
),
454+
line: lineNumberAt(source, node.start),
455+
description:
456+
leak ??
457+
`persist has no partialize; add \`partialize: (state) => ({ <durable fields> })\` (sim-stores.md). If the options object is hoisted into a variable that has one, mark the call // ${SAFE_ANNOTATION} <where>`,
458+
snippet: oneLineSnippet(source, node.start, Math.min(node.end, node.start + 180)),
351459
})
352-
}
460+
})
353461
return violations
354462
}
355463

0 commit comments

Comments
 (0)