diff --git a/.env.example b/.env.example index 460a4492c7..16af5d4766 100644 --- a/.env.example +++ b/.env.example @@ -68,24 +68,25 @@ GITTENSORY_REVIEW_ENRICHMENT=false # nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity # ciCheckSignals,undocumentedExport,staleBranch,commitHygiene,pendingReviewRequests,testRatio # migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover -# commitLint +# unsafeAny,commitLint # # Profile defaults: # fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl # actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio # migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover +# unsafeAny # balanced (default): dependency,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,commitLint +# conflictMarker,debugLeftover,unsafeAny,commitLint # deep: dependency,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,commitLint +# conflictMarker,debugLeftover,unsafeAny,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 30e72ae3f9..13b26e8ec3 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -933,6 +933,28 @@ export const REES_ANALYZERS = [ "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching.", }, }, + { + name: "unsafeAny", + title: "Unsafe `any` usage", + category: "quality", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxLineChars: 2000, + }, + docs: { + summary: + "Counts explicit `any` type annotations, `as any` casts, and `` assertions newly added in TypeScript files.", + looksAt: "Added lines in changed non-test .ts/.tsx/.mts/.cts source files.", + reports: "File, line, and kind: annotation, cast, or assertion.", + network: "Pure local analyzer. No external network call.", + notes: + "Structural regex only — no type-checker. String literals and full-line comments are ignored; inline `//` and `/* */` comments are stripped before matching.", + }, + }, { name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index 5bd5db75a5..ea5f7adad8 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -1052,6 +1052,32 @@ "notes": "Distinct from the secrets-in-logs analyzer: this catches plain debug noise regardless of payload. String literals are stripped before matching." } }, + { + "name": "unsafeAny", + "title": "Unsafe `any` usage", + "category": "quality", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxLineChars": 2000 + }, + "docs": { + "summary": "Counts explicit `any` type annotations, `as any` casts, and `` assertions newly added in TypeScript files.", + "looksAt": "Added lines in changed non-test .ts/.tsx/.mts/.cts source files.", + "reports": "File, line, and kind: annotation, cast, or assertion.", + "network": "Pure local analyzer. No external network call.", + "notes": "Structural regex only — no type-checker. String literals and full-line comments are ignored; inline `//` and `/* */` comments are stripped before matching." + } + }, { "name": "commitLint", "title": "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index f5fcc1ea39..4485ef0b9a 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -36,6 +36,7 @@ import { scanTerminology } from "./terminology.js"; import { scanTodoMarker } from "./todo-marker.js"; import { scanTyposquat } from "./typosquat.js"; import { scanUndocumentedExport } from "./undocumented-export.js"; +import { scanUnsafeAny } from "./unsafe-any.js"; import type { AnalyzerDescriptor, AnalyzerFn, @@ -977,6 +978,35 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanDebugLeftover(req, signal), }), + descriptor({ + name: "unsafeAny", + title: "Unsafe `any` usage", + category: "quality", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxLineChars: 2000 }, + docs: { + summary: + "Counts explicit `any` type annotations, `as any` casts, and `` assertions newly added in TypeScript files.", + looksAt: "Added lines in changed non-test .ts/.tsx/.mts/.cts source files.", + reports: "File, line, and kind: annotation, cast, or assertion.", + network: "Pure local analyzer. No external network call.", + notes: + "Structural regex only — no type-checker. String literals and full-line comments are ignored; inline `//` and `/* */` comments are stripped before matching.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Unsafe `any` usage (type-safety erosion introduced by this PR)"]; + for (const item of findings) { + lines.push( + `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} — ${helpers.safeCodeSpan(item.kind)}`, + ); + } + return lines; + }, + run: (req, { signal }) => scanUnsafeAny(req, signal), + }), descriptor({ name: "commitLint", title: "Conventional-commit subjects", diff --git a/review-enrichment/src/analyzers/unsafe-any.ts b/review-enrichment/src/analyzers/unsafe-any.ts new file mode 100644 index 0000000000..f33eedb2f7 --- /dev/null +++ b/review-enrichment/src/analyzers/unsafe-any.ts @@ -0,0 +1,137 @@ +// Unsafe-`any` counter analyzer (#2017). Flags explicit `any` type annotations, `as any` casts, and `` +// assertions newly introduced in TypeScript diffs — a type-safety erosion signal for the reviewer. Pure compute +// over added lines in .ts/.tsx files, no network. Structural regex only (no type-checker), fail-safe. +import type { EnrichRequest, UnsafeAnyFinding } from "../types.js"; +import { codeOnly } from "./secret-log.js"; +import { isTestPath } from "./test-ratio.js"; + +const MAX_FINDINGS = 25; +const MAX_LINE_CHARS = 2000; + +const TS_EXTS = new Set(["ts", "tsx", "mts", "cts"]); + +const CAST_RE = /\bas\s+any\b/; +const ASSERTION_RE = /<\s*any\s*>/; +const ANNOTATION_RE = /:\s*any\b(?!\s*[,}])/; + +function isTypeScriptPath(path: string): boolean { + const ext = /\.([^.]+)$/.exec(path)?.[1]?.toLowerCase(); + return Boolean(ext && TS_EXTS.has(ext) && !isTestPath(path)); +} + +function stripComments(code: string): string { + const noBlock = code.replace(/\/\*[\s\S]*?\*\//g, " "); + const slash = noBlock.indexOf("//"); + return slash >= 0 ? noBlock.slice(0, slash) : noBlock; +} + +function isFullLineComment(line: string): boolean { + const trimmed = line.trimStart(); + if (trimmed.startsWith("//")) return true; + return /^\s*\/\*[\s\S]*\*\/\s*$/.test(line); +} + +type BlockCommentState = { open: boolean }; + +/** Remove block-comment spans from one added line, tracking multi-line comment state. Pure aside from state. */ +export function stripBlockCommentsFromAddedLine(line: string, state: BlockCommentState): string | null { + let scanLine = line; + if (state.open) { + const end = scanLine.indexOf("*/"); + if (end < 0) return null; + state.open = false; + scanLine = scanLine.slice(end + 2); + } + while (true) { + const start = scanLine.indexOf("/*"); + if (start < 0) break; + const end = scanLine.indexOf("*/", start + 2); + if (end < 0) { + scanLine = scanLine.slice(0, start); + state.open = true; + break; + } + scanLine = `${scanLine.slice(0, start)} ${scanLine.slice(end + 2)}`; + } + return scanLine; +} + +/** Classify one added TS line for an unsafe-`any` pattern, or null. Pure. */ +export function detectUnsafeAny(line: string): UnsafeAnyFinding["kind"] | null { + if (isFullLineComment(line)) return null; + const code = stripComments(codeOnly(line)); + if (CAST_RE.test(code)) return "cast"; + if (ASSERTION_RE.test(code)) return "assertion"; + if (ANNOTATION_RE.test(code)) return "annotation"; + return null; +} + +type ScanLimits = { + maxFindings?: number; + signal?: AbortSignal; +}; + +/** Scan one file patch's added lines for unsafe `any`, line-cited via hunk headers. Pure. */ +export function scanPatchForUnsafeAny( + path: string, + patch: string, + limits: ScanLimits = {}, +): UnsafeAnyFinding[] { + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || !isTypeScriptPath(path)) return []; + const findings: UnsafeAnyFinding[] = []; + let newLine = 0; + let inHunk = false; + const blockComment: BlockCommentState = { open: false }; + for (const line of patch.split("\n")) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (hunk) { + newLine = Number(hunk[1]); + inHunk = true; + blockComment.open = false; + continue; + } + if (!inHunk) continue; + if (line.startsWith("+")) { + const body = line.slice(1); + if (body.length <= MAX_LINE_CHARS) { + const scanLine = stripBlockCommentsFromAddedLine(body, blockComment); + if (scanLine !== null) { + const kind = detectUnsafeAny(scanLine); + if (kind) { + findings.push({ file: path, line: newLine, kind }); + if (findings.length >= maxFindings) return findings; + } + } + } + newLine++; + } else { + blockComment.open = false; + if (!line.startsWith("-") && !line.startsWith("\\")) { + newLine++; + } + } + } + return findings; +} + +/** Analyzer entrypoint: scan every changed TS file's added lines for unsafe `any`. */ +export async function scanUnsafeAny( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: UnsafeAnyFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForUnsafeAny(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index b865adefd9..ca5ed8d3ba 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -481,6 +481,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber)); lines.push(...renderDescriptorSection("conflictMarker", findings.conflictMarker)); lines.push(...renderDescriptorSection("debugLeftover", findings.debugLeftover)); + lines.push(...renderDescriptorSection("unsafeAny", findings.unsafeAny)); 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 3080f0a32f..865bd4ffad 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -473,6 +473,14 @@ export interface DebugLeftoverFinding { kind: "debugger" | "console" | "print"; } +/** Explicit `any` type erosion newly introduced in TypeScript source (#2017, part of #1499). + * Reports location and kind only — never the surrounding code. */ +export interface UnsafeAnyFinding { + file: string; + line: number; + kind: "annotation" | "cast" | "assertion"; +} + /** An absolute HTTP(S) URL or raw IP:port endpoint hardcoded in non-test, non-config source (#2027, part of #1499). * Reports location, kind, and a redacted/truncated host — never full paths or query strings. */ export interface HardcodedUrlFinding { @@ -528,6 +536,7 @@ export interface BriefFindings { magicNumber?: MagicNumberFinding[]; conflictMarker?: ConflictMarkerFinding[]; debugLeftover?: DebugLeftoverFinding[]; + unsafeAny?: UnsafeAnyFinding[]; hardcodedUrl?: HardcodedUrlFinding[]; commitLint?: CommitLintFinding[]; } diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 509f2c8758..4d77eca34e 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -47,6 +47,7 @@ const EXPECTED_ANALYZERS = [ "magicNumber", "conflictMarker", "debugLeftover", + "unsafeAny", "commitLint", ]; diff --git a/review-enrichment/test/unsafe-any.test.ts b/review-enrichment/test/unsafe-any.test.ts new file mode 100644 index 0000000000..8ae5a3b8d2 --- /dev/null +++ b/review-enrichment/test/unsafe-any.test.ts @@ -0,0 +1,105 @@ +// Units for the unsafe-any analyzer (#2017). Own file so concurrent analyzer PRs don't collide. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + detectUnsafeAny, + scanPatchForUnsafeAny, + scanUnsafeAny, +} from "../dist/analyzers/unsafe-any.js"; +import { renderBrief } from "../dist/render.js"; + +const patchOf = (lines: string[]) => + `@@ -1,0 +1,${lines.length} @@\n${lines.map((l) => `+${l}`).join("\n")}`; + +test("detectUnsafeAny: flags annotation, cast, and assertion shapes", () => { + assert.equal(detectUnsafeAny("const x: any = 1;"), "annotation"); + assert.equal(detectUnsafeAny("function f(v: any | null) {}"), "annotation"); + assert.equal(detectUnsafeAny("return payload as any;"), "cast"); + assert.equal(detectUnsafeAny("const rows = data;"), "assertion"); +}); + +test("detectUnsafeAny: prefers cast over annotation on the same line", () => { + assert.equal(detectUnsafeAny("const x: any = value as any;"), "cast"); +}); + +test("detectUnsafeAny: ignores any inside strings and full-line comments", () => { + assert.equal(detectUnsafeAny('const s = ": any"'), null); + assert.equal(detectUnsafeAny('log(`typed as any inside`);'), null); + assert.equal(detectUnsafeAny("// const x: any"), null); + assert.equal(detectUnsafeAny(" // TODO: remove as any"), null); +}); + +test("detectUnsafeAny: ignores inline comments after real code", () => { + assert.equal(detectUnsafeAny("const x: any = 1; // legacy"), "annotation"); +}); + +test("scanPatchForUnsafeAny: flags added lines with correct locations", () => { + const findings = scanPatchForUnsafeAny( + "src/widget.ts", + patchOf(["function load(): any {", " return data as any;", "}"]), + ); + assert.deepEqual(findings, [ + { file: "src/widget.ts", line: 1, kind: "annotation" }, + { file: "src/widget.ts", line: 2, kind: "cast" }, + ]); +}); + +test("detectUnsafeAny: ignores block comments after real code", () => { + assert.equal(detectUnsafeAny("const x = 1; /* as any */"), null); + assert.equal(detectUnsafeAny("/* @internal */ const value: any = input;"), "annotation"); +}); + +test("detectUnsafeAny: does not treat object-literal values as type annotations", () => { + assert.equal(detectUnsafeAny("return { value: any };"), null); + assert.equal(detectUnsafeAny("return { value: any, other: true };"), null); +}); + +test("scanPatchForUnsafeAny: ignores multi-line block comments spanning added lines", () => { + const patch = [ + "@@ -1,0 +1,3 @@", + "+/*", + "+ * const x: any = 1;", + "+ */", + ].join("\n"); + assert.deepEqual(scanPatchForUnsafeAny("src/a.ts", patch), []); +}); + +test("scanPatchForUnsafeAny: accepts .mts and .cts extensions", () => { + assert.deepEqual(scanPatchForUnsafeAny("src/a.mts", patchOf(["const x: any = 1;"])), [ + { file: "src/a.mts", line: 1, kind: "annotation" }, + ]); + assert.deepEqual(scanPatchForUnsafeAny("src/b.cts", patchOf(["return value as any;"])), [ + { file: "src/b.cts", line: 1, kind: "cast" }, + ]); +}); + +test("scanPatchForUnsafeAny: skips non-TS files and test paths", () => { + assert.deepEqual(scanPatchForUnsafeAny("src/widget.js", patchOf(["const x: any = 1;"])), []); + assert.deepEqual( + scanPatchForUnsafeAny("src/widget.test.ts", patchOf(["const x: any = 1;"])), + [], + ); +}); + +test("scanPatchForUnsafeAny: respects the findings cap", () => { + const lines = Array.from({ length: 30 }, () => "const x: any = 1;"); + assert.equal(scanPatchForUnsafeAny("src/a.ts", patchOf(lines), { maxFindings: 3 }).length, 3); +}); + +test("scanUnsafeAny: aggregates across files and renders a public-safe brief", async () => { + const findings = await scanUnsafeAny({ + files: [ + { path: "src/a.ts", patch: patchOf(["const x: any = 1;"]) }, + { path: "lib/b.tsx", patch: patchOf(["return value as any;"]) }, + ], + }); + assert.deepEqual(findings, [ + { file: "src/a.ts", line: 1, kind: "annotation" }, + { file: "lib/b.tsx", line: 1, kind: "cast" }, + ]); + + const { promptSection } = renderBrief({ unsafeAny: findings }); + assert.match(promptSection, /Unsafe `any`/); + assert.match(promptSection, /src\/a\.ts:1/); + assert.match(promptSection, /lib\/b\.tsx:1/); +}); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index f1f7d44de3..3dd55bf347 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -10,6 +10,7 @@ export const REES_ANALYZER_NAMES = [ "license", "installScript", "heavyDependency", + "hardcodedUrl", "actionPin", "eol", "redos", @@ -39,6 +40,8 @@ export const REES_ANALYZER_NAMES = [ "todoMarker", "magicNumber", "conflictMarker", + "debugLeftover", + "unsafeAny", "commitLint", ] as const;