diff --git a/.env.example b/.env.example index 0b3f2fa660..115b0b9d8d 100644 --- a/.env.example +++ b/.env.example @@ -67,22 +67,22 @@ GITTENSORY_REVIEW_ENRICHMENT=false # provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig,nativeBuild # history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity,ciCheckSignals # undocumentedExport,staleBranch,commitHygiene,pendingReviewRequests,testRatio,migrationSafety -# looseRange,terminology,todoMarker +# looseRange,terminology,todoMarker,magicNumber # # Profile defaults: # fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol # redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio,migrationSafety -# looseRange,terminology,todoMarker +# looseRange,terminology,todoMarker,magicNumber # balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency # 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 +# pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber # deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,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 +# migrationSafety,looseRange,terminology,todoMarker,magicNumber # 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 e987e59a3a..d622de7867 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -842,6 +842,29 @@ export const REES_ANALYZERS = [ "Precision-first: only UPPERCASE, comment-anchored markers are reported (a lowercase `todo` identifier or a marker inside a string literal is never flagged); a bare marker inside a multi-line block comment is intentionally not matched.", }, }, + { + name: "magicNumber", + title: "Magic numbers", + category: "quality", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxLineChars: 2000, + }, + docs: { + summary: + "Flags newly-added non-trivial numeric literals in non-test source where a named constant would clarify intent.", + looksAt: + "Added lines in source files, excluding tests, strings, comments, trivial sentinels/scales, named constants, array indexes, and enum-like initializers.", + reports: "File, line, and numeric literal text only.", + network: "Pure local analyzer. No external network call.", + notes: + "Precision-first: common values such as 0, 1, -1, 2, 100, 1000, and powers of ten are silent.", + }, + }, ] as const satisfies readonly ReesAnalyzerDoc[]; export const REES_ANALYZER_NAMES = REES_ANALYZERS.map((analyzer) => analyzer.name); diff --git a/review-enrichment/README.md b/review-enrichment/README.md index 88144d6d67..415fb409dc 100644 --- a/review-enrichment/README.md +++ b/review-enrichment/README.md @@ -49,6 +49,7 @@ inside the operator's trust boundary. The engine prefers a short-lived installat | `iacMisconfig` | Risky IaC/config changes like public buckets, open ingress, or insecure CORS. | Pure local. | | `nativeBuild` | Newly-added dependencies that compile native code or ship sdist-only builds. | Calls npm/PyPI registries. | | `history` | Author track record, same-file PR history, and linked-issue alignment. | Calls GitHub API with bounded fanout; needs author/token for private repos. | +| `magicNumber` | Non-trivial numeric literals newly added in non-test source. | Pure local. | The engine can send `analyzers: ["secret", "actionPin"]` to run a subset. If the field is omitted, REES runs the full registry. An explicit empty array runs no analyzers; the engine uses that fail-closed shape when an @@ -76,6 +77,69 @@ classes, per-analyzer limits, and self-host configuration. When adding or migrat - Make external-call analyzers fail open and respect the orchestrator abort signal when the scanner supports it. - Prefer a focused analyzer test file instead of expanding the shared `enrichment.test.ts` mega-test. +### Magic-number analyzer + +`magicNumber` is a precision-first local analyzer for unexplained numeric literals added by a PR. It is intended to +surface values that look like policy, timing, sizing, retry, threshold, or scoring decisions hidden directly inside an +expression, where a named constant would make intent and future review safer. + +The analyzer scans only added diff lines in non-test source files. It never fetches repository content, never +evaluates code, and never returns source snippets. Findings carry only `{ file, line, value }`, so the review brief can +say that `src/retry.ts:42` added `37` without copying the surrounding line. + +What it reports: + +- Numeric literals in expressions, such as `attempt * 37`, `timeout + 250`, `ratio > 0.73`, or `0xff` masks. +- Signed and fractional forms when they are part of the literal, such as `-42`, `.75`, `6e-3`, or `99n`. +- Multiple reportable values on one added line, capped by the analyzer-level finding limit. +- Added content whose source text begins with plus signs, matching the unified-diff edge cases covered by sibling + analyzers. + +What it suppresses: + +- Test files and snapshot paths, because assertion literals are usually expected examples rather than production + policy. +- Documentation, JSON/YAML, lockfiles, fixtures, and other non-source files. +- String literals and inline comments before numeric scanning, so prose like `"wait 37 seconds"` or `// retry in 42` + does not generate a finding. +- Common sentinel and scale values: `0`, `1`, `-1`, `2`, `100`, `1000`, and powers of ten. +- Named constant declarations, including common language forms such as `const MAX_BATCH = 250`, `static readonly + RETRY_WINDOW = 30`, `public static final int LIMIT = 50`, `final DEFAULT_LIMIT = 50`, and `val MAX_PAGE_SIZE = 250`. +- Array indexes like `rows[3]`, numeric object keys like `{ 404: handler }`, and enum-like member initializers like + `PENDING = 3`. + +The goal is not to ban numeric literals. It is to highlight newly-added non-obvious values that can silently encode +review-critical behavior. The analyzer favors false negatives over noisy findings: if a literal looks named, +structural, test-only, or conventional, it stays silent. + +Example outcomes: + +| Added source line | Analyzer result | Rationale | +| ----------------- | --------------- | --------- | +| `const timeoutMs = attempts * 37;` | Reports `37`. | The value is embedded in behavior and is not self-describing. | +| `const RETRY_WINDOW_MS = 37;` | Suppressed. | The uppercase declaration gives the literal a reviewable name. | +| `if (ratio > 0.73) return true;` | Reports `0.73`. | Fractional thresholds are usually policy choices. | +| `return rows[3];` | Suppressed. | Small positional array indexes are structural. | +| `return items[:37];` | Reports `37`. | Slice bounds can encode a batch or display limit. | +| `{ 404: handleMissing }` | Suppressed. | Numeric object keys are commonly protocol or lookup labels. | +| `enum State { Pending = 3 }` | Suppressed. | Enum-like member initializers are named states. | +| `const mask = flags & 0xff;` | Reports `0xff`. | Radix literals can hide bitmask decisions. | +| `const sample = 1_337;` | Reports `1_337`. | Numeric separators keep the original literal readable in findings. | +| `const scale = 1000;` | Suppressed. | Powers and common scales are intentionally quiet. | + +Operational notes: + +- Keep findings public-safe: report the file, line, and literal only, never the surrounding source text. +- Use the diff hunk line number, not a best-effort grep against the repository checkout. +- Scan added lines only. Removed or context lines should never create findings. +- Apply the source-path filter before scanning content so generated metadata and docs stay quiet. +- Respect the abort signal both before and during patch scanning. +- Keep line-level work bounded; very long added lines are skipped to avoid pathological input. +- Preserve deterministic ordering by scanning files, hunks, and literals in diff order. +- Cap per-line and per-request findings so one generated file cannot dominate the brief. +- Add tests for both the reported and suppressed side of every new heuristic. +- Regenerate analyzer metadata whenever the registry descriptor changes. + ## Shared analysis context Each `/v1/enrich` request now gets a request-scoped `AnalysisContext` before analyzers run. New and migrated diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index f9ac48fd8c..fd2fc254ce 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -947,6 +947,32 @@ "network": "Pure local analyzer. No external network call.", "notes": "Precision-first: only UPPERCASE, comment-anchored markers are reported (a lowercase `todo` identifier or a marker inside a string literal is never flagged); a bare marker inside a multi-line block comment is intentionally not matched." } + }, + { + "name": "magicNumber", + "title": "Magic numbers", + "category": "quality", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxLineChars": 2000 + }, + "docs": { + "summary": "Flags newly-added non-trivial numeric literals in non-test source where a named constant would clarify intent.", + "looksAt": "Added lines in source files, excluding tests, strings, comments, trivial sentinels/scales, named constants, array indexes, and enum-like initializers.", + "reports": "File, line, and numeric literal text only.", + "network": "Pure local analyzer. No external network call.", + "notes": "Precision-first: common values such as 0, 1, -1, 2, 100, 1000, and powers of ten are silent." + } } ] } diff --git a/review-enrichment/src/analyzers/magic-number.ts b/review-enrichment/src/analyzers/magic-number.ts new file mode 100644 index 0000000000..734133c0d1 --- /dev/null +++ b/review-enrichment/src/analyzers/magic-number.ts @@ -0,0 +1,308 @@ +// Magic-number analyzer (#2018). Flags newly-added numeric literals in non-test source when a named constant would +// make the intent clearer. Pure local compute over added diff lines: no network, no checkout, no cross-file state. +// Precision-first: common sentinels/scales are allowlisted, const NAME = declarations are treated as already +// named, array indexes and enum/member initializers are suppressed, and string/comment content is blanked first. +import type { EnrichRequest, MagicNumberFinding } 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 REPORT_CHARS = 40; + +const SOURCE_EXTS = new Set([ + "ts", + "tsx", + "mts", + "cts", + "js", + "jsx", + "mjs", + "cjs", + "py", + "go", + "rb", + "dart", + "java", + "kt", + "kts", + "scala", + "groovy", + "cs", + "swift", + "php", + "rs", + "c", + "cc", + "cpp", + "h", + "hpp", +]); + +const NAMED_CONST_RE = + /^\s*(?:export\s+)?(?:(?:const|let|var|readonly|final|val)\s+|static\s+(?:readonly\s+)?|public\s+static\s+final\s+\w+\s+|private\s+static\s+final\s+\w+\s+)?[A-Z][A-Z0-9_]{1,}\s*[:=]/; +const ENUM_MEMBER_RE = /^\s*[A-Z][A-Za-z0-9_]*\s*=\s*[-+]?(?:0[xob])?[0-9]/; +const NUMERIC_SEPARATOR_RE = /_/g; + +type ScanLimits = { + maxFindings?: number; + signal?: AbortSignal; +}; + +type NumericToken = { + value: string; + start: number; + end: number; +}; + +function sourceExtOf(path: string): string | null { + const match = /\.([A-Za-z0-9]+)$/.exec(path); + return match ? match[1]!.toLowerCase() : null; +} + +/** Whether a file can contain source literals this analyzer should judge. Test paths are intentionally silent. */ +export function isMagicNumberSourcePath(path: string): boolean { + const ext = sourceExtOf(path); + return Boolean(ext && SOURCE_EXTS.has(ext) && !isTestPath(path)); +} + +function previousCodeChar(line: string, index: number): string | null { + for (let i = index - 1; i >= 0; i--) { + const ch = line[i]!; + if (ch !== " " && ch !== "\t") return ch; + } + return null; +} + +function nextCodeChar(line: string, index: number): string | null { + for (let i = index; i < line.length; i++) { + const ch = line[i]!; + if (ch !== " " && ch !== "\t") return ch; + } + return null; +} + +function isIdentifierChar(ch: string | null): boolean { + return Boolean(ch && /[A-Za-z0-9_$]/.test(ch)); +} + +function isDigit(ch: string | undefined): boolean { + return Boolean(ch && ch >= "0" && ch <= "9"); +} + +function isSignPartOfNumber(line: string, i: number): boolean { + const ch = line[i]; + if (ch !== "-" && ch !== "+") return false; + if (!isDigit(line[i + 1]) && line[i + 1] !== ".") return false; + if (i === 0 || line[i - 1] === " " || line[i - 1] === "\t") return true; + const prev = previousCodeChar(line, i); + return !prev || "([{:;,=+-*/%!<>?&|^~".includes(prev); +} + +function readDigits(line: string, i: number, radix: "binary" | "octal" | "decimal" | "hex"): number { + const re = + radix === "binary" + ? /[01_]/ + : radix === "octal" + ? /[0-7_]/ + : radix === "hex" + ? /[0-9A-Fa-f_]/ + : /[0-9_]/; + while (i < line.length && re.test(line[i]!)) i++; + return i; +} + +/** Extract numeric tokens with source spans from one code-only line. Skips property suffixes and identifiers. */ +export function extractNumericTokens(line: string): NumericToken[] { + const tokens: NumericToken[] = []; + let i = 0; + while (i < line.length) { + const start = i; + let sign = ""; + if (isSignPartOfNumber(line, i)) { + sign = line[i]!; + i++; + } + const numberStart = i; + const prev = start > 0 ? line[start - 1]! : null; + if (isIdentifierChar(prev) || prev === ".") { + i = Math.max(start + 1, i); + continue; + } + + if (line[i] === "0" && /[xX]/.test(line[i + 1] ?? "")) { + i += 2; + const digitsStart = i; + i = readDigits(line, i, "hex"); + if (i === digitsStart) continue; + } else if (line[i] === "0" && /[bB]/.test(line[i + 1] ?? "")) { + i += 2; + const digitsStart = i; + i = readDigits(line, i, "binary"); + if (i === digitsStart) continue; + } else if (line[i] === "0" && /[oO]/.test(line[i + 1] ?? "")) { + i += 2; + const digitsStart = i; + i = readDigits(line, i, "octal"); + if (i === digitsStart) continue; + } else { + if (line[i] === ".") { + if (!isDigit(line[i + 1])) { + i = start + 1; + continue; + } + i++; + i = readDigits(line, i, "decimal"); + } else if (isDigit(line[i])) { + i = readDigits(line, i, "decimal"); + if (line[i] === "." && isDigit(line[i + 1])) { + i++; + i = readDigits(line, i, "decimal"); + } + } else { + i = start + 1; + continue; + } + if (/[eE]/.test(line[i] ?? "")) { + const expStart = i; + let j = i + 1; + if (line[j] === "+" || line[j] === "-") j++; + const digitsStart = j; + j = readDigits(line, j, "decimal"); + if (j > digitsStart) i = j; + else i = expStart; + } + } + + if (/[nN]/.test(line[i] ?? "")) i++; + const next = line[i] ?? null; + if (isIdentifierChar(next)) continue; + const raw = `${sign}${line.slice(numberStart, i)}`; + tokens.push({ value: raw, start, end: i }); + } + return tokens; +} + +function numericMagnitude(value: string): number | null { + const unsigned = value.replace(NUMERIC_SEPARATOR_RE, "").replace(/^[+-]/, "").replace(/n$/i, ""); + if (/^0[xX][0-9A-Fa-f]+$/.test(unsigned)) return Number.parseInt(unsigned.slice(2), 16); + if (/^0[bB][01]+$/.test(unsigned)) return Number.parseInt(unsigned.slice(2), 2); + if (/^0[oO][0-7]+$/.test(unsigned)) return Number.parseInt(unsigned.slice(2), 8); + const parsed = Number(unsigned); + return Number.isFinite(parsed) ? Math.abs(parsed) : null; +} + +function isPowerOfTen(value: number): boolean { + if (!Number.isInteger(value) || value < 10) return false; + while (value > 1 && value % 10 === 0) value /= 10; + return value === 1; +} + +/** Allowlist trivial/sentinel/scaling values that are more noise than signal for this analyzer. */ +export function isAllowedMagicNumberValue(value: string): boolean { + const magnitude = numericMagnitude(value); + if (magnitude === null) return true; + if (magnitude === 0 || magnitude === 1 || magnitude === 2) return true; + if (magnitude === 100 || magnitude === 1000) return true; + if (isPowerOfTen(magnitude)) return true; + return false; +} + +function isArrayIndex(line: string, token: NumericToken): boolean { + return previousCodeChar(line, token.start) === "[" && nextCodeChar(line, token.end) === "]"; +} + +function isLikelyEnumInitializer(line: string, token: NumericToken): boolean { + const before = line.slice(0, token.start); + return ENUM_MEMBER_RE.test(line) || /^[\s,]*[A-Z][A-Za-z0-9_]*\s*=\s*$/.test(before); +} + +function isNamedConstantDeclaration(line: string): boolean { + return NAMED_CONST_RE.test(line); +} + +function isNumericObjectKey(line: string, token: NumericToken): boolean { + return nextCodeChar(line, token.end) === ":" && ["{", ","].includes(previousCodeChar(line, token.start) ?? ""); +} + +function reportValue(value: string): string { + return value.length > REPORT_CHARS ? value.slice(0, REPORT_CHARS) : value; +} + +function stripInlineComments(line: string): string { + const slash = line.indexOf("//"); + const block = line.indexOf("/*"); + const hash = /(^|\s)#/.exec(line); + const cuts = [slash, block, hash?.index].filter((value): value is number => value !== undefined && value >= 0); + return cuts.length ? line.slice(0, Math.min(...cuts)) : line; +} + +/** Detect reportable numeric literals on one added source line after stripping strings/comments. */ +export function detectMagicNumbers(line: string): Array<{ value: string }> { + if (line.length > MAX_LINE_CHARS) return []; + const code = stripInlineComments(codeOnly(line)); + if (isNamedConstantDeclaration(code)) return []; + const findings: Array<{ value: string }> = []; + for (const token of extractNumericTokens(code)) { + if (isAllowedMagicNumberValue(token.value)) continue; + if (isArrayIndex(code, token)) continue; + if (isLikelyEnumInitializer(code, token)) continue; + if (isNumericObjectKey(code, token)) continue; + findings.push({ value: reportValue(token.value) }); + } + return findings; +} + +/** Scan one file patch's added lines for unexplained numeric literals, line-cited via hunk headers. Pure. */ +export function scanPatchForMagicNumbers( + path: string, + patch?: string, + limits: ScanLimits = {}, +): MagicNumberFinding[] { + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0 || !patch || !isMagicNumberSourcePath(path)) return []; + const findings: MagicNumberFinding[] = []; + let newLine = 0; + let inHunk = 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; + continue; + } + if (!inHunk) continue; + if (line.startsWith("+")) { + for (const hit of detectMagicNumbers(line.slice(1))) { + findings.push({ file: path, line: newLine, value: hit.value }); + if (findings.length >= maxFindings) return findings; + } + newLine++; + } else if (!line.startsWith("-") && !line.startsWith("\\")) { + // A `\ No newline at end of file` marker is not a new-file line -- do not advance the cursor. + newLine++; + } + } + return findings; +} + +/** Analyzer entrypoint: scan every changed non-test source file's added lines for reportable magic numbers. */ +export async function scanMagicNumbers( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: MagicNumberFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch) continue; + for (const finding of scanPatchForMagicNumbers(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/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index f8725729fe..516784b8f3 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -27,6 +27,7 @@ import { scanStaleBranch } from "./stale-branch.js"; import { scanTestRatio } from "./test-ratio.js"; import { scanMigrationSafety } from "./migration-safety.js"; import { scanLooseRanges } from "./loose-range.js"; +import { scanMagicNumbers } from "./magic-number.js"; import { scanTerminology } from "./terminology.js"; import { scanTodoMarker } from "./todo-marker.js"; import { scanTyposquat } from "./typosquat.js"; @@ -855,6 +856,36 @@ export const ANALYZER_DESCRIPTORS = [ }, run: (req, { signal }) => scanTodoMarker(req, signal), }), + descriptor({ + name: "magicNumber", + title: "Magic numbers", + category: "quality", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxLineChars: 2000 }, + docs: { + summary: + "Flags newly-added non-trivial numeric literals in non-test source where a named constant would clarify intent.", + looksAt: + "Added lines in source files, excluding tests, strings, comments, trivial sentinels/scales, named constants, array indexes, and enum-like initializers.", + reports: "File, line, and numeric literal text only.", + network: "Pure local analyzer. No external network call.", + notes: + "Precision-first: common values such as 0, 1, -1, 2, 100, 1000, and powers of ten are silent.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Magic numbers (new unexplained numeric literals)"]; + for (const item of findings) { + lines.push( + `- ${helpers.safeCodeSpan(`${item.file}:${item.line}`)} adds ${helpers.safeCodeSpan(item.value)}; consider naming the intent with a constant`, + ); + } + return lines; + }, + run: (req, { signal }) => scanMagicNumbers(req, signal), + }), ] as const satisfies readonly AnyAnalyzerDescriptor[]; export const ANALYZER_NAMES = ANALYZER_DESCRIPTORS.map( diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 3173f73aad..df05642f19 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -458,6 +458,7 @@ export function renderBrief( lines.push(...renderDescriptorSection("looseRange", findings.looseRange)); lines.push(...renderDescriptorSection("terminology", findings.terminology)); lines.push(...renderDescriptorSection("todoMarker", findings.todoMarker)); + lines.push(...renderDescriptorSection("magicNumber", findings.magicNumber)); if (!lines.length) return { promptSection: "", systemSuffix: "" }; diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 34a082923b..175aecb10c 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -438,6 +438,15 @@ export interface TodoMarkerFinding { note?: string; } +/** An unexplained numeric literal newly added in non-test source where a named constant would usually make intent + * clearer (#2018, part of #1499). Precision-first: common sentinels/scales, named constants, array indexes, and + * enum/member initializers are suppressed. Reports only the location and literal text. */ +export interface MagicNumberFinding { + file: string; + line: number; + value: string; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; @@ -473,6 +482,7 @@ export interface BriefFindings { looseRange?: LooseRangeFinding[]; terminology?: TerminologyFinding[]; todoMarker?: TodoMarkerFinding[]; + magicNumber?: MagicNumberFinding[]; } /** A JSDoc/TSDoc block whose `@param` tags name parameters the adjacent function no longer declares — a diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 56dc8d5b6b..0b90e20549 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -43,6 +43,7 @@ const EXPECTED_ANALYZERS = [ "looseRange", "terminology", "todoMarker", + "magicNumber", ]; test("analyzer descriptors cover the runtime registry in stable order", () => { diff --git a/review-enrichment/test/magic-number.test.ts b/review-enrichment/test/magic-number.test.ts new file mode 100644 index 0000000000..3e6123d86d --- /dev/null +++ b/review-enrichment/test/magic-number.test.ts @@ -0,0 +1,546 @@ +// Units for the magic-number analyzer (#2018). Own file so concurrent analyzer PRs do not collide. Pure local +// scanner: no network, no checkout, and every assertion runs against the compiled dist output. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + detectMagicNumbers, + extractNumericTokens, + isAllowedMagicNumberValue, + isMagicNumberSourcePath, + scanMagicNumbers, + scanPatchForMagicNumbers, +} from "../dist/analyzers/magic-number.js"; +import { buildBrief } from "../dist/brief.js"; +import { renderBrief } from "../dist/render.js"; + +const patchOf = (lines) => `@@ -1,0 +1,${lines.length} @@\n${lines.map((line) => `+${line}`).join("\n")}`; + +test("isMagicNumberSourcePath: accepts non-test source extensions used by REES analyzers", () => { + for (const path of [ + "src/app.ts", + "src/app.tsx", + "src/app.mts", + "src/app.cts", + "src/app.js", + "src/app.jsx", + "src/app.mjs", + "src/app.cjs", + "pkg/app.py", + "pkg/app.go", + "pkg/app.rb", + "lib/app.dart", + "src/App.java", + "src/App.kt", + "src/App.kts", + "src/App.scala", + "src/App.groovy", + "src/App.cs", + "src/App.swift", + "src/App.php", + "src/lib.rs", + "src/app.c", + "src/app.cc", + "src/app.cpp", + "src/app.h", + "src/app.hpp", + ]) { + assert.equal(isMagicNumberSourcePath(path), true, path); + } +}); + +test("isMagicNumberSourcePath: skips tests, snapshots, docs, config, and generated data files", () => { + for (const path of [ + "test/app.ts", + "tests/app.py", + "spec/app.rb", + "src/__tests__/app.js", + "src/app.test.ts", + "src/app.spec.js", + "pkg/app_test.go", + "pkg/test_app.py", + "lib/app_test.dart", + "src/AppTest.java", + "src/app.cy.ts", + "src/app.e2e.js", + "src/__snapshots__/app.snap", + "README.md", + "package.json", + "schema.yaml", + "fixtures/data.txt", + ]) { + assert.equal(isMagicNumberSourcePath(path), false, path); + } +}); + +test("extractNumericTokens: finds decimal, signed, fractional, exponent, bigint, and radix literals", () => { + assert.deepEqual( + extractNumericTokens("return -42 + 3.14 + .5 + 6e-3 + 99n + 0xff + 0b1010 + 0o77;").map((token) => token.value), + ["-42", "3.14", ".5", "6e-3", "99n", "0xff", "0b1010", "0o77"], + ); +}); + +test("extractNumericTokens: skips identifiers, property suffixes, and malformed radix prefixes", () => { + assert.deepEqual( + extractNumericTokens("v2 + thing42 + obj.404 + 0x + 0b + 0o + next_7").map((token) => token.value), + [], + ); +}); + +test("extractNumericTokens: treats signs as numeric only in expression-start positions", () => { + assert.deepEqual(extractNumericTokens("return value-7 + (-8) + x + +9;").map((token) => token.value), [ + "7", + "-8", + "+9", + ]); +}); + +test("isAllowedMagicNumberValue: suppresses trivial sentinels and common scales", () => { + for (const value of ["0", "-0", "1", "-1", "+1", "2", "-2", "100", "1000", "10", "10000", "1_000"]) { + assert.equal(isAllowedMagicNumberValue(value), true, value); + } +}); + +test("isAllowedMagicNumberValue: suppresses equivalent radix representations of common scales", () => { + for (const value of ["0x0", "0x1", "0x2", "0x64", "0b10", "0o144"]) { + assert.equal(isAllowedMagicNumberValue(value), true, value); + } +}); + +test("isAllowedMagicNumberValue: reports non-trivial values across number syntaxes", () => { + for (const value of ["3", "-3", "42", "255", "0xff", "0b1011", "0o77", "3.14", "6e-3", "99n"]) { + assert.equal(isAllowedMagicNumberValue(value), false, value); + } +}); + +test("detectMagicNumbers: reports genuine expression literals", () => { + assert.deepEqual(detectMagicNumbers("const timeoutMs = attempts * 37 + jitter(13);"), [ + { value: "37" }, + { value: "13" }, + ]); +}); + +test("detectMagicNumbers: ignores strings and trailing comments before scanning", () => { + assert.deepEqual(detectMagicNumbers('logger.info("wait 37 seconds"); // retry in 42 seconds'), []); + assert.deepEqual(detectMagicNumbers("return attempts * 19;"), [{ value: "19" }]); +}); + +test("detectMagicNumbers: ignores named constant declarations", () => { + for (const line of [ + "const MAX_RETRY_DELAY_MS = 37;", + "export const MAX_BATCH_SIZE = 500;", + "static readonly DEFAULT_WINDOW_DAYS = 90;", + "public static final int RETRY_WINDOW = 30;", + "final DEFAULT_LIMIT = 50;", + "val MAX_PAGE_SIZE = 250", + ]) { + assert.deepEqual(detectMagicNumbers(line), [], line); + } +}); + +test("detectMagicNumbers: does not suppress ordinary lower-case assignments", () => { + assert.deepEqual(detectMagicNumbers("const timeoutMs = 37;"), [{ value: "37" }]); + assert.deepEqual(detectMagicNumbers("let retryWindow = 45;"), [{ value: "45" }]); +}); + +test("detectMagicNumbers: ignores array indexes but reports values used in the indexed expression", () => { + assert.deepEqual(detectMagicNumbers("return rows[3] + columns[index + 7];"), [{ value: "7" }]); +}); + +test("detectMagicNumbers: ignores enum-like member initializers", () => { + assert.deepEqual(detectMagicNumbers("PENDING = 3,"), []); + assert.deepEqual(detectMagicNumbers(" Done = 4"), []); + assert.deepEqual(detectMagicNumbers("value = 5"), [{ value: "5" }]); +}); + +test("detectMagicNumbers: ignores numeric object keys but reports numeric values", () => { + assert.deepEqual(detectMagicNumbers("return { 404: handler, 503: fallback, retryAfter: 37 };"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: ignores allowlisted values while preserving non-trivial siblings", () => { + assert.deepEqual(detectMagicNumbers("return [0, 1, -1, 2, 100, 1000, 10, 25, 1_500];"), [ + { value: "25" }, + { value: "1_500" }, + ]); +}); + +test("detectMagicNumbers: skips pathologically long lines defensively", () => { + assert.deepEqual(detectMagicNumbers(`const x = ${"1".repeat(2100)};`), []); +}); + +test("scanPatchForMagicNumbers: flags added lines with correct new-file locations", () => { + const findings = scanPatchForMagicNumbers( + "src/retry.ts", + patchOf([ + "export function backoff(attempt: number) {", + " const delay = attempt * 37;", + " return Math.min(delay, 250);", + "}", + ]), + ); + assert.deepEqual(findings, [ + { file: "src/retry.ts", line: 2, value: "37" }, + { file: "src/retry.ts", line: 3, value: "250" }, + ]); +}); + +test("scanPatchForMagicNumbers: removed lines are ignored and hunk cursor stays correct", () => { + const patch = [ + "@@ -10,4 +10,4 @@", + " export function retry() {", // line 10 + "- return oldValue * 37;", + "+ return newValue * 43;", // line 11 + " }", // line 12 + "\\ No newline at end of file", + "@@ -30,2 +30,3 @@", + " function more() {", // line 30 + "+ return 256;", // line 31 + " }", // line 32 + ].join("\n"); + assert.deepEqual(scanPatchForMagicNumbers("src/retry.ts", patch), [ + { file: "src/retry.ts", line: 11, value: "43" }, + { file: "src/retry.ts", line: 31, value: "256" }, + ]); +}); + +test("scanPatchForMagicNumbers: added content that starts with plus signs is not mistaken for a file header", () => { + const patch = ["@@ -1,0 +1,2 @@", "+++counter += 33;", "+return value;"].join("\n"); + assert.deepEqual(scanPatchForMagicNumbers("src/counter.ts", patch), [ + { file: "src/counter.ts", line: 1, value: "33" }, + ]); +}); + +test("scanPatchForMagicNumbers: skips test files even when they contain reportable values", () => { + assert.deepEqual(scanPatchForMagicNumbers("src/retry.test.ts", patchOf(["expect(delay).toBe(37);"])), []); + assert.deepEqual(scanPatchForMagicNumbers("tests/retry.py", patchOf(["assert delay == 37"])), []); +}); + +test("scanPatchForMagicNumbers: skips non-source files", () => { + assert.deepEqual(scanPatchForMagicNumbers("README.md", patchOf(["Use 37 workers in the example."])), []); + assert.deepEqual(scanPatchForMagicNumbers("package.json", patchOf(['"port": 3377'])), []); +}); + +test("scanPatchForMagicNumbers: enforces the maxFindings cap", () => { + const lines = Array.from({ length: 30 }, (_, i) => `return metric + ${i + 3};`); + const patch = patchOf(lines); + assert.equal(scanPatchForMagicNumbers("src/a.ts", patch, { maxFindings: 5 }).length, 5); + assert.deepEqual(scanPatchForMagicNumbers("src/a.ts", patch, { maxFindings: 0 }), []); +}); + +test("scanPatchForMagicNumbers: abort signal stops scanning", () => { + const controller = new AbortController(); + controller.abort(); + assert.throws( + () => scanPatchForMagicNumbers("src/a.ts", patchOf(["return 37;"]), { signal: controller.signal }), + /analyzer_aborted/, + ); +}); + +test("scanMagicNumbers: scans every changed file and honors the global cap", async () => { + const noisyLines = Array.from({ length: 40 }, (_, i) => `export const value${i} = base + ${i + 3};`); + const findings = await scanMagicNumbers({ + repoFullName: "octo/repo", + prNumber: 1, + files: [ + { path: "src/quiet.ts", patch: patchOf(["const MAX_SIZE = 50;"]) }, + { path: "src/noisy.ts", patch: patchOf(noisyLines) }, + { path: "src/noisy.test.ts", patch: patchOf(["expect(value).toBe(777);"]) }, + ], + }); + + assert.equal(findings.length, 25); + assert.ok(findings.every((finding) => finding.file === "src/noisy.ts")); +}); + +test("scanMagicNumbers: no files or patches yields no findings", async () => { + assert.deepEqual(await scanMagicNumbers({ repoFullName: "octo/repo", prNumber: 1 }), []); + assert.deepEqual( + await scanMagicNumbers({ repoFullName: "octo/repo", prNumber: 1, files: [{ path: "src/a.ts" }] }), + [], + ); +}); + +test("scanMagicNumbers: abort signal stops the analyzer entrypoint", async () => { + const controller = new AbortController(); + controller.abort(); + await assert.rejects( + () => + scanMagicNumbers( + { + repoFullName: "octo/repo", + prNumber: 1, + files: [{ path: "src/a.ts", patch: patchOf(["return 37;"]) }], + }, + controller.signal, + ), + /analyzer_aborted/, + ); +}); + +test("detectMagicNumbers: strips block and hash comments after string blanking", () => { + assert.deepEqual(detectMagicNumbers("return value; /* retry in 37 seconds */"), []); + assert.deepEqual(detectMagicNumbers("return value # retry in 37 seconds"), []); + assert.deepEqual(detectMagicNumbers("return value + 29; /* cap at 37 */"), [{ value: "29" }]); +}); + +test("detectMagicNumbers: preserves numbers in template interpolation code but not template prose", () => { + assert.deepEqual(detectMagicNumbers("logger.debug(`retry in 37 seconds`)"), []); + assert.deepEqual(detectMagicNumbers("logger.debug(`retry ${attempt + 37}`)"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: recognizes public static final constants with primitive and reference types", () => { + assert.deepEqual(detectMagicNumbers("public static final int RETRY_WINDOW = 37;"), []); + assert.deepEqual(detectMagicNumbers("private static final Duration RETRY_WINDOW = 37;"), []); + assert.deepEqual(detectMagicNumbers("public static final RETRY_WINDOW = 37;"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: named constants require an uppercase constant-style name", () => { + assert.deepEqual(detectMagicNumbers("const MaxRetries = 37;"), [{ value: "37" }]); + assert.deepEqual(detectMagicNumbers("const MAX_RETRIES = 37;"), []); + assert.deepEqual(detectMagicNumbers("readonly MAX_RETRIES = 37;"), []); +}); + +test("detectMagicNumbers: reports numeric thresholds in common boolean and ternary expressions", () => { + assert.deepEqual(detectMagicNumbers("return latencyMs > 37 && failures < 9 ? 250 : 5;"), [ + { value: "37" }, + { value: "9" }, + { value: "250" }, + { value: "5" }, + ]); +}); + +test("detectMagicNumbers: reports negative thresholds after expression punctuation", () => { + assert.deepEqual(detectMagicNumbers("return clamp(value, -37, +43);"), [{ value: "-37" }, { value: "+43" }]); + assert.deepEqual(detectMagicNumbers("return value - 37;"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: handles numeric separators without changing the reported literal", () => { + assert.deepEqual(detectMagicNumbers("return bytes > 65_536 ? 4_096 : 512;"), [ + { value: "65_536" }, + { value: "4_096" }, + { value: "512" }, + ]); +}); + +test("detectMagicNumbers: ignores object keys only when they are key positions", () => { + assert.deepEqual(detectMagicNumbers("return { 37: handler, nested: { code: 43 } };"), [{ value: "43" }]); + assert.deepEqual(detectMagicNumbers("return map.get(37) ?? fallback[43];"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: ignores enum-like members only at assignment starts", () => { + assert.deepEqual(detectMagicNumbers("enumValue = STARTED = 37;"), [{ value: "37" }]); + assert.deepEqual(detectMagicNumbers("STARTED = 37,"), []); + assert.deepEqual(detectMagicNumbers(" STARTED = 37"), []); +}); + +test("extractNumericTokens: keeps source spans around accepted tokens", () => { + assert.deepEqual(extractNumericTokens("return x + 37;"), [{ value: "37", start: 11, end: 13 }]); + assert.deepEqual(extractNumericTokens("return x + -37;"), [{ value: "-37", start: 11, end: 14 }]); +}); + +test("scanPatchForMagicNumbers: handles multiple hunks with plus-prefixed content and no-newline marker", () => { + const patch = [ + "@@ -1,2 +1,2 @@", + "+const first = 37;", + "\\ No newline at end of file", + "@@ -20,2 +20,2 @@", + "+++value += 43;", + "+const second = 250;", + ].join("\n"); + assert.deepEqual(scanPatchForMagicNumbers("src/multi.ts", patch), [ + { file: "src/multi.ts", line: 1, value: "37" }, + { file: "src/multi.ts", line: 20, value: "43" }, + { file: "src/multi.ts", line: 21, value: "250" }, + ]); +}); + +test("scanPatchForMagicNumbers: keeps caps local to each call and global at entrypoint", async () => { + const first = patchOf(Array.from({ length: 20 }, (_, i) => `export const a${i} = base + ${i + 3};`)); + const second = patchOf(Array.from({ length: 20 }, (_, i) => `export const b${i} = base + ${i + 53};`)); + assert.equal(scanPatchForMagicNumbers("src/a.ts", first, { maxFindings: 7 }).length, 7); + const findings = await scanMagicNumbers({ + repoFullName: "octo/repo", + prNumber: 3, + files: [ + { path: "src/a.ts", patch: first }, + { path: "src/b.ts", patch: second }, + ], + }); + assert.equal(findings.length, 25); + assert.equal(findings.at(0)?.file, "src/a.ts"); + assert.equal(findings.at(-1)?.file, "src/b.ts"); +}); + +test("scanMagicNumbers: descriptor-compatible request with explicit analyzer subset stays deterministic", async () => { + const request = { + repoFullName: "octo/repo", + prNumber: 4, + analyzers: ["magicNumber"], + files: [{ path: "src/signal.ts", patch: patchOf(["return score * 37 + 43;"]) }], + }; + const first = await buildBrief(request); + const second = await buildBrief(request); + assert.deepEqual(first.findings.magicNumber, second.findings.magicNumber); + assert.equal(first.promptSection, second.promptSection); +}); + +test("detectMagicNumbers: handles language-specific collection and slicing syntax conservatively", () => { + assert.deepEqual(detectMagicNumbers("return items[3:37]"), [{ value: "3" }, { value: "37" }]); + assert.deepEqual(detectMagicNumbers("return items[:37]"), [{ value: "37" }]); + assert.deepEqual(detectMagicNumbers("return items[37:]"), [{ value: "37" }]); + assert.deepEqual(detectMagicNumbers("return matrix[2][37]"), []); +}); + +test("detectMagicNumbers: does not treat decimals as property access or object keys", () => { + assert.deepEqual(detectMagicNumbers("return ratio > 0.375 ? 3.5 : .25;"), [ + { value: "0.375" }, + { value: "3.5" }, + { value: ".25" }, + ]); +}); + +test("detectMagicNumbers: keeps hexadecimal and binary masks visible unless they are trivial", () => { + assert.deepEqual(detectMagicNumbers("return flags & 0xff;"), [{ value: "0xff" }]); + assert.deepEqual(detectMagicNumbers("return flags & 0b101010;"), [{ value: "0b101010" }]); + assert.deepEqual(detectMagicNumbers("return flags & 0b10;"), []); +}); + +test("detectMagicNumbers: ignores numeric-looking version fragments in identifiers", () => { + assert.deepEqual(detectMagicNumbers("return http2Enabled && ipv6Ready && tls13Ready;"), []); + assert.deepEqual(detectMagicNumbers("return handlerV2(input) + 37;"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: reports values in common standard-library calls", () => { + assert.deepEqual(detectMagicNumbers("return Math.round(value * 37) / 43;"), [{ value: "37" }, { value: "43" }]); + assert.deepEqual(detectMagicNumbers("return setTimeout(fn, 250);"), [{ value: "250" }]); + assert.deepEqual(detectMagicNumbers("return timedelta(seconds=37)"), [{ value: "37" }]); +}); + +test("detectMagicNumbers: supports signed exponents and BigInt suffixes", () => { + assert.deepEqual(detectMagicNumbers("return 6.25e-3 + 99n + -12n;"), [ + { value: "6.25e-3" }, + { value: "99n" }, + { value: "-12n" }, + ]); +}); + +test("detectMagicNumbers: trims reported literals to the public brief cap", () => { + const longLiteral = "9".repeat(80); + const [finding] = detectMagicNumbers(`return ${longLiteral};`); + assert.equal(finding.value, "9".repeat(40)); +}); + +test("scanPatchForMagicNumbers: respects source path gating across mixed-language patches", () => { + const patch = patchOf(["return threshold + 37;"]); + assert.deepEqual(scanPatchForMagicNumbers("src/a.rs", patch), [{ file: "src/a.rs", line: 1, value: "37" }]); + assert.deepEqual(scanPatchForMagicNumbers("src/a.swift", patch), [{ file: "src/a.swift", line: 1, value: "37" }]); + assert.deepEqual(scanPatchForMagicNumbers("src/a.yaml", patch), []); +}); + +test("scanPatchForMagicNumbers: handles hunk start line zero and empty hunk bodies", () => { + assert.deepEqual(scanPatchForMagicNumbers("src/generated.ts", "@@ -0,0 +0,0 @@"), []); + assert.deepEqual(scanPatchForMagicNumbers("src/zero.ts", "@@ -0,0 +0,1 @@\n+return 37;"), [ + { file: "src/zero.ts", line: 0, value: "37" }, + ]); +}); + +test("scanPatchForMagicNumbers: skips preamble additions until the first real hunk", () => { + const patch = ["+return 37;", "diff --git a/src/a.ts b/src/a.ts", "@@ -1,0 +10,1 @@", "+return 43;"].join("\n"); + assert.deepEqual(scanPatchForMagicNumbers("src/a.ts", patch), [{ file: "src/a.ts", line: 10, value: "43" }]); +}); + +test("scanPatchForMagicNumbers: one added line can emit several findings with the same location", () => { + assert.deepEqual(scanPatchForMagicNumbers("src/many.ts", patchOf(["return 37 + 43 + 59;"])), [ + { file: "src/many.ts", line: 1, value: "37" }, + { file: "src/many.ts", line: 1, value: "43" }, + { file: "src/many.ts", line: 1, value: "59" }, + ]); +}); + +test("scanPatchForMagicNumbers: ignores binary-file and no-patch markers", () => { + assert.deepEqual(scanPatchForMagicNumbers("src/image.ts", "Binary files differ"), []); + assert.deepEqual(scanPatchForMagicNumbers("src/empty.ts", ""), []); + assert.deepEqual(scanPatchForMagicNumbers("src/empty.ts", undefined), []); +}); + +test("scanPatchForMagicNumbers: preserves hunk cursor through deleted and context lines", () => { + const patch = [ + "@@ -10,4 +20,5 @@", + " const baseline = 37;", + "-return retry + 43;", + "+return retry + 59;", + " const after = 61;", + "+return after + 67;", + ].join("\n"); + + assert.deepEqual(scanPatchForMagicNumbers("src/cursor.ts", patch), [ + { file: "src/cursor.ts", line: 21, value: "59" }, + { file: "src/cursor.ts", line: 23, value: "67" }, + ]); +}); + +test("scanMagicNumbers: skips files without usable patches while scanning later files", async () => { + const findings = await scanMagicNumbers({ + repoFullName: "octo/repo", + prNumber: 9, + files: [ + { path: "src/no-patch.ts" }, + { path: "src/skip.md", patch: patchOf(["return 37;"]) }, + { path: "src/ok.ts", patch: patchOf(["return 43;"]) }, + ], + }); + + assert.deepEqual(findings, [{ file: "src/ok.ts", line: 1, value: "43" }]); +}); + +test("buildBrief: magicNumber participates in default registry when explicitly requested with another local analyzer", async () => { + const brief = await buildBrief({ + repoFullName: "octo/repo", + prNumber: 5, + analyzers: ["magicNumber", "todoMarker"], + files: [{ path: "src/a.ts", patch: patchOf(["// TODO: explain retry", "return attempts * 37;"]) }], + }); + + assert.equal(brief.analyzerStatus.magicNumber, "ok"); + assert.equal(brief.analyzerStatus.todoMarker, "ok"); + assert.deepEqual(brief.findings.magicNumber, [{ file: "src/a.ts", line: 2, value: "37" }]); + assert.deepEqual(brief.findings.todoMarker, [{ file: "src/a.ts", line: 1, tag: "TODO", note: "explain retry" }]); + assert.match(brief.promptSection, /Magic numbers/); + assert.match(brief.promptSection, /Incomplete-work markers/); +}); + +test("renderBrief: magic-number output is public-safe and does not include surrounding source", () => { + const { promptSection } = renderBrief({ + magicNumber: [{ file: "src/retry.ts", line: 7, value: "37" }], + }); + assert.match(promptSection, /src\/retry\.ts:7/); + assert.match(promptSection, /37/); + assert.doesNotMatch(promptSection, /attempt/); + assert.doesNotMatch(promptSection, /return/); +}); + +test("buildBrief: descriptor registry runs magicNumber explicitly", async () => { + const brief = await buildBrief({ + repoFullName: "octo/repo", + prNumber: 2, + analyzers: ["magicNumber"], + files: [{ path: "src/backoff.ts", patch: patchOf(["export const wait = attempt * 37;"]) }], + }); + + assert.equal(brief.partial, false); + assert.equal(brief.analyzerStatus.magicNumber, "ok"); + assert.deepEqual(brief.findings.magicNumber, [{ file: "src/backoff.ts", line: 1, value: "37" }]); + assert.match(brief.promptSection, /Magic numbers/); +}); + +test("renderBrief: magic-number findings render location and literal value", () => { + const { promptSection } = renderBrief({ + magicNumber: [ + { file: "src/retry.ts", line: 7, value: "37" }, + { file: "src/cache.ts", line: 9, value: "250" }, + ], + }); + assert.match(promptSection, /Magic numbers/); + assert.match(promptSection, /src\/retry\.ts:7/); + assert.match(promptSection, /37/); + assert.match(promptSection, /src\/cache\.ts:9/); +}); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index 20991c6e54..3690d8de16 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -37,6 +37,7 @@ export const REES_ANALYZER_NAMES = [ "looseRange", "terminology", "todoMarker", + "magicNumber", ] as const; export type ReesAnalyzerName = (typeof REES_ANALYZER_NAMES)[number];