diff --git a/.changeset/redis-wrapper-scan-cursor.md b/.changeset/redis-wrapper-scan-cursor.md new file mode 100644 index 0000000..ba7daaa --- /dev/null +++ b/.changeset/redis-wrapper-scan-cursor.md @@ -0,0 +1,5 @@ +--- +'uptide': patch +--- + +The redis (node-redis) 4 to 5 and 5 to 6 pack is verified: 5 public upgrades, 20 real breaking sites, breaking precision 100% and recall 83%; its four rewrites no real site exercises (`keepAlive`, `quit`/`disconnect`, scan iterators, `unstableResp3`) run as assisted. It now reports numeric SCAN cursors in files that use the client through the repository's own module (`getRedisClient` from `../config/redis`, `redisClient` from `~/utils/redis.server`) without importing redis, including files the compile scope left out under the importer memory cap, and treats `scan(cursor)` after `let cursor = 0` as a numeric cursor. The resolver behind it, shared with the zod pack's form-error rule, moves to `check/importers.ts` as `packageImports`. diff --git a/README.md b/README.md index 4234638..9229f21 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ The contract: [migration packs](docs/packs.md). | `openai` | 4.x → 7.x | 100% | 15% | 20 | zeikar/charivo, openai/openai-guardrails-js, wardby/wardby, csnyder256/ux-struggle-detector, grantsingleton/batch-ai, digitalchokro/askchokro, microsoft/prompty | candidate | | `react` | 18.x → 19.x | 100% | 69% | 130 | excalidraw/excalidraw, tldraw/tldraw | verified | | `react-router` | 6.x → 7.x | 100% | 88% | 18 | Arize-ai/phoenix, getlago/lago-front | verified | -| `redis` | >=4 <6 → >=5 <7 | n/a | n/a | — | deanmoses/tacocat-gallery-sam, swoofer/mcp-coordinator, deanmoses/tacocat-gallery-sam, UNN-Devotek/FCM-Fallout-Chat-Mod, tearingItUp786/blog | candidate | +| `redis` | >=4 <6 → >=5 <7 | 100% | 83% | 20 | deanmoses/tacocat-gallery-sam, swoofer/mcp-coordinator, deanmoses/tacocat-gallery-sam, UNN-Devotek/FCM-Fallout-Chat-Mod, tearingItUp786/blog | verified | | `@sentry/browser` | 10.x → 11.x | 100% | 100% | 3 | Automancer-Ltd/automancer-site, sbomify/sbomify | candidate | | `@sentry/nextjs` | 10.x → 11.x | 100% | 100% | 8 | cohstats/coh3-stats, xdoubleu/tools.xdoubleu.com, maximunited/yomu | verified | | `@sentry/react` | 10.x → 11.x | 100% | 100% | 2 | owine/youth-activity-scheduler, successible/cleanslate | candidate | diff --git a/packages/core/src/check/importers.ts b/packages/core/src/check/importers.ts index b82f9fe..3e48293 100644 --- a/packages/core/src/check/importers.ts +++ b/packages/core/src/check/importers.ts @@ -1,5 +1,5 @@ -import { readdirSync, readFileSync, statSync } from 'node:fs'; -import { basename, dirname, join, resolve } from 'node:path'; +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; +import { basename, dirname, join, relative, resolve } from 'node:path'; import { ts } from 'ts-morph'; const SOURCE = /\.(?:[cm]?[jt]sx?)$/; @@ -299,3 +299,104 @@ export function importersOf( return packages === undefined || takesDependent(file, hits); }); } + +/** The repository file a specifier names, the way the compiler resolves it; never a package. */ +function localModule( + specifier: string, + from: string, + options: ts.CompilerOptions, +): string | undefined { + const target = ts.resolveModuleName(specifier, from, options, ts.sys).resolvedModule + ?.resolvedFileName; + return target && !target.includes('/node_modules/') && !/\.d\.[cm]?ts$/.test(target) + ? resolve(target) + : undefined; +} + +/** + * What a repository module exports that depends on `packages` (`packageExports`), through its + * own re-exports too: `export * from './schema'` is how a `lib/zod/index.ts` hands on its schemas. + */ +function localExports( + file: string, + packages: ReadonlySet, + options: ts.CompilerOptions, + seen: Map>, +): Set { + const known = seen.get(file); + if (known) return known; + const exported = packageExports(file, packages); + seen.set(file, exported); + for (const statement of parseFile(file)?.statements ?? []) { + if ( + !ts.isExportDeclaration(statement) || + statement.isTypeOnly || + !statement.moduleSpecifier || + !ts.isStringLiteral(statement.moduleSpecifier) + ) + continue; + const target = localModule(statement.moduleSpecifier.text, file, options); + if (!target) continue; + const inner = localExports(target, packages, options, seen); + const clause = statement.exportClause; + if (!clause) { + for (const name of inner) if (name !== 'default') exported.add(name); + } else if (ts.isNamespaceExport(clause)) { + if (inner.size) exported.add(clause.name.text); + } else + for (const e of clause.elements) + if (inner.has((e.propertyName ?? e.name).text)) exported.add(e.name.text); + } + return exported; +} + +/** + * The names `file` imports from the repository's own modules that depend on one of `packages` + * there (a zod schema behind `@/lib/zod`, a redis client behind `../config/redis`), resolved the + * way the compiler resolves them. Type-only imports are left out: they hold no value. + */ +export function packageImports( + file: string, + packages: ReadonlySet, + options: ts.CompilerOptions, +): Set { + const names = new Set(); + const seen = new Map>(); + for (const statement of parseFile(file)?.statements ?? []) { + if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier)) + continue; + const clause = statement.importClause; + if (!clause || clause.isTypeOnly) continue; + const target = localModule(statement.moduleSpecifier.text, file, options); + if (!target) continue; + const exported = localExports(target, packages, options, seen); + if (exported.size === 0) continue; + if (clause.name && exported.has('default')) names.add(clause.name.text); + const bindings = clause.namedBindings; + if (bindings && ts.isNamespaceImport(bindings)) names.add(bindings.name.text); + else if (bindings) + for (const e of bindings.elements) + if (!e.isTypeOnly && (exported.has((e.propertyName ?? e.name).text) || exported.has('*'))) + names.add(e.name.text); + } + return names; +} + +/** + * A workspace's own TypeScript files (workspace-relative; no declarations, nothing under + * `node_modules` or outside it) and compiler options, from its `tsconfig.json`. + */ +export function projectFiles(dir: string): { files: string[]; options: ts.CompilerOptions } { + const config = join(dir, 'tsconfig.json'); + const parsed = existsSync(config) + ? ts.getParsedCommandLineOfConfigFile( + config, + {}, + { ...ts.sys, onUnRecoverableConfigFileDiagnostic: () => {} }, + ) + : undefined; + const files = (parsed?.fileNames ?? []) + .map((f) => relative(dir, resolve(f))) + .filter((f) => !f.startsWith('..') && !f.includes('node_modules/') && !/\.d\.[cm]?ts$/.test(f)); + return { files, options: parsed?.options ?? {} }; +} diff --git a/packages/core/src/packs/contract.ts b/packages/core/src/packs/contract.ts index baab9f0..330b700 100644 --- a/packages/core/src/packs/contract.ts +++ b/packages/core/src/packs/contract.ts @@ -356,7 +356,7 @@ export function ruleFor( } /** Rule and note sites found in one file's text, as `check` reports a pack's own finding. */ -function detected( +export function detected( pack: Pick, input: { file: string; text: string; from: string; to: string }, ): Finding[] { diff --git a/packages/core/src/packs/redis/RESEARCH.md b/packages/core/src/packs/redis/RESEARCH.md index e922a3d..ca65b4e 100644 --- a/packages/core/src/packs/redis/RESEARCH.md +++ b/packages/core/src/packs/redis/RESEARCH.md @@ -98,7 +98,8 @@ repository's own fix, and deprecations the code already had at the installed ver ## Measured pack test -`uptide pack test redis`, 2026-10-14: +Recorded **verified** on 2026-10-15 (UPT-66): `uptide pack test redis --write` under UPT-48's gate, +ground truth unchanged (the record's digest is that of the committed `ground-truth.json`): ``` rule precision recall tp fp fn @@ -106,32 +107,63 @@ repository's own fix, and deprecations the code already had at the installed ver client-type 100% 33% 2 0 4 generic 50% 100% 2 2 0 renamed-exports 100% 100% 2 0 0 - scan-cursor 100% 30% 3 0 7 - breaking (sites) 100% 61% 11 0 7 + scan-cursor 100% 70% 7 0 3 + overall (sites) 89% 77% 17 2 5 behavior notes (sites) 50% 50% 2 2 2 + breaking (sites) 100% 83% 15 0 3 + +gate 20 breaking sites, 2 behavior-note sites in public ground truth · verified + · no real site exercises keep-alive, quit-disconnect, scan-iterators, unstable-resp3: their rewrites run as assisted ``` -Breaking misses, none of them a rule that did not match: +5 repositories, 20 real breaking sites, no breaking false positive. The four rewrites no real site +exercises (`keep-alive`, `quit-disconnect`, `scan-iterators`, `unstable-resp3`) are listed as +`unexercised` in `verification.json`, so `fix` hands those sites to the agent (assisted) instead of +applying the rewrite. + +The three remaining breaking misses, all in tearingItUp786/blog: + +- `app/inngest/refresh-cache.ts:204` and `:206`: TS2345 at `scanRedisKeys(redisClient, …)`, a + compile error only. `check` follows the file's import of `redisClient` from + `~/utils/redis.server` (`importersOf`), but the importer memory cap (`IMPORTER_GROWTH`, 0.2 of the + roots' own estimated heap) leaves it out of the program, and the run says so: "4 importing files + not analyzed (memory cap)". A growth of 0.5 would take it. The cap is engine-wide and stays as it + is here; raising it is a separate decision. +- `app/inngest/utils.ts:106`: `cursor !== 0` in a helper that takes a local structural + `RedisScanClient` and imports nothing from redis or from a module that depends on it. Following + imports cannot reach it; only a call-graph step (the caller passes the client in) could. -- blog (5 scan-cursor): `refresh-cache.ts` and `utils.ts` reach the client through a local module - and a local `RedisScanClient` type and never import `redis`; `check` reports no compile error - there, and runtime detection reads importing files only. -- FCM and tacocat 5 → 6 (4 client-type): the repositories' own `tsc` reports TS2322 at these lines, - `check` reports nothing there. Both lines already had a different TS2322 at the installed version - (`ReturnType` against a configured client) and `check` subtracts errors it - finds at the installed version, which is most likely why (not confirmed in the engine). -- FCM `cursor !== 0` (2): the files import a local wrapper, not `redis`. +Reported under another rule (they count as found for breaking, against `client-type` in the rule +rows): FCM `src/config/redis.ts:39`, `:46` (as `generic`) and tacocat 5 → 6 +`redisClientUtils.ts:16`, `:21` (as `client-defaults`). Behavior: the two `client-defaults` false positives are clients whose repositories accepted the RESP3 default without pinning (mcp-coordinator, blog). The defaults did change for them; the ground truth rule (only what the repository changed) counts them against the note. FCM pins RESP2 in an options function the detector cannot read (it reads inline literals only). +### Clients reached through a local module (UPT-66) + +The first score (2026-10-14) had breaking recall 61% (11/18): 7 `scan-cursor` misses, FCM's two +`cursor !== 0` and blog's five. The detector read only files that import redis, and those files +take the client from the repository's own module. + +`scan-cursor` now also reads files that never import redis but import a value from a repository +module that depends on it (`getRedisClient` from `../config/redis`, `redisClient` from +`~/utils/redis.server`), resolved the way the compiler resolves it, `export *` chains included: +`packageImports` in `check/importers.ts`, the resolver the zod pack's `error-members` uses. Every +tsconfig file is a candidate, so a file the importer cap left out is read too. Such a file needs a +scan call; the other detectors still read importing files only (their sites are option names, not +client calls). `scan(cursor)` after `let cursor = 0` counts as a numeric cursor (fixture +`scan-cursor` line 9 is now marked). Found since: FCM `adminNukeController.ts:68`, +`roleVerificationService.ts:72`; blog `refresh-cache.ts:322`, `:326`. 61% → 83%, no new false +positive. + ### Per-rule evidence (for the UPT-48 gate) Exercised by a real site: `renamed-exports` (its rewrite too), `scan-cursor`, `client-type`. Rewrites with no real site: `scan-iterators`, `keep-alive`, `quit-disconnect`, `unstable-resp3`; -under UPT-48 they run as assisted. Rules and notes with no real site: `legacy-mode`, +they run as assisted (`unexercised` in the record). Rules and notes with no real site: `legacy-mode`, `command-options`, `number-replies`, `deep-imports`, `graph-removed`, `set-expiration`, `sentinel-node-command-options`, `search-dialect-2` and the decision-only notes. @@ -147,4 +179,5 @@ has `client-type` in four repositories, two of them scored. gate in `transform` only refuses rewrites of a major the upgrade already passed. - A site reported but deliberately not rewritten (a scan loop with `break`) cannot be a fixture marker; it is a unit test. -- Compile errors in files that use the client without importing the package are not reported (blog). +- Compile errors in an importing file the importer memory cap leaves out are not reported (blog + `refresh-cache.ts:204`, `:206`); only the syntactic `scan-cursor` sites there are. diff --git a/packages/core/src/packs/redis/detect.ts b/packages/core/src/packs/redis/detect.ts index 634ceea..b48a4e6 100644 --- a/packages/core/src/packs/redis/detect.ts +++ b/packages/core/src/packs/redis/detect.ts @@ -79,12 +79,25 @@ function bindings(source: SourceFile): Bindings { return { named, modules, uses }; } -function using(text: string, file: string): { source: SourceFile; b: Bindings } | undefined { +const MENTIONS = /['"](?:redis|@redis\/[\w-]+)(?:\/[^'"]*)?['"]/; + +/** Whether a file names one of the packages at all: what `detect` reads. */ +export const mentionsRedis = (text: string): boolean => MENTIONS.test(text); + +/** + * A node-redis file, or, with `clients` (names it imports from a repository module that depends + * on redis: `getRedisClient` from `../config/redis`), a file that uses the client through it. + */ +function using( + text: string, + file: string, + clients?: ReadonlySet, +): { source: SourceFile; b: Bindings } | undefined { // Cheap gate before parsing: most files never mention the packages. - if (!/['"](?:redis|@redis\/[\w-]+)(?:\/[^'"]*)?['"]/.test(text)) return undefined; + if (!MENTIONS.test(text) && !clients?.size) return undefined; const source = parse(text, file); const b = bindings(source); - return b.uses ? { source, b } : undefined; + return b.uses || clients?.size ? { source, b } : undefined; } const lineOf = (source: SourceFile, node: Node) => @@ -267,12 +280,30 @@ const EQUALITY = new Set([ SyntaxKind.ExclamationEqualsToken, ]); +const SCANS = /\.(?:scan|hScan|sScan|zScan)\s*\(/; + +/** A numeric literal, or a variable declared with one (`let cursor = 0`). */ +function numeric(node: Node): boolean { + if (Node.isNumericLiteral(node)) return true; + if (!Node.isIdentifier(node)) return false; + return (node.getSymbol()?.getDeclarations() ?? []).some((d) => { + const value = Node.isVariableDeclaration(d) ? d.getInitializer() : undefined; + return value !== undefined && Node.isNumericLiteral(value); + }); +} + /** - * A numeric SCAN cursor: `scan(0)`, `hScan(key, 0)`, or a cursor compared with a number. 5 takes - * and returns the cursor as a string, so `cursor !== 0` never ends the loop in JavaScript. + * A numeric SCAN cursor: `scan(0)`, `hScan(key, 0)`, `scan(cursor)` after `let cursor = 0`, or a + * cursor compared with a number. 5 takes and returns the cursor as a string, so `cursor !== 0` + * never ends the loop in JavaScript. With `clients`, a file that never imports redis is read too + * when it makes a scan call (see `using`). */ -export function scanCursorSites(text: string, file: string): SourceSite[] { - const found = using(text, file); +export function scanCursorSites( + text: string, + file: string, + clients?: ReadonlySet, +): SourceSite[] { + const found = using(text, file, SCANS.test(text) ? clients : undefined); if (!found) return []; const { source } = found; const sites: SourceSite[] = []; @@ -282,7 +313,7 @@ export function scanCursorSites(text: string, file: string): SourceSite[] { const name = callee.getName(); const index = name === 'scan' ? 0 : /^(?:hScan|sScan|zScan)$/.test(name) ? 1 : -1; const cursor = index >= 0 ? call.getArguments()[index] : undefined; - if (cursor && Node.isNumericLiteral(cursor)) sites.push(site(source, call)); + if (cursor && numeric(cursor)) sites.push(site(source, call)); } for (const binary of source.getDescendantsOfKind(SyntaxKind.BinaryExpression)) { if (!EQUALITY.has(binary.getOperatorToken().getKind())) continue; diff --git a/packages/core/src/packs/redis/fixtures/scan-cursor/before.ts b/packages/core/src/packs/redis/fixtures/scan-cursor/before.ts index 43f9f2e..cb9cc93 100644 --- a/packages/core/src/packs/redis/fixtures/scan-cursor/before.ts +++ b/packages/core/src/packs/redis/fixtures/scan-cursor/before.ts @@ -6,7 +6,7 @@ export async function keys(pattern: string) { const found: string[] = []; let cursor = 0; do { - const reply = await client.scan(cursor, { MATCH: pattern, COUNT: 100 }); + const reply = await client.scan(cursor, { MATCH: pattern, COUNT: 100 }); // @uptide scan-cursor cursor = reply.cursor; found.push(...reply.keys); } while (cursor !== 0); // @uptide scan-cursor diff --git a/packages/core/src/packs/redis/index.ts b/packages/core/src/packs/redis/index.ts index 79dc7f9..b80569f 100644 --- a/packages/core/src/packs/redis/index.ts +++ b/packages/core/src/packs/redis/index.ts @@ -1,5 +1,8 @@ +import { join, resolve } from 'node:path'; +import { packageImports, projectFiles } from '../../check/importers.js'; +import type { Finding } from '../../domain/report.js'; import { satisfies } from '../../fetch/range.js'; -import { definePack, type Pack, type PackRule, replaceAtSite } from '../contract.js'; +import { definePack, detected, type Pack, type PackRule, replaceAtSite } from '../contract.js'; import type { TransformResult } from '../types.js'; import { crosses, NOTE_MAJOR, redisBehavior } from './behavior.js'; import { @@ -7,6 +10,7 @@ import { graphSites, keepAliveSites, legacyModeSites, + mentionsRedis, rewriteKeepAlive, rewriteScanIterator, rewriteUnstableResp3, @@ -270,6 +274,39 @@ const base = definePack({ "Sources read 2026-10-14 at node-redis b64bacb; see RESEARCH.md. Migrate only the reported site. An upgrade from 4 crosses both majors: apply the 5 changes, then the 6 ones. Keep 5's client behavior unless the decision for that client says otherwise: where a client is reported under client-defaults, pin the 5 defaults on it (`RESP: 2`, `commandOptions: { timeout: undefined }`, `socket: { keepAliveInitialDelay: 5000 }`) rather than adapting callers to RESP3 replies. Never cast a client or a reply to any, never convert a SCAN cursor to a number, and never suppress a diagnostic to get a clean compile.", }); +const PACKAGES = new Set([ + 'redis', + '@redis/client', + '@redis/bloom', + '@redis/json', + '@redis/search', + '@redis/time-series', + '@redis/graph', +]); +const SCAN_CURSOR = V5.find((r) => r.id === 'scan-cursor') as PackRule; + +/** + * SCAN cursors in files that use the client through the repository's own module and never import + * redis (`getRedisClient` from `../config/redis`, `redisClient` from `~/utils/redis.server`): + * `detect` reads only files that import it, and the compile scope can leave such a file out (the + * importer cap). Every project file is a candidate; the text tests are the cheap part. + */ +function wrapperFindings(input: Parameters>[0]): Finding[] { + const dir = resolve(input.root, input.workspace); + const { files, options } = projectFiles(dir); + return [...new Set([...input.usages.map((u) => u.file), ...files])].sort().flatMap((file) => { + const text = input.read(join(input.workspace, file)); + if (text === undefined || mentionsRedis(text) || !/\bcursor\b/i.test(text)) return []; + const clients = packageImports(join(dir, file), PACKAGES, options); + if (clients.size === 0) return []; + const rule = { + ...SCAN_CURSOR, + detect: (t: string, f: string) => scanCursorSites(t, f, clients), + }; + return detected({ meta: base.meta, rules: [rule], behavior: [] }, { ...input, file, text }); + }); +} + /** * redis >=4 <6 → >=5 <7. One package, two majors: 4 → 5 and 5 → 6 are separate rule sets, and * an upgrade gets the sets of the majors it crosses (4 → 6 gets both). Pack lookup stays one pack @@ -299,5 +336,7 @@ export const redisPack: Pack = { return base.transform(text, finding, context); }, runtimeFindings: (input) => - (base.runtimeFindings?.(input) ?? []).filter((f) => appliesTo(f.rule, input.from, input.to)), + [...(base.runtimeFindings?.(input) ?? []), ...wrapperFindings(input)].filter((f) => + appliesTo(f.rule, input.from, input.to), + ), }; diff --git a/packages/core/src/packs/redis/redis.test.ts b/packages/core/src/packs/redis/redis.test.ts index f690e6b..8bda6d4 100644 --- a/packages/core/src/packs/redis/redis.test.ts +++ b/packages/core/src/packs/redis/redis.test.ts @@ -1,9 +1,12 @@ +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { expect, it } from 'vitest'; +import { afterAll, describe, expect, it } from 'vitest'; import type { Finding } from '../../domain/report.js'; import type { Usage } from '../../domain/usage.js'; import { runFixtures } from '../tooling/fixtures.js'; -import { rewriteScanIterator } from './detect.js'; +import { rewriteScanIterator, scanCursorSites } from './detect.js'; import { appliesTo, redisPack } from './index.js'; it('redis: every fixture rewrites and detects exactly its marked sites', () => { @@ -169,3 +172,145 @@ it('redis: compiler findings are claimed by the rule the diagnostic names', () = ), ).toBe('renamed-exports'); }); + +it('redis: a scan call whose cursor variable starts as a number is a numeric cursor', () => { + const text = [ + "import type { RedisClientType } from 'redis';", + 'declare const client: RedisClientType;', + 'export async function wipe() {', + ' let cursor = 0;', + " const reply = await client.scan(cursor, { MATCH: 'k:*' });", + " let next = '0';", + ' await client.scan(next);', + '}', + ].join('\n'); + expect(scanCursorSites(text, 'a.ts').map((s) => s.line)).toEqual([5]); +}); + +describe('redis in check: a client reached through a local module', () => { + const root = mkdtempSync(join(tmpdir(), 'uptide-redis-wrapper-')); + afterAll(() => rmSync(root, { recursive: true, force: true })); + const write = (file: string, text: string) => { + mkdirSync(dirname(join(root, file)), { recursive: true }); + writeFileSync(join(root, file), text); + }; + const loop = (client: string) => + [ + ' let cursor = 0;', + ' do {', + ` const reply = await ${client}.scan(cursor, { MATCH: 'k:*', COUNT: 100 });`, + ' cursor = Number(reply.cursor);', + ' } while (cursor !== 0);', + ].join('\n'); + // UNN-Devotek/FCM-Fallout-Chat-Mod (backend): a getter typed off createClient, imported by + // relative path. + write('fcm/tsconfig.json', JSON.stringify({ compilerOptions: { strict: true } })); + write( + 'fcm/src/config/redis.ts', + [ + "import { createClient } from 'redis';", + 'const client = createClient();', + 'async function getRedisClient(): Promise> {', + ' return client;', + '}', + 'export { getRedisClient };', + ].join('\n'), + ); + write( + 'fcm/src/controllers/nuke.ts', + [ + "import { getRedisClient } from '../config/redis';", + 'export async function nuke() {', + ' const redis = await getRedisClient();', + loop('redis'), + '}', + ].join('\n'), + ); + // Not redis: a local helper's own cursor. + write( + 'fcm/src/lib/page.ts', + 'export const pages = { scan: async (c: number) => ({ cursor: c }) };\n', + ); + write( + 'fcm/src/controllers/list.ts', + [ + "import { pages } from '../lib/page';", + 'export async function list() {', + loop('pages'), + '}', + ].join('\n'), + ); + // tearingItUp786/blog: the client behind a `~/` path alias, and a helper that only takes a + // structural client type and never names redis. + write( + 'blog/tsconfig.json', + JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { '~/*': ['./app/*'] } } }), + ); + write( + 'blog/app/utils/redis.server.ts', + [ + "import * as redis from 'redis';", + 'function createRedisClient(): redis.RedisClientType {', + ' return redis.createClient();', + '}', + 'const redisClient = createRedisClient();', + 'export { redisClient };', + ].join('\n'), + ); + write( + 'blog/app/inngest/refresh-cache.ts', + [ + "import { redisClient } from '~/utils/redis.server';", + "import { scanRedisKeys } from './utils';", + 'export async function refresh() {', + loop('redisClient'), + " return scanRedisKeys(redisClient, 'gql:*');", + '}', + ].join('\n'), + ); + write( + 'blog/app/inngest/utils.ts', + [ + 'type Client = { scan: (c: number, o: object) => Promise<{ cursor: string; keys: string[] }> };', + 'export async function scanRedisKeys(redisClient: Client, pattern: string) {', + loop('redisClient'), + '}', + ].join('\n'), + ); + const run = (workspace: string, usages: string[]) => + ( + redisPack.runtimeFindings?.({ + root, + workspace, + from: '4.7.1', + to: '6.0.0', + installedDir: '', + targetDir: '', + usages: usages.map((file) => ({ file }) as Usage), + read: (file) => { + try { + return readFileSync(join(root, file), 'utf8'); + } catch { + return undefined; + } + }, + }) ?? [] + ) + .filter((f) => f.rule === 'scan-cursor') + .map((f) => `${f.usage.file}:${f.usage.line}`) + .sort(); + it('reports the cursor of a client typed off createClient in another module (FCM)', () => { + expect(run('fcm', ['src/config/redis.ts', 'src/controllers/nuke.ts'])).toEqual([ + 'src/controllers/nuke.ts:6', + 'src/controllers/nuke.ts:8', + ]); + }); + it('reads files the compile scope left out, through tsconfig paths (blog)', () => { + // refresh-cache.ts is not among the usages: the importer cap left it out of the program. + // utils.ts never takes anything from a redis module, so it stays out of reach. + expect(run('blog', ['app/utils/redis.server.ts'])).toEqual([ + 'app/inngest/refresh-cache.ts:6', + 'app/inngest/refresh-cache.ts:8', + ]); + }); +}); diff --git a/packages/core/src/packs/redis/verification.json b/packages/core/src/packs/redis/verification.json index ed27c4b..5da4f02 100644 --- a/packages/core/src/packs/redis/verification.json +++ b/packages/core/src/packs/redis/verification.json @@ -1,19 +1,23 @@ { - "status": "candidate", - "truth": "sha256-8a955c4e896f7c89acebcb71f61869a533cdca703fd82a3d5da78af9e129fc6e", - "repos": 0, + "status": "verified", + "truth": "sha256-c1f3f368599bf3da7dfe06384c336917a4cd38e25451a0e0a2f4e5749d4b5b46", + "repos": 5, "breaking": { "precision": 1, - "recall": 1, + "recall": 0.8333, "falsePositives": 0, - "truePositives": 0, - "falseNegatives": 0 + "truePositives": 15, + "falseNegatives": 3 }, "behavior": { - "precision": 1, - "recall": 1, - "falsePositives": 0, - "truePositives": 0, - "falseNegatives": 0 - } + "precision": 0.5, + "recall": 0.5, + "falsePositives": 2, + "truePositives": 2, + "falseNegatives": 2 + }, + "class": "breaking", + "breakingSites": 20, + "behaviorSites": 2, + "unexercised": ["keep-alive", "quit-disconnect", "scan-iterators", "unstable-resp3"] } diff --git a/packages/core/src/packs/zod/error-members.ts b/packages/core/src/packs/zod/error-members.ts index 6d4d9c0..cb1a4de 100644 --- a/packages/core/src/packs/zod/error-members.ts +++ b/packages/core/src/packs/zod/error-members.ts @@ -1,5 +1,4 @@ -import { existsSync, readFileSync } from 'node:fs'; -import { join, relative, resolve } from 'node:path'; +import { join, resolve } from 'node:path'; import { type BindingElement, type Expression, @@ -11,7 +10,7 @@ import { SyntaxKind, ts, } from 'ts-morph'; -import { packageExports } from '../../check/importers.js'; +import { packageImports, projectFiles } from '../../check/importers.js'; import type { Finding } from '../../domain/report.js'; import type { Usage } from '../../domain/usage.js'; import { onReset } from '../../shared-state.js'; @@ -36,6 +35,7 @@ const FLAT = new Set(['fieldErrors', 'formErrors']); const MENTIONS = /\bformErrors\b|\.errors\b/; /** The specifier whose `ZodError` lost the members: zod 4's root. `zod/v3` keeps them. */ const ZOD = 'zod'; +const ZODS = new Set([ZOD]); const parsed = new Map(); onReset(() => parsed.clear()); @@ -182,7 +182,7 @@ function usesZod(source: SourceFile, schemas: ReadonlySet): boolean { const LOCAL = /^(?:\.{1,2}\/|[@~#]\/)/; /** * Without the repository (fixtures, `detect`), the names imported from a local specifier stand - * in for the resolved ones `schemaImports` gives `check`. + * in for the resolved ones `packageImports` gives `check`. */ function localImports(source: SourceFile, local = LOCAL): Set { const names = new Set(); @@ -200,7 +200,7 @@ function localImports(source: SourceFile, local = LOCAL): Set { /** * Sites in one file. `schemas` are the names it imports from the repository's own modules - * that are zod schemas there (`schemaImports`); without them, the names it imports from local + * that are zod schemas there (`packageImports`); without them, the names it imports from local * specifiers. A file that neither imports zod nor such a name has none. */ export function errorMemberSites( @@ -223,99 +223,6 @@ export function errorMemberSites( }); } -const sourceOf = (file: string): ts.SourceFile | undefined => { - try { - return ts.createSourceFile(file, readFileSync(file, 'utf8'), ts.ScriptTarget.Latest, false); - } catch { - return undefined; - } -}; -/** The repository file a specifier names, the way the compiler resolves it; never a package. */ -function local(specifier: string, from: string, options: ts.CompilerOptions): string | undefined { - const target = ts.resolveModuleName(specifier, from, options, ts.sys).resolvedModule - ?.resolvedFileName; - return target && !target.includes('/node_modules/') && !/\.d\.[cm]?ts$/.test(target) - ? resolve(target) - : undefined; -} - -/** - * What a repository module exports that depends on zod (`packageExports`), through its own - * re-exports too: `export * from './schema'` is how a `lib/zod/index.ts` hands on its schemas. - */ -function zodExports( - file: string, - options: ts.CompilerOptions, - seen = new Map>(), -): Set { - const known = seen.get(file); - if (known) return known; - const exported = packageExports(file, new Set([ZOD])); - seen.set(file, exported); - const source = sourceOf(file); - for (const statement of source?.statements ?? []) { - if ( - !ts.isExportDeclaration(statement) || - statement.isTypeOnly || - !statement.moduleSpecifier || - !ts.isStringLiteral(statement.moduleSpecifier) - ) - continue; - const target = local(statement.moduleSpecifier.text, file, options); - if (!target) continue; - const inner = zodExports(target, options, seen); - const clause = statement.exportClause; - if (!clause) { - for (const name of inner) if (name !== 'default') exported.add(name); - } else if (ts.isNamespaceExport(clause)) { - if (inner.size) exported.add(clause.name.text); - } else - for (const e of clause.elements) - if (inner.has((e.propertyName ?? e.name).text)) exported.add(e.name.text); - } - return exported; -} - -/** - * The names `file` imports from the repository's own modules that depend on zod there - * (`export const createApiKeySchema = z.object(...)` behind `@/lib/zod`), resolved the way the - * compiler resolves them. - */ -export function schemaImports(file: string, options: ts.CompilerOptions): Set { - const names = new Set(); - const seen = new Map>(); - for (const statement of sourceOf(file)?.statements ?? []) { - if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier)) - continue; - const clause = statement.importClause; - if (!clause || clause.isTypeOnly) continue; - const target = local(statement.moduleSpecifier.text, file, options); - if (!target) continue; - const exported = zodExports(target, options, seen); - if (exported.size === 0) continue; - if (clause.name && exported.has('default')) names.add(clause.name.text); - const bindings = clause.namedBindings; - if (bindings && ts.isNamespaceImport(bindings)) names.add(bindings.name.text); - else if (bindings) - for (const e of bindings.elements) - if (!e.isTypeOnly && (exported.has((e.propertyName ?? e.name).text) || exported.has('*'))) - names.add(e.name.text); - } - return names; -} - -function compilerOptions(dir: string): { files: string[]; options: ts.CompilerOptions } { - const config = join(dir, 'tsconfig.json'); - const parsed = existsSync(config) - ? ts.getParsedCommandLineOfConfigFile( - config, - {}, - { ...ts.sys, onUnRecoverableConfigFileDiagnostic: () => {} }, - ) - : undefined; - return { files: parsed?.fileNames ?? [], options: parsed?.options ?? {} }; -} - /** * The pack's own findings for one workspace: the files that use zod, and the files importing * a zod schema from one of them, which the type scan does not visit (a form validating with @@ -333,16 +240,17 @@ export function errorMemberFindings(input: { }): Finding[] { const dir = resolve(input.root, input.workspace); const direct = [...new Set(input.usages.map((u) => u.file))].sort(); - const { files, options } = compilerOptions(dir); // The project's own files, not only those the scan visited: the text test is the cheap part. - const own = files - .map((f) => relative(dir, resolve(f))) - .filter((f) => !f.startsWith('..') && !f.includes('node_modules/') && !/\.d\.[cm]?ts$/.test(f)); + const { files: own, options } = projectFiles(dir); const findings: Finding[] = []; for (const file of [...new Set([...direct, ...own])].sort()) { const text = input.read(join(input.workspace, file)); if (text === undefined || !MENTIONS.test(text)) continue; - for (const site of errorMemberSites(text, file, schemaImports(join(dir, file), options))) { + for (const site of errorMemberSites( + text, + file, + packageImports(join(dir, file), ZODS, options), + )) { const finding: Finding = { change: { package: ZOD,