From d9f13ce129371ca1df4cd90af01e136eb9526954 Mon Sep 17 00:00:00 2001 From: bohdansolovie <153934212+bohdansolovie@users.noreply.github.com> Date: Sun, 5 Jul 2026 20:18:27 +0200 Subject: [PATCH] feat(enrichment): add enum/union exhaustiveness-drift analyzer Detect switches that covered every old enum or union member but omit a variant newly added by the PR. Fixes #2028 --- .env.example | 6 +- apps/gittensory-ui/src/lib/rees-analyzers.ts | 26 ++ review-enrichment/analyzer-metadata.json | 28 ++ .../src/analyzers/exhaustiveness-drift.ts | 355 ++++++++++++++++++ review-enrichment/src/analyzers/registry.ts | 35 ++ review-enrichment/src/render.ts | 1 + review-enrichment/src/types.ts | 11 + .../test/analyzer-registry.test.ts | 1 + .../test/exhaustiveness-drift.test.ts | 152 ++++++++ src/review/enrichment-analyzer-names.ts | 1 + 10 files changed, 613 insertions(+), 3 deletions(-) create mode 100644 review-enrichment/src/analyzers/exhaustiveness-drift.ts create mode 100644 review-enrichment/test/exhaustiveness-drift.test.ts diff --git a/.env.example b/.env.example index 9088385604..14cb5dddaf 100644 --- a/.env.example +++ b/.env.example @@ -69,7 +69,7 @@ GITTENSORY_REVIEW_ENRICHMENT=false # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber # conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y -# i18n,unusedExport,commitLint +# i18n,unusedExport,exhaustiveness,commitLint # # Profile defaults: # fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency @@ -82,14 +82,14 @@ GITTENSORY_REVIEW_ENRICHMENT=false # churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch # commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology # todoMarker,magicNumber,conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting -# errorSwallow,unsafeAny,a11y,i18n,unusedExport,commitLint +# errorSwallow,unsafeAny,a11y,i18n,unusedExport,exhaustiveness,commitLint # deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber # conflictMarker,debugLeftover,sizeSmell,floatingPromise,deepNesting,errorSwallow,unsafeAny,a11y -# i18n,unusedExport,commitLint +# i18n,unusedExport,exhaustiveness,commitLint # END GENERATED REES ANALYZERS # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index 69254b823f..49bdb53cd2 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -1142,6 +1142,32 @@ export const REES_ANALYZERS = [ "Conservative: re-export lists and `export *` are ignored (same as undocumented-export). Skips symbols shorter than 3 chars. Checks same-file references in the headSha file before querying default-branch Code Search (where brand-new PR exports are usually absent). Fail-safe on search errors or incomplete results.", }, }, + { + name: "exhaustiveness", + title: "Enum/union exhaustiveness drift", + category: "quality", + cost: "github-light", + defaultEnabled: true, + profiles: ["balanced", "deep"], + requires: ["files", "github-token", "head-sha"], + limits: { + maxFiles: 10, + maxFetches: 10, + maxFindings: 25, + }, + docs: { + summary: + "Flags when a PR adds a new enum member or string-literal union variant but an exhaustive switch still omits it.", + looksAt: + "Added enum/union members in changed TS/JS files, comparing pre-PR vs headSha member sets and scanning changed files for switches that covered all old members.", + reports: + "Type file, line, union/enum name, added member, and optional consumer file — never file contents.", + network: + "Bounded GitHub contents fetches at headSha for changed source files. Requires GitHub token forwarding for private repos.", + notes: + "Conservative: only explicit enum/union case labels; switches with a default branch are skipped. Fail-safe on fetch errors or ambiguous type parsing.", + }, + }, { name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index 6cd01c9b18..881d724e51 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1293,6 +1293,34 @@ "notes": "Conservative: re-export lists and `export *` are ignored (same as undocumented-export). Skips symbols shorter than 3 chars. Checks same-file references in the headSha file before querying default-branch Code Search (where brand-new PR exports are usually absent). Fail-safe on search errors or incomplete results." } }, + { + "name": "exhaustiveness", + "title": "Enum/union exhaustiveness drift", + "category": "quality", + "cost": "github-light", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "requires": [ + "files", + "github-token", + "head-sha" + ], + "limits": { + "maxFiles": 10, + "maxFetches": 10, + "maxFindings": 25 + }, + "docs": { + "summary": "Flags when a PR adds a new enum member or string-literal union variant but an exhaustive switch still omits it.", + "looksAt": "Added enum/union members in changed TS/JS files, comparing pre-PR vs headSha member sets and scanning changed files for switches that covered all old members.", + "reports": "Type file, line, union/enum name, added member, and optional consumer file — never file contents.", + "network": "Bounded GitHub contents fetches at headSha for changed source files. Requires GitHub token forwarding for private repos.", + "notes": "Conservative: only explicit enum/union case labels; switches with a default branch are skipped. Fail-safe on fetch errors or ambiguous type parsing." + } + }, { "name": "commitLint", "title": "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/exhaustiveness-drift.ts b/review-enrichment/src/analyzers/exhaustiveness-drift.ts new file mode 100644 index 0000000000..e41852ccc0 --- /dev/null +++ b/review-enrichment/src/analyzers/exhaustiveness-drift.ts @@ -0,0 +1,355 @@ +// Enum / literal-union exhaustiveness-drift analyzer (#2028). Flags when a PR adds a new enum member or string-literal +// union variant but a switch that previously covered every old member still omits the new one. Fetches changed type +// files and other changed consumer files at headSha (injected fetch), reverse-applies the patch to recover the +// pre-PR member set, and only reports high-confidence misses (explicit enum/union cases, no default branch). Bounded +// file-fetch caps; fail-safe on missing token/headSha, bad slug, or fetch errors. +import type { EnrichRequest, ExhaustivenessFinding } from "../types.js"; +import { reconstructOldContent } from "./doc-comment-drift.js"; +import { isDiffFileHeaderLine } from "./diff-lines.js"; +import { isTestPath } from "./test-ratio.js"; + +const GITHUB_API = "https://api.github.com"; +const SLUG_RE = /^[A-Za-z0-9._-]+$/; +const MAX_FILES = 10; +const MAX_FETCHES = 10; +const MAX_FINDINGS = 25; +const MAX_FETCH_BYTES = 1_000_000; +const SOURCE_RE = /\.(?:ts|tsx|mts|cts|js|jsx|mjs|cjs)$/; +const SKIP_RE = /(?:\.d\.ts$|\.min\.|(?:^|\/)(?:dist|build|vendor)\/)/; + +const ENUM_DECL_RE = /^\s*(?:export\s+)?(?:declare\s+)?(?:const\s+)?enum\s+([A-Za-z_$][\w$]*)\s*\{/; +const ENUM_MEMBER_RE = /^\s*([A-Za-z_$][\w$]*)\s*(?:=\s*[^,{]+)?,?\s*(?:\/\/.*)?$/; +const UNION_DECL_RE = /^\s*(?:export\s+)?type\s+([A-Za-z_$][\w$]*)\s*=\s*/; +const UNION_MEMBER_RE = /^\s*\|\s*["']([^"']+)["']\s*/; +const DEFAULT_CASE_RE = /^\s*default\s*:/; + +interface ScanOptions { + signal?: AbortSignal; +} + +interface AddedMemberCandidate { + file: string; + unionName: string; + addedMember: string; + line: number; + kind: "enum" | "union"; +} + +function escapeRegExp(value: string): string { + return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&"); +} + +function isScannablePath(path: string): boolean { + return SOURCE_RE.test(path) && !SKIP_RE.test(path) && !isTestPath(path); +} + +function githubHeaders(token: string): Record { + return { + Authorization: `Bearer ${token}`, + Accept: "application/vnd.github.raw", + "X-GitHub-Api-Version": "2022-11-28", + }; +} + +async function readBoundedText(resp: Response, signal?: AbortSignal): Promise { + const length = Number(resp.headers.get("content-length")); + if (Number.isFinite(length) && length > MAX_FETCH_BYTES) return null; + if (!resp.body) return null; + const reader = resp.body.getReader(); + const decoder = new TextDecoder(); + let size = 0; + let text = ""; + try { + while (true) { + if (signal?.aborted) return null; + const { done, value } = await reader.read(); + if (done) break; + size += value.byteLength; + if (size > MAX_FETCH_BYTES) { + await reader.cancel(); + return null; + } + text += decoder.decode(value, { stream: true }); + } + text += decoder.decode(); + return text; + } finally { + reader.releaseLock(); + } +} + +async function fetchFileAtHead( + owner: string, + repo: string, + path: string, + headSha: string, + token: string, + fetchFn: typeof fetch, + signal: AbortSignal | undefined, +): Promise { + try { + const encoded = path.split("/").map(encodeURIComponent).join("/"); + const resp = await fetchFn( + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encoded}?ref=${encodeURIComponent(headSha)}`, + { headers: githubHeaders(token), signal }, + ); + if (!resp.ok) return null; + return await readBoundedText(resp, signal); + } catch { + return null; + } +} + +/** Walk a unified diff and collect newly added enum/union members with their declaring type name and new-file line. */ +export function parseAddedTypeMembers( + patch: string, +): Array<{ unionName: string; addedMember: string; line: number; kind: "enum" | "union" }> { + const out: Array<{ unionName: string; addedMember: string; line: number; kind: "enum" | "union" }> = []; + let newLine = 0; + let enumName: string | null = null; + let unionName: string | null = null; + let enumDepth = 0; + + for (const raw of patch.split("\n")) { + const header = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(raw); + if (header) { + newLine = Number(header[1]); + enumName = null; + unionName = null; + enumDepth = 0; + continue; + } + + const isAdd = raw.startsWith("+") && !isDiffFileHeaderLine(raw); + const isContext = !raw.startsWith("-") && !raw.startsWith("\\") && !isDiffFileHeaderLine(raw); + if (!isAdd && !isContext) continue; + + const line = isAdd ? raw.slice(1) : raw.startsWith(" ") ? raw.slice(1) : raw; + const enumDecl = ENUM_DECL_RE.exec(line); + if (enumDecl) { + enumName = enumDecl[1]!; + unionName = null; + enumDepth = (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length; + } + const unionDecl = UNION_DECL_RE.exec(line); + if (unionDecl) { + unionName = unionDecl[1]!; + enumName = null; + enumDepth = 0; + } + + if (isAdd) { + if (enumName && enumDepth >= 0) { + const member = ENUM_MEMBER_RE.exec(line); + if (member && member[1] !== "const") { + out.push({ unionName: enumName, addedMember: member[1]!, line: newLine, kind: "enum" }); + } + } + const unionMember = UNION_MEMBER_RE.exec(line); + if (unionName && unionMember) { + out.push({ unionName, addedMember: unionMember[1]!, line: newLine, kind: "union" }); + } + newLine += 1; + } else { + if (enumName) { + enumDepth += (line.match(/\{/g) ?? []).length - (line.match(/\}/g) ?? []).length; + if (enumDepth <= 0 && line.includes("}")) enumName = null; + } + newLine += 1; + } + } + return out; +} + +/** Extract the member names of a TS enum declaration from file content. Returns null when the enum is not found. */ +export function extractEnumMembers(content: string, enumName: string): Set | null { + const decl = new RegExp(`(?:export\\s+)?(?:declare\\s+)?(?:const\\s+)?enum\\s+${escapeRegExp(enumName)}\\s*\\{`).exec( + content, + ); + if (!decl) return null; + const start = decl.index + decl[0].length; + let depth = 1; + let i = start; + const members = new Set(); + let chunk = ""; + while (i < content.length && depth > 0) { + const ch = content[i]!; + if (ch === "{") depth += 1; + else if (ch === "}") depth -= 1; + if (depth === 1) chunk += ch; + i += 1; + } + for (const part of chunk.split(",")) { + const trimmed = part.trim(); + if (!trimmed || trimmed.startsWith("//")) continue; + const name = /^([A-Za-z_$][\w$]*)/.exec(trimmed); + if (name) members.add(name[1]!); + } + return members.size ? members : null; +} + +/** Extract string-literal members from a `type Name = ...` alias. Returns null when not found or ambiguous. */ +export function extractUnionMembers(content: string, unionName: string): Set | null { + const decl = new RegExp( + `(?:export\\s+)?type\\s+${escapeRegExp(unionName)}\\s*=\\s*([^;]+);`, + "s", + ).exec(content); + if (!decl) return null; + const literals = [...decl[1]!.matchAll(/["']([^"']+)["']/g)].map((m) => m[1]!); + return literals.length ? new Set(literals) : null; +} + +interface SwitchGap { + line: number; +} + +/** Find a switch that covered all `oldMembers` but omits `addedMember`. Skips switches with a default branch. */ +export function findExhaustivenessGap( + content: string, + kind: "enum" | "union", + typeName: string, + oldMembers: Set, + addedMember: string, +): SwitchGap | null { + const lines = content.split("\n"); + for (let i = 0; i < lines.length; i++) { + if (!/^\s*switch\s*\(/.test(lines[i]!)) continue; + const block = extractSwitchBlock(lines, i); + if (!block) continue; + if (block.some((l) => DEFAULT_CASE_RE.test(l))) continue; + const cases = kind === "enum" ? collectEnumCases(block, typeName) : collectUnionCases(block); + if (!oldMembers.size || ![...oldMembers].every((m) => cases.has(m))) continue; + if (cases.has(addedMember)) continue; + return { line: i + 1 }; + } + return null; +} + +function extractSwitchBlock(lines: string[], switchLine: number): string[] | null { + let depth = 0; + let started = false; + const block: string[] = []; + for (let i = switchLine; i < lines.length; i++) { + const line = lines[i]!; + block.push(line); + for (const ch of line) { + if (ch === "{") { + depth += 1; + started = true; + } else if (ch === "}") depth -= 1; + } + if (started && depth === 0) return block; + } + return null; +} + +function collectEnumCases(block: string[], enumName: string): Set { + const cases = new Set(); + const qualified = new RegExp(`case\\s+${escapeRegExp(enumName)}\\.([A-Za-z_$][\\w$]*)\\s*:`); + const bare = /case\s+([A-Za-z_$][\w$]*)\s*:/; + for (const line of block) { + const q = qualified.exec(line); + if (q) cases.add(q[1]!); + else { + const b = bare.exec(line); + if (b) cases.add(b[1]!); + } + } + return cases; +} + +function collectUnionCases(block: string[]): Set { + const cases = new Set(); + const re = /case\s+["']([^"']+)["']\s*:/; + for (const line of block) { + const match = re.exec(line); + if (match) cases.add(match[1]!); + } + return cases; +} + +/** Analyzer entrypoint. Fail-safe — returns no finding on missing token/headSha or fetch errors. */ +export async function scanExhaustivenessDrift( + req: EnrichRequest, + fetchFn: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + const { repoFullName, githubToken, headSha, files = [] } = req; + if (!githubToken || !headSha) return []; + const parts = repoFullName.split("/"); + const [owner, repo] = parts; + if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return []; + + const candidates: AddedMemberCandidate[] = []; + for (const file of files) { + if (!file.patch || !isScannablePath(file.path)) continue; + for (const item of parseAddedTypeMembers(file.patch)) { + candidates.push({ file: file.path, ...item }); + } + } + if (!candidates.length) return []; + + const scannableFiles = files.filter((f) => f.patch && isScannablePath(f.path)).slice(0, MAX_FILES); + const contentCache = new Map(); + let fetches = 0; + + const loadFile = async (path: string, patch?: string): Promise => { + if (contentCache.has(path)) return contentCache.get(path) ?? null; + if (fetches >= MAX_FETCHES) { + contentCache.set(path, null); + return null; + } + fetches += 1; + const content = await fetchFileAtHead(owner, repo, path, headSha, githubToken, fetchFn, options.signal); + contentCache.set(path, content); + return content; + }; + + const findings: ExhaustivenessFinding[] = []; + const seen = new Set(); + + for (const candidate of candidates) { + if (options.signal?.aborted) break; + if (findings.length >= MAX_FINDINGS) break; + if (!scannableFiles.some((f) => f.path === candidate.file)) continue; + + const typeFile = files.find((f) => f.path === candidate.file); + if (!typeFile?.patch) continue; + const headContent = await loadFile(candidate.file, typeFile.patch); + if (!headContent) continue; + const oldContent = reconstructOldContent(headContent, typeFile.patch); + if (!oldContent) continue; + + const extract = candidate.kind === "enum" ? extractEnumMembers : extractUnionMembers; + const oldMembers = extract(oldContent, candidate.unionName); + const newMembers = extract(headContent, candidate.unionName); + if (!oldMembers || !newMembers) continue; + if (!newMembers.has(candidate.addedMember) || oldMembers.has(candidate.addedMember)) continue; + + for (const consumer of scannableFiles) { + const consumerContent = + consumer.path === candidate.file ? headContent : await loadFile(consumer.path, consumer.patch); + if (!consumerContent) continue; + const gap = findExhaustivenessGap( + consumerContent, + candidate.kind, + candidate.unionName, + oldMembers, + candidate.addedMember, + ); + if (!gap) continue; + const key = `${consumer.path}:${gap.line}:${candidate.unionName}:${candidate.addedMember}`; + if (seen.has(key)) continue; + seen.add(key); + findings.push({ + file: candidate.file, + line: candidate.line, + unionName: candidate.unionName, + addedMember: candidate.addedMember, + ...(consumer.path !== candidate.file ? { consumerFile: consumer.path } : {}), + }); + break; + } + } + return findings; +} diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index a9f2b04833..730927b7c9 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -45,6 +45,7 @@ import { scanTodoMarker } from "./todo-marker.js"; import { scanTyposquat } from "./typosquat.js"; import { scanUndocumentedExport } from "./undocumented-export.js"; import { scanUnusedExport } from "./unused-export.js"; +import { scanExhaustivenessDrift } from "./exhaustiveness-drift.js"; import type { AnalyzerDescriptor, AnalyzerFn, @@ -1259,6 +1260,40 @@ export const ANALYZER_DESCRIPTORS = [ run: (req, { signal, analysis, diagnostics }) => scanUnusedExport(req, fetch, { signal, analysis, diagnostics }), }), + descriptor({ + name: "exhaustiveness", + title: "Enum/union exhaustiveness drift", + category: "quality", + cost: "github-light", + defaultEnabled: true, + requires: ["files", "github-token", "head-sha"], + limits: { maxFiles: 10, maxFetches: 10, maxFindings: 25 }, + docs: { + summary: + "Flags when a PR adds a new enum member or string-literal union variant but an exhaustive switch still omits it.", + looksAt: + "Added enum/union members in changed TS/JS files, comparing pre-PR vs headSha member sets and scanning changed files for switches that covered all old members.", + reports: "Type file, line, union/enum name, added member, and optional consumer file — never file contents.", + network: + "Bounded GitHub contents fetches at headSha for changed source files. Requires GitHub token forwarding for private repos.", + notes: + "Conservative: only explicit enum/union case labels; switches with a default branch are skipped. Fail-safe on fetch errors or ambiguous type parsing.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Enum/union exhaustiveness drift (switch missing a newly added member)"]; + for (const item of findings) { + const where = item.consumerFile + ? `${helpers.safeCodeSpan(item.consumerFile)} switch` + : helpers.safeCodeSpan(`${item.file}:${item.line}`); + lines.push( + `- ${where} omits ${helpers.safeCodeSpan(item.unionName)}.${helpers.safeCodeSpan(item.addedMember)} after it was added at ${helpers.safeCodeSpan(`${item.file}:${item.line}`)}`, + ); + } + return lines; + }, + run: (req, { signal }) => scanExhaustivenessDrift(req, fetch, { signal }), + }), descriptor({ name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 4dbb9cff7c..26e1984fb9 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -494,6 +494,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("a11y", findings.a11y)); lines.push(...renderDescriptorSection("i18n", findings.i18n)); lines.push(...renderDescriptorSection("unusedExport", findings.unusedExport)); + lines.push(...renderDescriptorSection("exhaustiveness", findings.exhaustiveness)); lines.push(...renderDescriptorSection("hardcodedUrl", findings.hardcodedUrl)); lines.push(...renderDescriptorSection("commitLint", findings.commitLint)); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 068ed9889e..d7a15419f6 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -368,6 +368,16 @@ export interface UnusedExportFinding { symbol: string; } +/** A TS enum member or string-literal union variant newly ADDED by the PR that a previously exhaustive switch still + * omits. Reports the type name, added member, and optional consumer file — never full file contents. (#2028) */ +export interface ExhaustivenessFinding { + file: string; + line: number; + unionName: string; + addedMember: string; + consumerFile?: string; +} + /** A review/approval integrity signal, read from structured PR-reviews API fields only (state, commit_id, * user.login, submitted_at) — never diff/file content. `stale-approval`: the reviewer's latest APPROVED review * predates the PR's current head commit. `self-approval`: the PR author approved their own PR. @@ -620,6 +630,7 @@ export interface BriefFindings { a11y?: A11yFinding[]; i18n?: I18nFinding[]; unusedExport?: UnusedExportFinding[]; + exhaustiveness?: ExhaustivenessFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; } diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index a4dd685de6..61086f2e39 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -56,6 +56,7 @@ const EXPECTED_ANALYZERS = [ "a11y", "i18n", "unusedExport", + "exhaustiveness", "commitLint", ]; diff --git a/review-enrichment/test/exhaustiveness-drift.test.ts b/review-enrichment/test/exhaustiveness-drift.test.ts new file mode 100644 index 0000000000..759ad54413 --- /dev/null +++ b/review-enrichment/test/exhaustiveness-drift.test.ts @@ -0,0 +1,152 @@ +// Units for the exhaustiveness-drift analyzer (#2028). Own file (not enrichment.test.ts) so concurrent analyzer PRs +// don't collide. All network is mocked. Runs against the compiled dist/. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + parseAddedTypeMembers, + extractEnumMembers, + extractUnionMembers, + findExhaustivenessGap, + scanExhaustivenessDrift, +} from "../dist/analyzers/exhaustiveness-drift.js"; +import { renderBrief } from "../dist/render.js"; + +const req = (files, extra = {}) => ({ + repoFullName: "octo/repo", + prNumber: 1, + githubToken: "ghp_test", + headSha: "abc123", + files, + ...extra, +}); + +const HEAD_UNCOVERED = [ + "export enum Status {", + " Active,", + " Pending,", + " Archived,", + "}", + "", + "export function dispatch(status: Status) {", + " switch (status) {", + " case Status.Active:", + " case Status.Pending:", + " break;", + " }", + "}", +].join("\n"); + +const PATCH_ADD_ARCHIVED = [ + "@@ -1,4 +1,5 @@", + " export enum Status {", + " Active,", + " Pending,", + "+ Archived,", + " }", +].join("\n"); + +test("parseAddedTypeMembers: collects added enum members with line numbers", () => { + assert.deepEqual(parseAddedTypeMembers(PATCH_ADD_ARCHIVED), [ + { unionName: "Status", addedMember: "Archived", line: 4, kind: "enum" }, + ]); +}); + +test("findExhaustivenessGap: flags a switch that covered all old enum members but omits the new one", () => { + const oldMembers = new Set(["Active", "Pending"]); + const gap = findExhaustivenessGap(HEAD_UNCOVERED, "enum", "Status", oldMembers, "Archived"); + assert.deepEqual(gap, { line: 8 }); +}); + +test("findExhaustivenessGap: does not flag when the switch already covers the new member", () => { + const covered = HEAD_UNCOVERED.replace( + " case Status.Pending:", + " case Status.Pending:\n case Status.Archived:", + ); + const oldMembers = new Set(["Active", "Pending"]); + assert.equal(findExhaustivenessGap(covered, "enum", "Status", oldMembers, "Archived"), null); +}); + +test("extractUnionMembers: reads string-literal union members from a type alias", () => { + const src = 'export type Role = "admin" | "user";'; + assert.deepEqual([...extractUnionMembers(src, "Role")!], ["admin", "user"]); +}); + +test("scanExhaustivenessDrift: end-to-end flags an uncovered added enum member and renders it", async () => { + const fetchFn = async (url) => { + if (url.includes("/contents/")) return new Response(HEAD_UNCOVERED, { status: 200 }); + return new Response("", { status: 404 }); + }; + const findings = await scanExhaustivenessDrift( + req([{ path: "src/status.ts", status: "modified", patch: PATCH_ADD_ARCHIVED }]), + fetchFn, + ); + assert.deepEqual(findings, [ + { + file: "src/status.ts", + line: 4, + unionName: "Status", + addedMember: "Archived", + }, + ]); + const brief = renderBrief({ exhaustiveness: findings }).promptSection; + assert.match(brief, /exhaustiveness drift/i); + assert.match(brief, /Archived/); +}); + +test("scanExhaustivenessDrift: does not flag when the switch is updated in the same file", async () => { + const head = HEAD_UNCOVERED.replace( + " case Status.Pending:", + " case Status.Pending:\n case Status.Archived:", + ); + const patch = [ + "@@ -1,4 +1,5 @@", + " export enum Status {", + " Active,", + " Pending,", + "+ Archived,", + " }", + "@@ -10,3 +11,4 @@", + " case Status.Active:", + " case Status.Pending:", + "+ case Status.Archived:", + " break;", + ].join("\n"); + const fetchFn = async (url) => { + if (url.includes("/contents/")) return new Response(head, { status: 200 }); + return new Response("", { status: 404 }); + }; + const findings = await scanExhaustivenessDrift( + req([{ path: "src/status.ts", status: "modified", patch }]), + fetchFn, + ); + assert.deepEqual(findings, []); +}); + +test("scanExhaustivenessDrift: enforces the maxFetches cap", async () => { + const patch = ["@@ -0,0 +1,2 @@", "+export enum E {", "+ A,", "+}"].join("\n"); + const files = Array.from({ length: 12 }, (_, i) => ({ + path: `src/file${i}.ts`, + status: "added", + patch: patch.replace("E", `E${i}`).replace("A", `A${i}`), + })); + let fetches = 0; + const fetchFn = async (url) => { + if (url.includes("/contents/")) { + fetches += 1; + return new Response("export enum E0 { A0 }\n", { status: 200 }); + } + return new Response("", { status: 404 }); + }; + await scanExhaustivenessDrift(req(files), fetchFn); + assert.equal(fetches, 10); +}); + +test("scanExhaustivenessDrift: returns no findings without a GitHub token", async () => { + const findings = await scanExhaustivenessDrift( + req([{ path: "src/status.ts", status: "modified", patch: PATCH_ADD_ARCHIVED }], { + githubToken: undefined, + }), + async () => new Response("", { status: 500 }), + ); + assert.deepEqual(findings, []); +}); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index 27872f692d..9bc86c6cca 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -50,6 +50,7 @@ export const REES_ANALYZER_NAMES = [ "a11y", "i18n", "unusedExport", + "exhaustiveness", "commitLint", ] as const;