diff --git a/.env.example b/.env.example index c9b3042c9f..9088385604 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,commitLint +# i18n,unusedExport,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,commitLint +# errorSwallow,unsafeAny,a11y,i18n,unusedExport,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,commitLint +# i18n,unusedExport,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 960b4ab37f..69254b823f 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -1117,6 +1117,31 @@ export const REES_ANALYZERS = [ "Inactive when the file's added lines show no t()/useTranslation/FormattedMessage-style convention. Skips technical props and dotted i18n-key-shaped literals.", }, }, + { + name: "unusedExport", + title: "Unused exports (dead-on-arrival)", + category: "quality", + cost: "github-light", + defaultEnabled: true, + profiles: ["balanced", "deep"], + requires: ["files", "github-token", "head-sha"], + limits: { + maxSymbols: 10, + maxSearches: 10, + maxFindings: 25, + }, + docs: { + summary: + "Flags exports newly added by the PR that have zero non-declaration references anywhere in the repo.", + looksAt: + "Direct `export const/let/var/function/class/interface/type/enum` declarations added in changed non-test TS/JS source files, cross-checked via repo-scoped GitHub Code Search.", + reports: "File, line, and symbol name of each dead-on-arrival export — never file contents.", + network: + "One bounded GitHub Code Search query per candidate symbol (capped). Requires headSha and GitHub token forwarding for private repos.", + 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: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index eb783794b9..6cd01c9b18 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1265,6 +1265,34 @@ "notes": "Inactive when the file's added lines show no t()/useTranslation/FormattedMessage-style convention. Skips technical props and dotted i18n-key-shaped literals." } }, + { + "name": "unusedExport", + "title": "Unused exports (dead-on-arrival)", + "category": "quality", + "cost": "github-light", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "requires": [ + "files", + "github-token", + "head-sha" + ], + "limits": { + "maxSymbols": 10, + "maxSearches": 10, + "maxFindings": 25 + }, + "docs": { + "summary": "Flags exports newly added by the PR that have zero non-declaration references anywhere in the repo.", + "looksAt": "Direct `export const/let/var/function/class/interface/type/enum` declarations added in changed non-test TS/JS source files, cross-checked via repo-scoped GitHub Code Search.", + "reports": "File, line, and symbol name of each dead-on-arrival export — never file contents.", + "network": "One bounded GitHub Code Search query per candidate symbol (capped). Requires headSha and GitHub token forwarding for private repos.", + "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": "commitLint", "title": "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index 24bb153e14..a9f2b04833 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -44,6 +44,7 @@ import { scanTerminology } from "./terminology.js"; import { scanTodoMarker } from "./todo-marker.js"; import { scanTyposquat } from "./typosquat.js"; import { scanUndocumentedExport } from "./undocumented-export.js"; +import { scanUnusedExport } from "./unused-export.js"; import type { AnalyzerDescriptor, AnalyzerFn, @@ -1226,6 +1227,38 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanI18nRegression(req, signal), }), + descriptor({ + name: "unusedExport", + title: "Unused exports (dead-on-arrival)", + category: "quality", + cost: "github-light", + defaultEnabled: true, + requires: ["files", "github-token", "head-sha"], + limits: { maxSymbols: 10, maxSearches: 10, maxFindings: 25 }, + docs: { + summary: + "Flags exports newly added by the PR that have zero non-declaration references anywhere in the repo.", + looksAt: + "Direct `export const/let/var/function/class/interface/type/enum` declarations added in changed non-test TS/JS source files, cross-checked via repo-scoped GitHub Code Search.", + reports: "File, line, and symbol name of each dead-on-arrival export — never file contents.", + network: + "One bounded GitHub Code Search query per candidate symbol (capped). Requires headSha and GitHub token forwarding for private repos.", + 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.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Unused exports (new export with no references in the repo)"]; + for (const item of findings) { + lines.push( + `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} exports ${helpers.safeCodeSpan(item.symbol)} with no references found`, + ); + } + return lines; + }, + run: (req, { signal, analysis, diagnostics }) => + scanUnusedExport(req, fetch, { signal, analysis, diagnostics }), + }), descriptor({ name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/unused-export.ts b/review-enrichment/src/analyzers/unused-export.ts new file mode 100644 index 0000000000..d565042064 --- /dev/null +++ b/review-enrichment/src/analyzers/unused-export.ts @@ -0,0 +1,250 @@ +// Unused-export / dead-on-arrival scan (#2025). Flags exports NEWLY ADDED by the PR that have zero non-declaration +// references anywhere in the repo — net-new public surface with no callers yet. Narrow subset of caller-impact (#1509): +// only added direct exports, not changed/removed symbols. Parses added export declarations from the diff, checks the +// declaring file at headSha for same-file references, then resolves external references via repo-scoped GitHub Code +// Search on the default-branch index (injected fetch). A brand-new PR export is usually absent from that index +// (`total_count: 0`), which is treated as dead-on-arrival once same-file uses are ruled out. Bounded symbol, search, +// and file-fetch caps; fail-safe on missing token/headSha, bad slug, search errors, or incomplete results. +import type { + AnalyzerDiagnostics, + EnrichRequest, + UnusedExportFinding, +} from "../types.js"; +import type { AnalysisContext } from "../analysis-context.js"; +import { boundedFetchJson } from "../external-fetch.js"; +import { exportedSymbols, parseAddedExports } from "./undocumented-export.js"; +import { isTestPath } from "./test-ratio.js"; + +const GITHUB_API = "https://api.github.com"; +const GITHUB_API_VERSION = "2022-11-28"; +const SLUG_RE = /^[A-Za-z0-9._-]+$/; +const MAX_SYMBOLS = 10; +const MAX_SEARCHES = 10; +const MAX_FILE_FETCHES = 10; +const MAX_FINDINGS = 25; +const MIN_SYMBOL_LEN = 3; +const MAX_FETCH_BYTES = 1_000_000; +const MAX_SEARCH_JSON_BYTES = 256 * 1024; + +const SOURCE_EXTS = new Set(["ts", "tsx", "js", "jsx", "mjs", "cjs", "mts", "cts"]); +const SKIP_RE = /(?:\.d\.ts$|\.min\.|(?:^|\/)(?:dist|build|vendor)\/)/; + +interface ScanOptions { + signal?: AbortSignal; + analysis?: Pick; + diagnostics?: AnalyzerDiagnostics; +} + +interface CodeSearchItem { + path?: string; +} + +interface CodeSearchResponse { + total_count?: number; + incomplete_results?: boolean; + items?: CodeSearchItem[]; +} + +function githubHeaders(token: string, raw = false): Record { + return { + Authorization: `Bearer ${token}`, + Accept: raw ? "application/vnd.github.raw" : "application/vnd.github+json", + "X-GitHub-Api-Version": GITHUB_API_VERSION, + "User-Agent": "gittensory-review-enrichment", + }; +} + +function escapeRegExp(value: string): string { + return value.replace(/[$.*+?^{}()|[\]\\]/g, "\\$&"); +} + +function isScannablePath(path: string): boolean { + const ext = /\.([^.]+)$/.exec(path)?.[1]?.toLowerCase(); + return Boolean(ext && SOURCE_EXTS.has(ext) && !SKIP_RE.test(path) && !isTestPath(path)); +} + +/** True when `source` references `symbol` on any line other than the export declaration at `declLine` (1-based). */ +export function referencesSymbolInSource( + source: string, + symbol: string, + declLine: number, +): boolean { + const refRe = new RegExp(`(? item.path && item.path !== exportFile)) return false; + return total === 1; +} + +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, + fetchImpl: typeof fetch, + signal: AbortSignal | undefined, +): Promise { + try { + const encoded = path.split("/").map(encodeURIComponent).join("/"); + const resp = await fetchImpl( + `${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${encoded}?ref=${encodeURIComponent(headSha)}`, + { headers: githubHeaders(token, true), signal }, + ); + if (!resp.ok) return null; + return await readBoundedText(resp, signal); + } catch { + return null; + } +} + +async function searchSymbolReferences( + owner: string, + repo: string, + symbol: string, + token: string, + fetchImpl: typeof fetch, + options: ScanOptions, +): Promise { + const q = `"${symbol}" repo:${owner}/${repo}`; + const url = `${GITHUB_API}/search/code?q=${encodeURIComponent(q)}&per_page=100`; + const fetchOptions = { + endpointCategory: "github-code-search", + headers: githubHeaders(token), + signal: options.signal, + fetchImpl, + diagnostics: options.diagnostics, + phase: "unused-export", + subcall: "code-search", + maxBytes: MAX_SEARCH_JSON_BYTES, + maxCallsPerCategory: MAX_SEARCHES, + }; + const response = options.analysis + ? await options.analysis.fetchJson(url, fetchOptions) + : await boundedFetchJson(url, fetchOptions); + return response.ok ? response.data : null; +} + +/** Analyzer entrypoint: parse added direct exports from changed source files and flag symbols with no non-declaration + * references. Fail-safe — returns no finding on missing token/headSha or search/fetch errors. */ +export async function scanUnusedExport( + 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: Array<{ file: string; symbol: string; line: number }> = []; + for (const file of files) { + if (!file.patch || !isScannablePath(file.path)) continue; + for (const { symbol, newLine } of parseAddedExports(file.patch)) { + if (symbol.length < MIN_SYMBOL_LEN) continue; + candidates.push({ file: file.path, symbol, line: newLine }); + if (candidates.length >= MAX_SYMBOLS) break; + } + if (candidates.length >= MAX_SYMBOLS) break; + } + if (!candidates.length) return []; + + const fileCache = new Map(); + let fileFetches = 0; + const loadFile = async (path: string): Promise => { + if (fileCache.has(path)) return fileCache.get(path) ?? null; + if (fileFetches >= MAX_FILE_FETCHES) { + fileCache.set(path, null); + return null; + } + fileFetches += 1; + const content = await fetchFileAtHead(owner, repo, path, headSha, githubToken, fetchFn, options.signal); + fileCache.set(path, content); + return content; + }; + + const findings: UnusedExportFinding[] = []; + let searches = 0; + for (const candidate of candidates) { + if (options.signal?.aborted) break; + if (searches >= MAX_SEARCHES) break; + + const content = await loadFile(candidate.file); + if (content) { + const idx = candidate.line - 1; + const line = content.split("\n")[idx]; + if (line !== undefined && !exportedSymbols(line).includes(candidate.symbol)) continue; + if (referencesSymbolInSource(content, candidate.symbol, candidate.line)) continue; + } + + let response: CodeSearchResponse | null = null; + try { + response = await searchSymbolReferences( + owner, + repo, + candidate.symbol, + githubToken, + fetchFn, + options, + ); + } catch { + response = null; + } + searches += 1; + if (response === null) continue; + + const dead = isDeadOnArrivalFromSearch(candidate.file, response); + if (dead !== true) continue; + findings.push({ + file: candidate.file, + line: candidate.line, + symbol: candidate.symbol, + }); + if (findings.length >= MAX_FINDINGS) break; + } + return findings; +} diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 63b97b26b5..4dbb9cff7c 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -493,6 +493,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("unsafeAny", findings.unsafeAny)); lines.push(...renderDescriptorSection("a11y", findings.a11y)); lines.push(...renderDescriptorSection("i18n", findings.i18n)); + lines.push(...renderDescriptorSection("unusedExport", findings.unusedExport)); 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 1fb604edf1..068ed9889e 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -360,6 +360,14 @@ export interface UndocumentedExportFinding { symbol: string; } +/** An export newly ADDED by the PR with zero non-declaration references anywhere in the repo (dead-on-arrival). + * Reports the symbol + its line only, never file contents. (#2025) */ +export interface UnusedExportFinding { + file: string; + line: number; + symbol: 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. @@ -611,6 +619,7 @@ export interface BriefFindings { unsafeAny?: UnsafeAnyFinding[]; a11y?: A11yFinding[]; i18n?: I18nFinding[]; + unusedExport?: UnusedExportFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; } diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 133ab89d9a..a4dd685de6 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -55,6 +55,7 @@ const EXPECTED_ANALYZERS = [ "unsafeAny", "a11y", "i18n", + "unusedExport", "commitLint", ]; diff --git a/review-enrichment/test/unused-export.test.ts b/review-enrichment/test/unused-export.test.ts new file mode 100644 index 0000000000..636f6871b9 --- /dev/null +++ b/review-enrichment/test/unused-export.test.ts @@ -0,0 +1,137 @@ +// Units for the unused-export analyzer (#2025). 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 { + isDeadOnArrivalFromSearch, + referencesSymbolInSource, + scanUnusedExport, +} from "../dist/analyzers/unused-export.js"; +import { renderBrief } from "../dist/render.js"; + +const searchJson = (total, items, incomplete = false) => + JSON.stringify({ total_count: total, incomplete_results: incomplete, items }); + +const req = (files, extra = {}) => ({ + repoFullName: "octo/repo", + prNumber: 1, + githubToken: "ghp_test", + headSha: "abc123", + files, + ...extra, +}); + +test("isDeadOnArrivalFromSearch: zero indexed hits is dead; external or multiple hits are alive", () => { + assert.equal(isDeadOnArrivalFromSearch("src/util.ts", { total_count: 0, items: [] }), true); + assert.equal( + isDeadOnArrivalFromSearch("src/util.ts", { + total_count: 1, + items: [{ path: "src/util.ts" }], + }), + true, + ); + assert.equal( + isDeadOnArrivalFromSearch("src/util.ts", { + total_count: 2, + items: [{ path: "src/util.ts" }, { path: "src/app.ts" }], + }), + false, + ); + assert.equal( + isDeadOnArrivalFromSearch("src/util.ts", { total_count: 1, incomplete_results: true, items: [] }), + null, + ); +}); + +test("referencesSymbolInSource: ignores the declaration line but catches same-file uses", () => { + const src = ["export function helper() {}", "helper();", "export const other = 1;"].join("\n"); + assert.equal(referencesSymbolInSource(src, "helper", 1), true); + assert.equal(referencesSymbolInSource(src, "other", 3), false); +}); + +test("scanUnusedExport: flags a newly added export absent from the default-branch index", async () => { + const patch = ["@@ -0,0 +1,1 @@", "+export function orphanHelper() {}"].join("\n"); + const head = "export function orphanHelper() {}"; + const fetchFn = async (url) => { + if (url.includes("/contents/")) return new Response(head, { status: 200 }); + if (url.includes("/search/code")) { + return new Response(searchJson(0, []), { status: 200 }); + } + return new Response("", { status: 404 }); + }; + const findings = await scanUnusedExport( + req([{ path: "src/util.ts", status: "added", patch }]), + fetchFn, + ); + assert.deepEqual(findings, [{ file: "src/util.ts", line: 1, symbol: "orphanHelper" }]); + const brief = renderBrief({ unusedExport: findings }).promptSection; + assert.match(brief, /Unused exports/i); + assert.match(brief, /orphanHelper/); +}); + +test("scanUnusedExport: does not flag when search finds a reference in another file", async () => { + const patch = ["@@ -0,0 +1,1 @@", "+export const shared = 1;"].join("\n"); + const fetchFn = async (url) => { + if (url.includes("/contents/")) return new Response("export const shared = 1;", { status: 200 }); + if (url.includes("/search/code")) { + return new Response( + searchJson(2, [{ path: "src/util.ts" }, { path: "src/app.ts" }]), + { status: 200 }, + ); + } + return new Response("", { status: 404 }); + }; + const findings = await scanUnusedExport( + req([{ path: "src/util.ts", status: "added", patch }]), + fetchFn, + ); + assert.deepEqual(findings, []); +}); + +test("scanUnusedExport: does not flag when the head file uses the export locally", async () => { + const patch = ["@@ -0,0 +1,2 @@", "+export function helper() {}", "+helper();"].join("\n"); + const head = "export function helper() {}\nhelper();"; + const fetchFn = async (url) => { + if (url.includes("/contents/")) return new Response(head, { status: 200 }); + if (url.includes("/search/code")) return new Response(searchJson(0, []), { status: 200 }); + return new Response("", { status: 404 }); + }; + const findings = await scanUnusedExport( + req([{ path: "src/util.ts", status: "added", patch }]), + fetchFn, + ); + assert.deepEqual(findings, []); +}); + +test("scanUnusedExport: enforces the maxSearches cap", async () => { + const patch = ["@@ -0,0 +1,1 @@", "+export function fn() {}"].join("\n"); + const files = Array.from({ length: 12 }, (_, i) => ({ + path: `src/file${i}.ts`, + status: "added", + patch: patch.replace("fn", `fn${i}`), + })); + let searches = 0; + const fetchFn = async (url) => { + if (url.includes("/contents/")) { + const match = /file(\d+)\.ts/.exec(url); + const idx = match ? match[1] : "0"; + return new Response(`export function fn${idx}() {}`, { status: 200 }); + } + if (url.includes("/search/code")) { + searches += 1; + return new Response(searchJson(0, []), { status: 200 }); + } + return new Response("", { status: 404 }); + }; + await scanUnusedExport(req(files), fetchFn); + assert.equal(searches, 10); +}); + +test("scanUnusedExport: returns no findings without a GitHub token", async () => { + const patch = ["@@ -0,0 +1,1 @@", "+export function lonely() {}"].join("\n"); + const findings = await scanUnusedExport( + req([{ path: "src/util.ts", status: "added", patch }], { 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 8c7113c29e..27872f692d 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -49,6 +49,7 @@ export const REES_ANALYZER_NAMES = [ "unsafeAny", "a11y", "i18n", + "unusedExport", "commitLint", ] as const;