diff --git a/docs/example-report.md b/docs/example-report.md index dee0f71..7cbbf1c 100644 --- a/docs/example-report.md +++ b/docs/example-report.md @@ -1,6 +1,6 @@ # DebtLens Report -Scanned **3** files with **36** rules in **162ms**. +Scanned **3** files with **35** rules in **162ms**. ## Summary diff --git a/docs/feature-flags-rfc.md b/docs/feature-flags-rfc.md new file mode 100644 index 0000000..2303db3 --- /dev/null +++ b/docs/feature-flags-rfc.md @@ -0,0 +1,56 @@ +# Feature-flag debt RFC + +Status: shipped with the opt-in `feature-flags` pack. + +## Contract + +The `stale-feature-flag` rule recognizes two conservative sources of flag identity: + +1. top-level boolean constants whose names match `featureFlags.constantNamePatterns`; and +2. boolean properties or top-level boolean constants in files matched by + `featureFlags.registryGlobs` (paths are relative to the scan target). + +Configured access patterns identify registry keys read through a call. `callee` is the +exact source-level callee text and `keyArgument` is a zero-based argument index. Only +string literals and no-substitution template literals are treated as keys. + +```json +{ + "pack": "feature-flags", + "featureFlags": { + "accessPatterns": [ + { "callee": "isEnabled", "keyArgument": 0 }, + { "callee": "featureClient.enabled", "keyArgument": 1 } + ], + "registryGlobs": ["src/flags.ts", "packages/*/src/flags/**"], + "constantNamePatterns": ["^(?:enable|disable)[A-Z]"] + } +} +``` + +`accessPatterns` and `constantNamePatterns` replace their defaults when configured; +`registryGlobs` extend across root and package configuration. Supported glob operators +are `*`, `**`, and `?`. Defaults recognize `isEnabled(key)`, `useFlag(key)`, and +`flags(key)`, plus common flag-like top-level constant names. The rule remains opt-in. + +## Findings + +- A literal boolean definition is always-on/off only when its identifier, property, or + configured literal-key access participates in conditional control flow. +- A configured registry entry is unreferenced only after all scanned files are + aggregated. Cross-file constant references therefore do not become false unused + findings. +- If a configured access call uses a dynamic key, unreferenced-registry findings are + suppressed for that scan because the detector cannot prove which entry it reads. + +## Non-goals + +- No flag-provider SDK is inferred without configuration. +- No dynamic key, computed registry property, remote rollout state, flag age, or rollout + percentage is resolved. +- No dead branch is rewritten automatically. +- Registry formats other than TS/JS boolean constants and object properties are not + parsed in this version. + +These limits favor missed findings over noisy cleanup advice. Add project-specific +patterns instead of broadening names globally. diff --git a/docs/parallel-scans.md b/docs/parallel-scans.md new file mode 100644 index 0000000..80998d7 --- /dev/null +++ b/docs/parallel-scans.md @@ -0,0 +1,47 @@ +# Parallel scans and shared cache + +DebtLens stays serial by default. For a large, CPU-heavy repository scan, enable +the bounded worker pool with either the automatic setting or an explicit size: + +```sh +debtlens scan . --parallel +debtlens scan . --concurrency 4 +``` + +Use `--concurrency 1` when comparing behavior, profiling startup, or running in a +single-CPU container. Parallel and serial scans have the same findings and stable +ordering. Cross-file rules still see the whole repository; they are not evaluated +independently on incomplete shards. + +Workers help when detector work is large enough to repay startup and source +transfer. Small repositories, narrow `--changed` scans, and source-tree execution +through `tsx` may be faster with `--concurrency 1`. The published built CLI avoids +the per-worker TypeScript runtime startup cost. + +## Restore the cache in CI + +`--cache-dir` enables the scan cache and writes `cache.json` below the supplied +directory: + +```sh +debtlens scan . --parallel --cache-dir .cache/debtlens +``` + +Save and restore `.cache/debtlens` with the CI provider's normal cache or artifact +mechanism. Cache entries use checkout-relative file identities and content hashes, +so a cache created at one runner's checkout path can hit after restoration at a +different path. The key also includes DebtLens version, selected rules, and all +finding-affecting rule configuration. + +Do not share a writable cache directory between simultaneous scans. Each cache +file is atomically replaced, but the store is a last-writer-wins local artifact, +not a network coordination service. Give parallel jobs separate writable paths, +then let the CI cache service publish one completed artifact. + +`--cache [path]` remains available for a specific cache file. Plugin-enabled scans +do not cache results because DebtLens cannot hash arbitrary plugin implementation +code. Plugin detectors also run in-process when worker concurrency is selected; +built-in file-local rules still use workers. + +For the design contract and comparative benchmark, see +[`performance-rfc.md`](./performance-rfc.md). diff --git a/docs/performance-rfc.md b/docs/performance-rfc.md new file mode 100644 index 0000000..bf05aa9 --- /dev/null +++ b/docs/performance-rfc.md @@ -0,0 +1,91 @@ +# Parallel scan and portable cache RFC + +Status: implemented in the scanner core. + +## Goals and invariants + +Large scans may use Node worker threads, but concurrency must never change the +finding contract. For the same files, DebtLens version, selected rules, and rule +configuration, serial and parallel scans emit byte-identical `issues` arrays. +Worker scheduling is not observable in finding or warning order. + +`--concurrency 1` is the reference serial implementation. `--parallel` selects a +CPU-based default capped at four workers; `--concurrency ` selects an explicit +pool size. Small scans can be slower in parallel because worker startup is real +work, so serial remains the default unless parallelism is requested. + +## Sharding and aggregation model + +Discovery and the canonical full source model are built by the coordinator. +File-local built-in rules are then given deterministic round-robin file shards. +Each worker parses its shard once and runs all selected file-local rules. The +coordinator concatenates shard findings in shard order and then restores detector +registry order before the existing normalization and stable issue sort. + +Cross-file rules are an explicit aggregation phase. Rules whose result depends on +repository-wide duplicates, graphs, imports, or paired instruction files run once +on the coordinator with the complete file set. This includes `duplicate-logic` +(and language variants), `duplicated-literal`, `import-cycle`, +`test-duplication`, `story-only-component`, `config-drift`, and the AI instruction +duplication/contradiction rules, plus `stale-feature-flag`, whose registry +definitions and uses may live in different files. They are never run independently +on file shards. + +Third-party plugin detectors are JavaScript functions and cannot be safely sent +through the structured-clone boundary. When worker concurrency is enabled, +built-in file-local rules use workers and plugin detectors retain the compatible +in-process path. Plugin findings are still merged in selected-rule order. Scan +caching remains disabled when plugins are loaded because their implementations +cannot be content-hash invalidated. + +## Worker protocol and failure behavior + +Workers receive source snapshots, clone-safe scan options, and built-in detector +IDs. They import the built-in registry themselves; detector functions are never +serialized. A response contains the detector ID, issues, warnings, and optional +profile timing. A worker error fails the scan instead of silently retrying with a +different correctness model. + +The source tree uses the TypeScript worker entry under `tsx`; built packages use +the compiled JavaScript entry. Benchmarks use built JavaScript because loading a +TypeScript runtime in every development worker adds startup cost that consumers of +the published CLI do not pay. + +## Portable cache contract + +Cache format version 3 is intentionally incompatible with earlier absolute-path +entries. Its scan key is SHA-256 over: + +- cache format version and DebtLens package version; +- checkout-root-relative scan target and changed-file identities; +- selected detector IDs and all finding-affecting scan/rule configuration. + +The entry also stores a sorted scan manifest as target-relative file identity plus +SHA-256 content hash. Cache hits therefore require the same logical paths and +contents, while the checkout may be restored under a different absolute root. +Absolute target and cache paths are not persisted in the cached result; they are +rehydrated for the current invocation. Writes continue to use a temporary file +followed by an atomic rename. + +Changing a file, rule configuration, selected rules, cache format, or DebtLens +version produces a miss. Concurrency is deliberately absent from the key because +it cannot affect findings. + +## Verification and performance gate + +Core tests compare serialized serial and parallel findings, exercise cross-file +rules, verify `--concurrency 1`, preserve deterministic warnings, and restore one +cache artifact into a different checkout root. + +After `npm run build`, this command generates a 240-file CPU-oriented fixture, +warms both modes, alternates execution order, compares median timings, and rejects +any byte-level finding difference: + +```sh +node scripts/benchmark.mjs --small-only --compare-parallel +``` + +The dedicated comparison requires at least a 1.05x median speedup by default. +`--runs` and `--min-speedup` make the sample count and machine-specific gate +explicit. The ordinary one-line benchmark fixtures retain their absolute runtime +budgets; they are intentionally not presented as evidence of parallel speedup. diff --git a/docs/rules.md b/docs/rules.md index 16c6e06..bb964b5 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -1043,7 +1043,7 @@ When this is a false positive: - deliberate fire-and-forget marked with `void` - callbacks passed directly to APIs that manage promise lifecycle -Confidence: **0.65–0.82**. Higher when the callee is clearly async or returns `Promise<...>`. +Confidence: **0.68–0.88**. Higher when the callee is clearly async or returns `Promise<...>`. ## `commented-out-code` @@ -1211,6 +1211,31 @@ When this is a false positive: Confidence: **0.62**. Co-occurring domain synonyms are often legitimate vocabulary, so this rule stays advisory. +## `stale-feature-flag` + +Flags feature flags that are hardcoded on/off in conditional control flow and configured +registry entries that are not referenced anywhere in the scan. This rule runs only in +the opt-in `feature-flags` pack (or when selected explicitly). + +Configuration: + +- `featureFlags.accessPatterns`: exact callee and zero-based literal-key argument shapes +- `featureFlags.registryGlobs`: registry paths/globs relative to the scan target +- `featureFlags.constantNamePatterns`: regexes for top-level boolean flag constants + +Why it matters: completed rollouts leave unreachable branches and unused registry entries +that continue to tax testing and maintenance. + +When this is a false positive: + +- the literal is a deliberate build-time switch rather than a rollout flag +- a registry is consumed through an unconfigured provider or non-TypeScript manifest +- a computed/dynamic access cannot be attributed to one literal key + +Dynamic configured accesses suppress unused-registry claims, and non-registry constants +must control a branch before they are reported. Confidence: **0.82–0.90**. See the +[feature-flag RFC](./feature-flags-rfc.md) for the exact contract and non-goals. + ## `ai-instruction-duplication` Flags the same normalized instruction block repeated across assistant instruction files such as `AGENTS.md`, `CLAUDE.md`, `.cursor/rules/**`, and `.github/copilot-instructions.md`. diff --git a/schema/debtlens.config.schema.json b/schema/debtlens.config.schema.json index fde724a..632d414 100644 --- a/schema/debtlens.config.schema.json +++ b/schema/debtlens.config.schema.json @@ -1396,6 +1396,51 @@ }, "additionalProperties": false }, + "featureFlags": { + "type": "object", + "description": "Configuration for the opt-in stale feature-flag detector.", + "properties": { + "accessPatterns": { + "type": "array", + "description": "Call shapes that read a literal feature-flag key.", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "callee": { + "type": "string", + "minLength": 1 + }, + "keyArgument": { + "type": "integer", + "minimum": 0, + "default": 0 + } + }, + "required": [ + "callee" + ] + } + }, + "registryGlobs": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "description": "Registry file globs relative to the scan target." + }, + "constantNamePatterns": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "description": "Regexes identifying top-level boolean flag constants outside registries." + } + }, + "additionalProperties": false + }, "failOn": { "enum": [ "info", diff --git a/scripts/benchmark.mjs b/scripts/benchmark.mjs index d1580da..286f967 100644 --- a/scripts/benchmark.mjs +++ b/scripts/benchmark.mjs @@ -1,10 +1,12 @@ import { spawnSync } from "node:child_process"; -import { readdirSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const cliEntrypoint = join(repoRoot, "src", "cli", "index.ts"); +const builtCliEntrypoint = join(repoRoot, "dist", "cli", "index.js"); const fixturesRoot = join(repoRoot, "tests", "benchmarks", "fixtures"); const fixtureNames = readdirSync(fixturesRoot).sort(); const args = process.argv.slice(2); @@ -12,11 +14,14 @@ const defaultBudgets = { small: 5000, medium: 30000, large: 120000 }; let failed = false; function usage() { - console.log(`Usage: node scripts/benchmark.mjs [--small-only] [--budget fixture=ms] + console.log(`Usage: node scripts/benchmark.mjs [--small-only] [--budget fixture=ms] [--compare-parallel] Options: --small-only Run only the small fixture for fast CI smoke checks. --budget fixture=ms Override a fixture budget. May be repeated or comma-separated. + --compare-parallel Compare built serial/worker scans on the large fixture. + --runs Measured comparison runs after warmup (default: 3). + --min-speedup Required median serial/parallel ratio (default: 1.05). Environment: DEBTLENS_BENCHMARK_BUDGETS Comma-separated fixture=ms overrides. @@ -46,12 +51,12 @@ function readOptionValues(flag) { function assertKnownArgs() { for (let index = 0; index < args.length; index += 1) { const arg = args[index]; - if (arg === "--help" || arg === "-h" || arg === "--small-only") continue; - if (arg === "--budget") { + if (arg === "--help" || arg === "-h" || arg === "--small-only" || arg === "--compare-parallel") continue; + if (arg === "--budget" || arg === "--runs" || arg === "--min-speedup") { index += 1; continue; } - if (arg.startsWith("--budget=")) continue; + if (arg.startsWith("--budget=") || arg.startsWith("--runs=") || arg.startsWith("--min-speedup=")) continue; throw new Error(`Unknown benchmark option: ${arg}`); } } @@ -155,4 +160,114 @@ for (const name of filter) { if (!withinBudget) failed = true; } +if (args.includes("--compare-parallel")) { + try { + runParallelComparison(); + } catch (error) { + console.error(error.message); + failed = true; + } +} + if (failed) process.exit(1); + +function runParallelComparison() { + if (!existsSync(builtCliEntrypoint)) { + throw new Error("Parallel comparison requires built output. Run `npm run build` first."); + } + const runs = parsePositiveInteger(readSingleOption("--runs") ?? "3", "--runs"); + const minSpeedup = parseBudgetMs(readSingleOption("--min-speedup") ?? "1.05", "--min-speedup"); + const target = createParallelBenchmarkFixture(); + const rules = "large-function,cognitive-complexity,complex-control-flow,long-parameter-list,god-file,todo-comment,commented-out-code,empty-catch,swallowed-error,floating-promise,dead-abstraction"; + const serialMs = []; + const parallelMs = []; + let expectedFindings; + + // Discard one run per mode so module and filesystem startup do not dominate + // the measured medians. Built JS is intentional: source/tsx worker startup is + // a development-mode cost, not the published CLI runtime. + try { + runBuiltScan(target, 1, rules); + runBuiltScan(target, 4, rules); + + for (let index = 0; index < runs; index += 1) { + const order = index % 2 === 0 ? [1, 4] : [4, 1]; + for (const concurrency of order) { + const result = runBuiltScan(target, concurrency, rules); + const findings = JSON.stringify(result.issues); + expectedFindings ??= findings; + if (findings !== expectedFindings) { + throw new Error(`Parallel comparison correctness failure at concurrency ${concurrency}: findings differ byte-for-byte.`); + } + (concurrency === 1 ? serialMs : parallelMs).push(result.summary.elapsedMs); + } + } + + const serialMedian = median(serialMs); + const parallelMedian = median(parallelMs); + const speedup = serialMedian / parallelMedian; + const passed = speedup >= minSpeedup; + console.log(`\nparallel comparison (generated large CPU fixture, 240 files, built JS): serial median ${serialMedian}ms, parallel median ${parallelMedian}ms, ${speedup.toFixed(2)}x speedup (required >= ${minSpeedup.toFixed(2)}x) ${passed ? "OK" : "FAIL"}`); + console.log(`findings: byte-identical across ${runs * 2} measured scans`); + if (!passed) failed = true; + } finally { + rmSync(target, { recursive: true, force: true }); + } +} + +function runBuiltScan(target, concurrency, rules) { + const result = spawnSync(process.execPath, [ + builtCliEntrypoint, + "scan", + target, + "--format", + "json", + "--min-severity", + "info", + "--concurrency", + String(concurrency), + ...(rules ? ["--rules", rules] : []), + ], { cwd: repoRoot, encoding: "utf8" }); + if (result.status !== 0) { + throw new Error(result.stderr || result.stdout || `Built scan exited ${result.status}`); + } + return JSON.parse(result.stdout); +} + +function createParallelBenchmarkFixture() { + const root = mkdtempSync(join(tmpdir(), "debtlens-parallel-benchmark-")); + const sourceDir = join(root, "src"); + mkdirSync(sourceDir); + for (let fileIndex = 0; fileIndex < 240; fileIndex += 1) { + const functions = []; + for (let functionIndex = 0; functionIndex < 10; functionIndex += 1) { + functions.push(`export function workload_${fileIndex}_${functionIndex}(input: number, alpha: number, beta: number) { + let total = input + ${fileIndex + functionIndex}; + for (let index = 0; index < 24; index += 1) { + if (index % 2 === 0) total += alpha; + else total += beta; + } + return total; +}`); + } + writeFileSync(join(sourceDir, `module-${fileIndex}.ts`), `${functions.join("\n\n")}\n`, "utf8"); + } + return root; +} + +function readSingleOption(flag) { + const values = readOptionValues(flag); + if (values.length > 1) throw new Error(`${flag} may only be provided once`); + return values[0]; +} + +function parsePositiveInteger(rawValue, label) { + const value = Number(rawValue); + if (!Number.isInteger(value) || value <= 0) throw new Error(`${label} must be a positive integer`); + return value; +} + +function median(values) { + const sorted = [...values].sort((a, b) => a - b); + return sorted[Math.floor(sorted.length / 2)]; +} diff --git a/src/config/defaults.ts b/src/config/defaults.ts index 533a48e..8fbbd10 100644 --- a/src/config/defaults.ts +++ b/src/config/defaults.ts @@ -118,4 +118,17 @@ export const defaultConfig: Required { }, additionalProperties: false, }, + featureFlags: { + type: "object", + description: "Configuration for the opt-in stale feature-flag detector.", + properties: { + accessPatterns: { + type: "array", + description: "Call shapes that read a literal feature-flag key.", + items: { + type: "object", + additionalProperties: false, + properties: { + callee: { type: "string", minLength: 1 }, + keyArgument: { type: "integer", minimum: 0, default: 0 }, + }, + required: ["callee"], + }, + }, + registryGlobs: { + type: "array", + items: { type: "string", minLength: 1 }, + description: "Registry file globs relative to the scan target.", + }, + constantNamePatterns: { + type: "array", + items: { type: "string", minLength: 1 }, + description: "Regexes identifying top-level boolean flag constants outside registries.", + }, + }, + additionalProperties: false, + }, failOn: { enum: [...severities], description: "Exit with code 1 when any reported issue meets this severity. The --fail-on CLI flag overrides this.", diff --git a/src/config/validateConfig.ts b/src/config/validateConfig.ts index 1a21008..b41cab7 100644 --- a/src/config/validateConfig.ts +++ b/src/config/validateConfig.ts @@ -23,6 +23,7 @@ const knownRootKeys = new Set([ "duplicatedLiteral", "todoComment", "namingDrift", + "featureFlags", "failOn", "failOnConfidence", "gatePreset", @@ -102,6 +103,7 @@ export function validateConfigShape(config: unknown): ConfigValidationResult { validateObjectWithStringArrayProperty(errors, "duplicatedLiteral", typed.duplicatedLiteral, "ignoreStrings"); validateNamingDrift(errors, typed.namingDrift); validateTodoComment(errors, typed.todoComment); + validateFeatureFlags(errors, typed.featureFlags); validateBudgets(errors, typed.budgets); validateBadge(errors, typed.badge); validatePriority(errors, typed.priority); @@ -109,6 +111,60 @@ export function validateConfigShape(config: unknown): ConfigValidationResult { return { valid: errors.length === 0, errors }; } +function validateFeatureFlags(errors: string[], value: unknown): void { + if (value === undefined) return; + if (!isPlainObject(value)) { + errors.push("featureFlags must be an object"); + return; + } + validateAllowedKeys(errors, "featureFlags", value, ["accessPatterns", "registryGlobs", "constantNamePatterns"]); + validateStringArray(errors, "featureFlags.registryGlobs", value.registryGlobs); + validateStringArray(errors, "featureFlags.constantNamePatterns", value.constantNamePatterns); + validateNonEmptyStringEntries(errors, "featureFlags.registryGlobs", value.registryGlobs); + validateNonEmptyStringEntries(errors, "featureFlags.constantNamePatterns", value.constantNamePatterns); + + if (Array.isArray(value.constantNamePatterns)) { + value.constantNamePatterns.forEach((pattern, index) => { + if (typeof pattern !== "string") return; + try { + new RegExp(pattern); + } catch { + errors.push(`featureFlags.constantNamePatterns[${index}] must be a valid regular expression`); + } + }); + } + + if (value.accessPatterns !== undefined) { + if (!Array.isArray(value.accessPatterns)) { + errors.push("featureFlags.accessPatterns must be an array"); + return; + } + value.accessPatterns.forEach((pattern, index) => { + const prefix = `featureFlags.accessPatterns[${index}]`; + if (!isPlainObject(pattern)) { + errors.push(`${prefix} must be an object`); + return; + } + validateAllowedKeys(errors, prefix, pattern, ["callee", "keyArgument"]); + if (typeof pattern.callee !== "string" || pattern.callee.trim().length === 0) { + errors.push(`${prefix}.callee must be a non-empty string`); + } + if (pattern.keyArgument !== undefined && (!Number.isInteger(pattern.keyArgument) || Number(pattern.keyArgument) < 0)) { + errors.push(`${prefix}.keyArgument must be a non-negative integer`); + } + }); + } +} + +function validateNonEmptyStringEntries(errors: string[], key: string, value: unknown): void { + if (!Array.isArray(value)) return; + value.forEach((entry, index) => { + if (typeof entry === "string" && entry.trim().length === 0) { + errors.push(`${key}[${index}] must be a non-empty string`); + } + }); +} + function validateUniqueStrings(errors: string[], key: string, value: unknown): void { if (!Array.isArray(value) || value.some((item) => typeof item !== "string")) return; if (new Set(value).size !== value.length) { diff --git a/src/core/parallelScan.ts b/src/core/parallelScan.ts index da15d5d..a2b1642 100644 --- a/src/core/parallelScan.ts +++ b/src/core/parallelScan.ts @@ -1,9 +1,58 @@ +import { existsSync } from "node:fs"; import { availableParallelism } from "node:os"; -import type { ScanOptions } from "./types.js"; +import { fileURLToPath } from "node:url"; +import { Worker } from "node:worker_threads"; +import type { DebtIssue, Detector, ScanOptions } from "./types.js"; +import type { FileSnapshot } from "./scanCache.js"; + +export interface WorkerDetectorResult { + detectorId: string; + issues: DebtIssue[]; + elapsedMs: number; + warnings: string[]; +} + +interface WorkerSuccessMessage { + ok: true; + results: WorkerDetectorResult[]; +} + +interface WorkerFailureMessage { + ok: false; + error: string; +} + +type WorkerMessage = WorkerSuccessMessage | WorkerFailureMessage; + +// These rules depend on repository-wide counts, graphs, duplicates, imports, +// or paired instruction surfaces. They must never run independently on file +// shards; scan.ts runs them once against the complete in-process context and +// merges them with worker results in detector registry order. +const CROSS_FILE_DETECTOR_IDS = new Set([ + "ai-instruction-contradiction", + "ai-instruction-duplication", + "config-drift", + "duplicate-logic", + "duplicated-literal", + "import-cycle", + "kotlin-duplicate-logic", + "python-duplicate-logic", + "ruby-duplicate-logic", + "story-only-component", + "stale-feature-flag", + "svelte-duplicate-logic", + "swift-duplicate-logic", + "test-duplication", + "vue-duplicate-logic", +]); + +export function isCrossFileDetector(detector: Detector): boolean { + return CROSS_FILE_DETECTOR_IDS.has(detector.id); +} export function resolveConcurrency(options: ScanOptions): number { if (options.concurrency !== undefined) return Math.max(1, options.concurrency); - return 1; + return options.parallel ? defaultConcurrency() : 1; } export function shouldUseWorkerPool(options: ScanOptions): boolean { @@ -14,10 +63,101 @@ export function defaultConcurrency(): number { return Math.max(1, Math.min(availableParallelism(), 4)); } -export async function shardFiles(items: T[], concurrency: number): Promise { +export function shardFiles(items: T[], concurrency: number): T[][] { + return shardRoundRobin(items, concurrency); +} + +/** + * Run file-local built-in detectors in real worker threads. Callers must keep + * cross-file detectors on the coordinator, where they retain a complete + * repository view and can be merged after the shard phase. + */ +export async function runBuiltinDetectorsInWorkers(input: { + detectors: Detector[]; + snapshots: FileSnapshot[]; + options: ScanOptions; + concurrency: number; +}): Promise { + if (input.detectors.length === 0) return []; + if (input.snapshots.length === 0) { + return input.detectors.map((detector) => ({ + detectorId: detector.id, + issues: [], + elapsedMs: 0, + warnings: [], + })); + } + + const workerCount = Math.min(Math.max(1, input.concurrency), input.snapshots.length); + const snapshotShards = shardRoundRobin(input.snapshots, workerCount); + const workerUrl = resolveWorkerUrl(); + const { pluginDetectors: _pluginDetectors, fileContents: _fileContents, ...workerOptions } = input.options; + + const detectorIds = input.detectors.map((detector) => detector.id); + const shardResults = await Promise.all(snapshotShards.map((snapshots) => runWorker(workerUrl, { + detectorIds, + snapshots, + options: workerOptions, + }))); + const byDetectorId = new Map(); + for (const result of shardResults.flat()) { + const aggregate = byDetectorId.get(result.detectorId); + if (!aggregate) { + byDetectorId.set(result.detectorId, { ...result, issues: [...result.issues], warnings: [...result.warnings] }); + continue; + } + aggregate.issues.push(...result.issues); + aggregate.elapsedMs += result.elapsedMs; + for (const warning of result.warnings) { + if (!aggregate.warnings.includes(warning)) aggregate.warnings.push(warning); + } + } + + return input.detectors.map((detector) => { + const result = byDetectorId.get(detector.id); + if (!result) throw new Error(`Parallel scan worker did not return detector "${detector.id}".`); + return result; + }); +} + +function shardRoundRobin(items: T[], concurrency: number): T[][] { const shards: T[][] = Array.from({ length: Math.max(1, concurrency) }, () => []); for (let index = 0; index < items.length; index += 1) { shards[index % shards.length]?.push(items[index] as T); } return shards.filter((shard) => shard.length > 0); } + +function resolveWorkerUrl(): URL { + const javascriptUrl = new URL("./parallelScanWorker.js", import.meta.url); + if (existsSync(fileURLToPath(javascriptUrl))) return javascriptUrl; + return new URL("./parallelScanWorkerSource.js", import.meta.url); +} + +function runWorker(workerUrl: URL, workerData: unknown): Promise { + return new Promise((resolve, reject) => { + const sourceWorker = workerUrl.pathname.endsWith("parallelScanWorkerSource.js"); + const workerPayload = sourceWorker + ? { + ...(workerData as Record), + sourceWorkerUrl: new URL("./parallelScanWorker.ts", import.meta.url).href, + } + : workerData; + const worker = new Worker(workerUrl, { workerData: workerPayload }); + let settled = false; + + worker.once("message", (message: WorkerMessage) => { + settled = true; + if (message.ok) resolve(message.results); + else reject(new Error(message.error)); + }); + worker.once("error", (error) => { + settled = true; + reject(error); + }); + worker.once("exit", (code) => { + if (!settled && code !== 0) reject(new Error(`Parallel scan worker exited with code ${code}.`)); + else if (!settled) reject(new Error("Parallel scan worker exited without returning results.")); + }); + }); +} diff --git a/src/core/parallelScanWorker.ts b/src/core/parallelScanWorker.ts new file mode 100644 index 0000000..184b350 --- /dev/null +++ b/src/core/parallelScanWorker.ts @@ -0,0 +1,77 @@ +import { basename, relative } from "node:path"; +import { parentPort, workerData } from "node:worker_threads"; +import { Project, ScriptTarget, ts } from "ts-morph"; +import { allDetectors } from "../detectors/index.js"; +import { detectSourceLanguage, languagesForDetector, parseSourceFile } from "./languages.js"; +import type { WorkerDetectorResult } from "./parallelScan.js"; +import type { FileSnapshot } from "./scanCache.js"; +import type { DetectorContext, ScanOptions, SourceFileInfo } from "./types.js"; + +interface ParallelWorkerData { + detectorIds: string[]; + snapshots: FileSnapshot[]; + options: ScanOptions; +} + +async function main(): Promise { + const data = workerData as ParallelWorkerData; + const project = new Project({ + compilerOptions: { + allowJs: true, + checkJs: false, + jsx: ts.JsxEmit.ReactJSX, + target: ScriptTarget.ES2022, + skipLibCheck: true, + }, + skipAddingFilesFromTsConfig: true, + }); + const files = loadSourceFiles(project, data.snapshots, data.options); + const detectorsById = new Map(allDetectors.map((detector) => [detector.id, detector])); + const results: WorkerDetectorResult[] = []; + + for (const detectorId of data.detectorIds) { + const detector = detectorsById.get(detectorId); + if (!detector) throw new Error(`Unknown built-in detector "${detectorId}" in parallel worker.`); + const warnings: string[] = []; + const startedAt = data.options.profile ? Date.now() : 0; + const allowedLanguages = new Set(languagesForDetector(detector)); + const context: DetectorContext = { + project, + files: files.filter((file) => allowedLanguages.has(file.language)), + options: data.options, + getThreshold: (key, fallback) => data.options.thresholds[key] ?? fallback, + addWarning: (warning) => { + if (!warnings.includes(warning)) warnings.push(warning); + }, + }; + const issues = await detector.detect(context); + results.push({ + detectorId, + issues, + elapsedMs: data.options.profile ? Date.now() - startedAt : 0, + warnings, + }); + } + + parentPort?.postMessage({ ok: true, results }); +} + +function loadSourceFiles(project: Project, snapshots: FileSnapshot[], options: ScanOptions): SourceFileInfo[] { + return snapshots.map((snapshot) => { + const relativePath = snapshot.absolutePath === options.target + ? basename(snapshot.absolutePath) + : relative(options.target, snapshot.absolutePath).replaceAll("\\", "/"); + return parseSourceFile({ + project, + absolutePath: snapshot.absolutePath, + relativePath, + content: snapshot.content, + language: detectSourceLanguage(snapshot.absolutePath), + }); + }); +} + +main().catch((error: unknown) => { + const message = error instanceof Error ? (error.stack ?? error.message) : String(error); + parentPort?.postMessage({ ok: false, error: message }); +}); diff --git a/src/core/parallelScanWorkerSource.js b/src/core/parallelScanWorkerSource.js new file mode 100644 index 0000000..c5e02b3 --- /dev/null +++ b/src/core/parallelScanWorkerSource.js @@ -0,0 +1,7 @@ +// Source-only worker bootstrap for tests and `tsx` development commands. +// Released builds resolve parallelScanWorker.js instead and do not depend on tsx. +import { workerData } from "node:worker_threads"; +import { register } from "tsx/esm/api"; + +await register(); +await import(workerData.sourceWorkerUrl); diff --git a/src/core/scan.ts b/src/core/scan.ts index 0905de9..350b8cf 100644 --- a/src/core/scan.ts +++ b/src/core/scan.ts @@ -5,7 +5,7 @@ import { allDetectors } from "../detectors/index.js"; import { buildDuplicateLogicClusters, buildRuleCorrelations, summarizeIssues } from "./issueAggregates.js"; import { buildImportGraphFromFiles } from "./importGraph.js"; import { DEFAULT_SOURCE_LANGUAGE, detectSourceLanguage, languagesForDetector, parseSourceFile } from "./languages.js"; -import { resolveConcurrency } from "./parallelScan.js"; +import { isCrossFileDetector, resolveConcurrency, runBuiltinDetectorsInWorkers, shouldUseWorkerPool } from "./parallelScan.js"; import { canonicalize, resolveFileSelection, type FileSelection } from "./resolveFiles.js"; import { buildScanCacheKey, getScanCachePath, hashContent, readCachedScan, writeCachedScan, type FileSnapshot } from "./scanCache.js"; import { compareSeverityDesc, meetsMinSeverity } from "./severity.js"; @@ -40,7 +40,7 @@ export async function scan(options: ScanOptions): Promise { const project = createScanProject(); const files = await loadSourceFiles(project, inputs.snapshots, options); const warnings = collectInitialWarnings(inputs, options); - const detectorResults = await runDetectors(inputs.detectors, { + const detectorResults = await runDetectors(inputs.detectors, inputs.snapshots, { project, files, options, @@ -75,7 +75,7 @@ async function prepareCoreScanInputs(options: ScanOptions): Promise, ): Promise { const runOne = async (detector: Detector): Promise => { @@ -344,11 +348,40 @@ async function runDetectors( }; }; - if (contextBase.options.parallel || (contextBase.options.concurrency ?? 0) > 1) { - const concurrency = contextBase.options.concurrency !== undefined - ? resolveConcurrency(contextBase.options) - : Math.max(1, detectors.length); - return runWithConcurrency(detectors, concurrency, runOne); + if (shouldUseWorkerPool(contextBase.options)) { + const builtinIds = new Set(allDetectors.map((detector) => detector.id)); + const builtinDetectors = detectors.filter((detector) => builtinIds.has(detector.id)); + const pluginDetectors = detectors.filter((detector) => !builtinIds.has(detector.id)); + const crossFileDetectors = builtinDetectors.filter(isCrossFileDetector); + const fileLocalDetectors = builtinDetectors.filter((detector) => !isCrossFileDetector(detector)); + const concurrency = resolveConcurrency(contextBase.options); + const [workerResults, crossFileResults, pluginResults] = await Promise.all([ + runBuiltinDetectorsInWorkers({ + detectors: fileLocalDetectors, + snapshots, + options: contextBase.options, + concurrency, + }), + // Repository-wide rules are their own explicit aggregation phase. They + // run once with all files after discovery, never once per file shard. + runWithConcurrency(crossFileDetectors, concurrency, runOne), + // Plugin functions cannot be structured-cloned. Retain the established + // in-process execution path for them while built-ins use worker threads. + runWithConcurrency(pluginDetectors, concurrency, runOne), + ]); + const detectorById = new Map(detectors.map((detector) => [detector.id, detector])); + const merged = [ + ...workerResults.map((result) => ({ + detector: detectorById.get(result.detectorId) as Detector, + issues: result.issues, + elapsedMs: result.elapsedMs, + warnings: result.warnings, + })), + ...crossFileResults, + ...pluginResults, + ]; + const byId = new Map(merged.map((result) => [result.detector.id, result])); + return detectors.map((detector) => byId.get(detector.id) as DetectorRunResult); } const results: DetectorRunResult[] = []; @@ -406,7 +439,9 @@ function selectDetectors( sourceLanguages: SourceLanguage[], ): Detector[] { if (!ruleIds || ruleIds.length === 0) { - return registry.filter((detector) => detectorCanRunOnSourceLanguages(detector, sourceLanguages)); + return registry.filter((detector) => + detector.defaultEnabled !== false + && detectorCanRunOnSourceLanguages(detector, sourceLanguages)); } const requested = new Set(ruleIds); diff --git a/src/core/scanCache.ts b/src/core/scanCache.ts index cb440ab..f50e2d2 100644 --- a/src/core/scanCache.ts +++ b/src/core/scanCache.ts @@ -1,12 +1,12 @@ import { mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { createHash } from "node:crypto"; -import { dirname, resolve } from "node:path"; +import { dirname, relative, resolve } from "node:path"; import type { Detector, ScanOptions, ScanResult } from "./types.js"; import { toCacheKeyPayload } from "./types.js"; import { packageVersion } from "../utils/packageInfo.js"; import { cleanupTempFile } from "../utils/tempFile.js"; -const CACHE_VERSION = 2; +const CACHE_VERSION = 3; const MAX_ENTRIES = 20; interface CacheFileEntry { @@ -28,6 +28,8 @@ interface CacheStore { export interface FileSnapshot { absolutePath: string; + /** Checkout-root-independent identity persisted in shareable caches. */ + cacheIdentity?: string; content: string; hash: string; } @@ -37,20 +39,27 @@ export function getScanCachePath(options: ScanOptions): string { return resolve(options.cwd, options.cachePath ?? ".debtlens/cache.json"); } -export function readCachedScan(cachePath: string, key: string, files: FileSnapshot[]): ScanResult | undefined { +export function readCachedScan(cachePath: string, key: string, files: FileSnapshot[], currentTarget?: string): ScanResult | undefined { const store = readCacheStore(cachePath); const entry = store.entries.find((candidate) => candidate.key === key); if (!entry || !sameFiles(entry.files, files)) return undefined; - return structuredClone(entry.result); + const result = structuredClone(entry.result); + if (currentTarget) result.options.target = currentTarget; + return result; } export function writeCachedScan(cachePath: string, key: string, files: FileSnapshot[], result: ScanResult): void { const store = readCacheStore(cachePath); + const portableResult = structuredClone(result); + portableResult.options.target = "."; + if (portableResult.summary.performance?.cache) { + portableResult.summary.performance.cache.path = "."; + } const nextEntry: CacheEntry = { key, createdAt: new Date().toISOString(), - files: files.map((file) => ({ path: file.absolutePath, hash: file.hash })), - result, + files: files.map((file) => ({ path: file.cacheIdentity ?? file.absolutePath, hash: file.hash })), + result: portableResult, }; const entries = [nextEntry, ...store.entries.filter((entry) => entry.key !== key)].slice(0, MAX_ENTRIES); mkdirSync(dirname(cachePath), { recursive: true }); @@ -65,8 +74,21 @@ export function writeCachedScan(cachePath: string, key: string, files: FileSnaps } } -export function buildScanCacheKey(options: ScanOptions, detectors: Detector[]): string { - return hashJson(toCacheKeyPayload(CACHE_VERSION, packageVersion, options, detectors)); +export function buildScanCacheKey( + options: ScanOptions, + detectors: Detector[], + files: FileSnapshot[] = [], + scannerVersion = packageVersion, +): string { + const payload = toCacheKeyPayload(CACHE_VERSION, scannerVersion, options, detectors); + return hashJson({ + ...payload, + target: portablePath(options.cwd, options.target), + changedFiles: options.changedFiles?.map((file) => portablePath(options.cwd, file)).sort(), + files: files + .map((file) => ({ path: file.cacheIdentity ?? portablePath(options.target, file.absolutePath), hash: file.hash })) + .sort((a, b) => a.path.localeCompare(b.path)), + }); } export function hashContent(content: string): string { @@ -88,7 +110,12 @@ function readCacheStore(cachePath: string): CacheStore { function sameFiles(cached: CacheFileEntry[], current: FileSnapshot[]): boolean { if (cached.length !== current.length) return false; const byPath = new Map(cached.map((file) => [file.path, file.hash])); - return current.every((file) => byPath.get(file.absolutePath) === file.hash); + return current.every((file) => byPath.get(file.cacheIdentity ?? file.absolutePath) === file.hash); +} + +function portablePath(root: string, path: string): string { + const portable = relative(resolve(root), resolve(path)).replaceAll("\\", "/"); + return portable || "."; } function hashJson(value: unknown): string { diff --git a/src/core/types.ts b/src/core/types.ts index b951808..8d9d756 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -51,6 +51,22 @@ export interface ScanThresholds { [key: string]: number; } +export interface FeatureFlagAccessPattern { + /** Exact callee name, such as `isEnabled` or `featureClient.isEnabled`. */ + callee: string; + /** Zero-based argument containing the literal flag key. Defaults to 0. */ + keyArgument?: number; +} + +export interface FeatureFlagsConfig { + /** Call shapes that read a flag by literal key. */ + accessPatterns?: FeatureFlagAccessPattern[]; + /** Registry file globs, relative to the scan target. */ + registryGlobs?: string[]; + /** Regexes identifying top-level boolean flag constants outside registries. */ + constantNamePatterns?: string[]; +} + export interface DebtLensConfig { include?: string[]; exclude?: string[]; @@ -87,6 +103,8 @@ export interface DebtLensConfig { /** Built-in labels to disable (e.g. "todo marker"). */ disableDefaults?: string[]; }; + /** Stale feature-flag detector configuration. */ + featureFlags?: FeatureFlagsConfig; /** Plugin API version this config targets; must match the DebtLens runtime version. */ pluginApiVersion?: number; /** Paths to local ESM plugin modules, resolved relative to the config file directory. */ @@ -147,6 +165,8 @@ export interface ScanOptions { todoCommentReplaceDefaults?: boolean; todoCommentDisableDefaults?: string[]; todoCommentMarkers?: Array<{ regex: RegExp; severity: Severity; label: string }>; + /** Configurable feature-flag access, registry, and constant-name contract. */ + featureFlags?: FeatureFlagsConfig; /** When true, collect per-rule timing in `summary.profile`. */ profile?: boolean; /** When true, emit valid inline suppression directives, including unused entries, for stale-suppression audits. */ @@ -225,6 +245,8 @@ export interface Detector { description: string; defaultSeverity: Severity; tags: string[]; + /** When false, the detector runs only when selected explicitly by rule id or pack. */ + defaultEnabled?: boolean; /** Languages this detector understands. Omitted means TypeScript/JavaScript only. */ languages?: SourceLanguage[]; detect: (context: DetectorContext) => Promise | DebtIssue[]; @@ -460,6 +482,7 @@ export interface CacheKeyInput { todoCommentReplaceDefaults?: boolean; todoCommentDisableDefaults?: string[]; todoCommentMarkers?: Array<{ regex: string; severity: Severity; label: string }>; + featureFlags?: FeatureFlagsConfig; } export function toCacheKeyPayload( @@ -497,6 +520,7 @@ export function toCacheKeyPayload( severity: marker.severity, label: marker.label, })), + featureFlags: options.featureFlags, }; } diff --git a/src/detectors/featureFlagDebt.ts b/src/detectors/featureFlagDebt.ts index 1f6a0b7..a14e42a 100644 --- a/src/detectors/featureFlagDebt.ts +++ b/src/detectors/featureFlagDebt.ts @@ -1,86 +1,345 @@ import { Node, SyntaxKind } from "ts-morph"; -import type { Node as MorphNode } from "ts-morph"; -import type { DebtIssue, Detector, DetectorContext } from "../core/types.js"; +import type { CallExpression, Node as MorphNode } from "ts-morph"; +import type { + DebtIssue, + Detector, + FeatureFlagAccessPattern, + FeatureFlagsConfig, + SourceFileInfo, +} from "../core/types.js"; +import { defaultConfig } from "../config/defaults.js"; import { createIssue } from "../utils/createIssue.js"; import { nodeLineSpan } from "../utils/lines.js"; +interface FlagDefinition { + file: string; + line: number; + key: string; + value: boolean; + registry: boolean; + referenced?: boolean; + conditionallyReferenced?: boolean; + unknownReference?: boolean; +} + +interface References { + keys: Set; + conditionalKeys: Set; + unknownKeyAccess: boolean; +} + +interface RegistryReceiver { + declaration: MorphNode; + name: string; + definitionsByKey: Map; +} + export const featureFlagDebtDetector: Detector = { id: "stale-feature-flag", name: "Stale feature flag", - description: "Flags feature flags that appear permanently enabled/disabled or unused.", + description: "Flags configured feature flags that are permanently enabled/disabled or unused.", defaultSeverity: "medium", tags: ["feature-flags", "cleanup", "maintainability"], - detect(context: DetectorContext): DebtIssue[] { - const patterns = ["flag", "feature", "enable", "enabled", "toggle"]; - const issues: DebtIssue[] = []; - - for (const file of context.files) { - const flagConstants = new Map(); - const flagUses = new Set(); - - for (const declaration of file.sourceFile.getVariableDeclarations()) { - if (!isTopLevelVariable(declaration)) continue; - const initializer = declaration.getInitializer(); - if (!initializer) continue; - const literalValue = readBooleanLiteral(initializer); - if (literalValue === undefined) continue; - const name = declaration.getName(); - if (!looksLikeFlagName(name, patterns)) continue; - const span = nodeLineSpan(declaration); - flagConstants.set(name, { file: file.relativePath, line: span.startLine, value: literalValue, name }); - } + defaultEnabled: false, + detect(context): DebtIssue[] { + const config = resolveConfig(context.options.featureFlags); + const registryMatchers = config.registryGlobs.map(globToRegExp); + const nameMatchers = config.constantNamePatterns.map((pattern) => new RegExp(pattern)); + const { definitions, registryReceivers } = collectDefinitions(context.files, registryMatchers, nameMatchers); + const references = collectReferences(context.files, config.accessPatterns, registryReceivers); - for (const identifier of file.sourceFile.getDescendantsOfKind(SyntaxKind.Identifier)) { - const text = identifier.getText(); - if (!flagConstants.has(text)) continue; - const parent = identifier.getParent(); - if (Node.isVariableDeclaration(parent) && parent.getName() === text) { - continue; - } - flagUses.add(text); - } + return definitions.flatMap((definition) => { + const configuredKeyReference = definition.registry && references.keys.has(definition.key); + const configuredConditionalReference = definition.registry && references.conditionalKeys.has(definition.key); + const referenced = definition.referenced === true || configuredKeyReference; + const conditionallyReferenced = definition.conditionallyReferenced === true || configuredConditionalReference; - for (const [name, info] of flagConstants.entries()) { - if (!flagUses.has(name)) { - issues.push(createIssue({ - detector: featureFlagDebtDetector, - confidence: 0.84, - file: info.file, - location: { startLine: info.line, endLine: info.line }, - message: `Feature flag ${name} is defined but never referenced.`, - evidence: [`Definition: ${name}`], - suggestion: "Remove the unused flag definition or wire it into the rollout path it was meant to guard.", - })); - continue; - } - issues.push(createIssue({ + if (definition.registry && !referenced && !definition.unknownReference && !references.unknownKeyAccess) { + return [createIssue({ detector: featureFlagDebtDetector, - confidence: 0.78, - file: info.file, - location: { startLine: info.line, endLine: info.line }, - message: `Feature flag ${name} is hardcoded to ${info.value}.`, - evidence: [`Literal value: ${String(info.value)}`], - suggestion: "Remove the flag and dead branch once rollout is complete, or source the value from configuration.", - })); + confidence: 0.9, + file: definition.file, + location: { startLine: definition.line, endLine: definition.line }, + message: `Feature flag ${definition.key} is defined in a configured registry but never referenced.`, + evidence: [`Registry definition: ${definition.key} = ${String(definition.value)}`], + suggestion: "Remove the unused registry entry, or add a configured access-pattern reference if the flag is still active.", + })]; } - } - return issues; + if (!conditionallyReferenced) return []; + return [createIssue({ + detector: featureFlagDebtDetector, + confidence: definition.registry ? 0.9 : 0.82, + file: definition.file, + location: { startLine: definition.line, endLine: definition.line }, + message: `Feature flag ${definition.key} is hardcoded to ${definition.value}.`, + evidence: [ + `Literal value: ${String(definition.value)}`, + "The flag is referenced by conditional control flow.", + ], + suggestion: "Remove the flag check and unreachable branch once rollout is complete, or source the value from a real flag provider.", + })]; + }); }, }; -function looksLikeFlagName(name: string, patterns: string[]): boolean { - const lower = name.toLowerCase(); - return patterns.some((pattern) => lower.includes(pattern)); +function resolveConfig(config: FeatureFlagsConfig | undefined): Required { + return { + accessPatterns: config?.accessPatterns ?? defaultConfig.featureFlags.accessPatterns ?? [], + registryGlobs: config?.registryGlobs ?? defaultConfig.featureFlags.registryGlobs ?? [], + constantNamePatterns: config?.constantNamePatterns ?? defaultConfig.featureFlags.constantNamePatterns ?? [], + }; } -function readBooleanLiteral(node: MorphNode): boolean | undefined { +function collectDefinitions( + files: SourceFileInfo[], + registryMatchers: RegExp[], + nameMatchers: RegExp[], +): { definitions: FlagDefinition[]; registryReceivers: RegistryReceiver[] } { + const definitions: FlagDefinition[] = []; + const receiversByDeclaration = new Map(); + + for (const file of files) { + const isRegistry = registryMatchers.some((matcher) => matcher.test(normalizePath(file.relativePath))); + + for (const declaration of file.sourceFile.getVariableDeclarations()) { + if (!isTopLevelVariable(declaration)) continue; + const value = readBooleanLiteral(declaration.getInitializer()); + if (value === undefined) continue; + const key = declaration.getName(); + if (!isRegistry && !nameMatchers.some((matcher) => matcher.test(key))) continue; + const nameNode = declaration.getNameNode(); + const referenceNodes = Node.isIdentifier(nameNode) ? nameNode.findReferencesAsNodes() : []; + definitions.push({ + ...definitionFor(file, declaration, key, value, isRegistry), + referenced: referenceNodes.length > 0, + conditionallyReferenced: referenceNodes.some(isUsedAsCondition), + }); + } + + if (!isRegistry) continue; + for (const property of file.sourceFile.getDescendantsOfKind(SyntaxKind.PropertyAssignment)) { + if (!Node.isObjectLiteralExpression(property.getParent())) continue; + const value = readBooleanLiteral(property.getInitializer()); + const key = readPropertyName(property.getNameNode()); + if (value === undefined || key === undefined) continue; + const definition = definitionFor(file, property, key, value, true); + definitions.push(definition); + + const objectLiteral = property.getParent(); + const declaration = objectLiteral.getParent(); + if (!Node.isVariableDeclaration(declaration) + || declaration.getInitializer() !== objectLiteral + || !isTopLevelVariable(declaration)) continue; + const nameNode = declaration.getNameNode(); + if (!Node.isIdentifier(nameNode)) continue; + const receiver = receiversByDeclaration.get(declaration) ?? { + declaration, + name: nameNode.getText(), + definitionsByKey: new Map(), + }; + const keyDefinitions = receiver.definitionsByKey.get(key) ?? []; + keyDefinitions.push(definition); + receiver.definitionsByKey.set(key, keyDefinitions); + receiversByDeclaration.set(declaration, receiver); + } + } + + return { definitions, registryReceivers: [...receiversByDeclaration.values()] }; +} + +function definitionFor( + file: SourceFileInfo, + node: MorphNode, + key: string, + value: boolean, + registry: boolean, +): FlagDefinition { + return { + file: file.relativePath, + line: nodeLineSpan(node).startLine, + key, + value, + registry, + }; +} + +function collectReferences( + files: SourceFileInfo[], + patterns: FeatureFlagAccessPattern[], + registryReceivers: RegistryReceiver[], +): References { + const references: References = { + keys: new Set(), + conditionalKeys: new Set(), + unknownKeyAccess: false, + }; + + for (const file of files) { + for (const access of file.sourceFile.getDescendantsOfKind(SyntaxKind.PropertyAccessExpression)) { + const receiver = findRegistryReceiver(access.getExpression(), registryReceivers); + if (receiver) markRegistryReference(receiver, access.getName(), isUsedAsCondition(access)); + } + for (const access of file.sourceFile.getDescendantsOfKind(SyntaxKind.ElementAccessExpression)) { + const receiver = findRegistryReceiver(access.getExpression(), registryReceivers); + if (!receiver) continue; + const key = readLiteralKey(access.getArgumentExpression()); + if (key === undefined) { + markUnknownRegistryReference(receiver); + continue; + } + markRegistryReference(receiver, key, isUsedAsCondition(access)); + } + + for (const call of file.sourceFile.getDescendantsOfKind(SyntaxKind.CallExpression)) { + const pattern = patterns.find((candidate) => callMatchesPattern(call, candidate)); + if (!pattern) continue; + const key = readLiteralKey(call.getArguments()[pattern.keyArgument ?? 0]); + if (key === undefined) { + references.unknownKeyAccess = true; + continue; + } + references.keys.add(key); + if (isUsedAsCondition(call)) references.conditionalKeys.add(key); + } + } + + return references; +} + +function findRegistryReceiver(expression: MorphNode, receivers: RegistryReceiver[]): RegistryReceiver | undefined { + const symbol = expression.getSymbol(); + const canonicalSymbol = symbol?.getAliasedSymbol() ?? symbol; + const declarations = canonicalSymbol?.getDeclarations() ?? []; + const symbolicMatch = receivers.find((receiver) => declarations.includes(receiver.declaration)); + if (symbolicMatch) return symbolicMatch; + if (!Node.isIdentifier(expression)) return undefined; + + const importedMatch = findImportedRegistryReceiver(expression, receivers); + if (importedMatch) return importedMatch; + if (declarations.length > 0) return undefined; + + const nameMatches = receivers.filter((receiver) => receiver.name === expression.getText()); + return nameMatches.length === 1 ? nameMatches[0] : undefined; +} + +function findImportedRegistryReceiver( + expression: MorphNode, + receivers: RegistryReceiver[], +): RegistryReceiver | undefined { + if (!Node.isIdentifier(expression)) return undefined; + const localName = expression.getText(); + for (const importDeclaration of expression.getSourceFile().getImportDeclarations()) { + const importedSource = importDeclaration.getModuleSpecifierSourceFile(); + if (!importedSource) continue; + for (const namedImport of importDeclaration.getNamedImports()) { + const importedLocalName = namedImport.getAliasNode()?.getText() ?? namedImport.getName(); + if (importedLocalName !== localName) continue; + const receiver = receivers.find((candidate) => + candidate.name === namedImport.getName() + && candidate.declaration.getSourceFile() === importedSource); + if (receiver) return receiver; + } + } + return undefined; +} + +function markUnknownRegistryReference(receiver: RegistryReceiver): void { + for (const definitions of receiver.definitionsByKey.values()) { + for (const definition of definitions) { + definition.unknownReference = true; + } + } +} + +function markRegistryReference(receiver: RegistryReceiver, key: string, conditional: boolean): void { + for (const definition of receiver.definitionsByKey.get(key) ?? []) { + definition.referenced = true; + if (conditional) definition.conditionallyReferenced = true; + } +} + +function callMatchesPattern(call: CallExpression, pattern: FeatureFlagAccessPattern): boolean { + return call.getExpression().getText() === pattern.callee; +} + +function isUsedAsCondition(node: MorphNode): boolean { + let current: MorphNode = node; + while (true) { + const parent = current.getParent(); + if (!parent) return false; + if (Node.isParenthesizedExpression(parent) || Node.isPrefixUnaryExpression(parent)) { + current = parent; + continue; + } + if (Node.isBinaryExpression(parent)) { + current = parent; + continue; + } + if (Node.isIfStatement(parent)) return parent.getExpression() === current; + if (Node.isConditionalExpression(parent)) return parent.getCondition() === current; + if (Node.isWhileStatement(parent) || Node.isDoStatement(parent)) return parent.getExpression() === current; + if (Node.isForStatement(parent)) return parent.getCondition() === current; + return false; + } +} + +function readBooleanLiteral(node: MorphNode | undefined): boolean | undefined { + if (!node) return undefined; if (Node.isTrueLiteral(node)) return true; if (Node.isFalseLiteral(node)) return false; return undefined; } -function isTopLevelVariable(declaration: MorphNode): boolean { - if (!Node.isVariableDeclaration(declaration)) return false; - return declaration.getVariableStatement()?.getParent() === declaration.getSourceFile(); +function readPropertyName(node: MorphNode): string | undefined { + if (Node.isIdentifier(node) || Node.isStringLiteral(node) || Node.isNumericLiteral(node)) { + return Node.isStringLiteral(node) ? node.getLiteralValue() : node.getText(); + } + return undefined; +} + +function readLiteralKey(node: MorphNode | undefined): string | undefined { + if (!node) return undefined; + if (Node.isStringLiteral(node) || Node.isNoSubstitutionTemplateLiteral(node)) return node.getLiteralValue(); + return undefined; +} + +function isTopLevelVariable(node: MorphNode): boolean { + return Node.isVariableDeclaration(node) + && node.getVariableStatement()?.getParent() === node.getSourceFile(); +} + +function normalizePath(path: string): string { + return path.replaceAll("\\", "/").replace(/^\.\//, ""); +} + +function globToRegExp(glob: string): RegExp { + const normalized = normalizePath(glob); + let regex = "^"; + for (let index = 0; index < normalized.length;) { + const char = normalized[index]; + if (char === "*") { + if (normalized[index + 1] === "*") { + if (normalized[index + 2] === "/") { + regex += "(?:.*/)?"; + index += 3; + } else { + regex += ".*"; + index += 2; + } + } else { + regex += "[^/]*"; + index += 1; + } + continue; + } + if (char === "?") { + regex += "[^/]"; + index += 1; + continue; + } + regex += char.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + index += 1; + } + return new RegExp(`${regex}$`); } diff --git a/tests/cli/scan.test.ts b/tests/cli/scan.test.ts index 8f477d0..4ab0ef5 100644 --- a/tests/cli/scan.test.ts +++ b/tests/cli/scan.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict"; import { execFileSync } from "node:child_process"; import { spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { createRequire } from "node:module"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -1096,6 +1096,41 @@ describe("debtlens scan performance flags", () => { rmSync(dir, { recursive: true, force: true }); } }); + + it("accepts explicit worker concurrency and a restorable cache directory", () => { + const dir = mkdtempSync(join(tmpdir(), "debtlens-cli-cache-dir-")); + try { + mkdirSync(join(dir, "src")); + writeFileSync(join(dir, "src", "Widget.ts"), "// TODO remove later\nexport const value = 1;\n"); + const args = [ + ".", + "--cwd", + dir, + "--rules", + "todo-comment", + "--cache-dir", + "restored/debtlens", + "--concurrency", + "2", + "--format", + "json", + ]; + + const first = runScan(args); + const second = runScan(args); + const firstJson = JSON.parse(first.stdout); + const secondJson = JSON.parse(second.stdout); + + assert.equal(first.status, 0); + assert.equal(firstJson.summary.performance.concurrency, 2); + assert.equal(firstJson.summary.performance.parallel, true); + assert.equal(existsSync(join(dir, "restored", "debtlens", "cache.json")), true); + assert.equal(second.status, 0); + assert.equal(secondJson.summary.performance.cache.hit, true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); describe("debtlens scan git modes", () => { diff --git a/tests/config/loadConfig.test.ts b/tests/config/loadConfig.test.ts index 20f53a9..dc10960 100644 --- a/tests/config/loadConfig.test.ts +++ b/tests/config/loadConfig.test.ts @@ -28,4 +28,27 @@ describe("mergeDebtLensConfig", () => { assert.equal(merged.exclude, undefined); assert.equal(merged.rules, undefined); }); + + it("merges registry globs while package access and name contracts override", () => { + const merged = mergeDebtLensConfig( + { + featureFlags: { + registryGlobs: ["src/flags.ts"], + accessPatterns: [{ callee: "isEnabled" }], + constantNamePatterns: ["^enable"], + }, + }, + { + featureFlags: { + registryGlobs: ["packages/app/flags.ts"], + accessPatterns: [{ callee: "appFlags.enabled", keyArgument: 1 }], + constantNamePatterns: ["^rollout"], + }, + }, + ); + + assert.deepEqual(merged.featureFlags?.registryGlobs, ["src/flags.ts", "packages/app/flags.ts"]); + assert.deepEqual(merged.featureFlags?.accessPatterns, [{ callee: "appFlags.enabled", keyArgument: 1 }]); + assert.deepEqual(merged.featureFlags?.constantNamePatterns, ["^rollout"]); + }); }); diff --git a/tests/config/mergeConfig.test.ts b/tests/config/mergeConfig.test.ts index 4183418..315e399 100644 --- a/tests/config/mergeConfig.test.ts +++ b/tests/config/mergeConfig.test.ts @@ -129,6 +129,22 @@ describe("mergeConfig", () => { assert.deepEqual(options.ruleConfidenceFloors, { "prop-drilling": 0.8 }); }); + it("merges feature-flag config and preserves conservative defaults", () => { + const options = mergeConfig(".", { + featureFlags: { + accessPatterns: [{ callee: "featureClient.enabled", keyArgument: 1 }], + registryGlobs: ["src/flags/**"], + constantNamePatterns: ["^rollout[A-Z]"], + }, + }, { cwd: process.cwd() }); + + assert.deepEqual(options.featureFlags?.accessPatterns, [ + { callee: "featureClient.enabled", keyArgument: 1 }, + ]); + assert.deepEqual(options.featureFlags?.registryGlobs, ["src/flags/**"]); + assert.deepEqual(options.featureFlags?.constantNamePatterns, ["^rollout[A-Z]"]); + }); + it("rejects invalid ruleSeverities values", () => { assert.throws( () => mergeConfig(".", { ruleSeverities: { "naming-drift": "loud" as never } }, { cwd: process.cwd() }), diff --git a/tests/config/schema.test.ts b/tests/config/schema.test.ts index 75f3734..e37b0b0 100644 --- a/tests/config/schema.test.ts +++ b/tests/config/schema.test.ts @@ -149,4 +149,24 @@ describe("config JSON schema", () => { assert.match(built.properties.pack?.anyOf[1]?.pattern ?? "", /compose/); assert.match(built.properties.pack?.anyOf[1]?.pattern ?? "", /svelte/); }); + + it("includes the featureFlags configuration contract", () => { + const built = buildConfigSchema() as { + properties: { + featureFlags?: { + type: string; + properties: { + accessPatterns: { type: string; items: { required: string[] } }; + registryGlobs: { type: string }; + constantNamePatterns: { type: string }; + }; + }; + }; + }; + + assert.equal(built.properties.featureFlags?.type, "object"); + assert.deepEqual(built.properties.featureFlags?.properties.accessPatterns.items.required, ["callee"]); + assert.equal(built.properties.featureFlags?.properties.registryGlobs.type, "array"); + assert.equal(built.properties.featureFlags?.properties.constantNamePatterns.type, "array"); + }); }); diff --git a/tests/config/validateConfig.test.ts b/tests/config/validateConfig.test.ts index 6fefcf2..28c0290 100644 --- a/tests/config/validateConfig.test.ts +++ b/tests/config/validateConfig.test.ts @@ -3,6 +3,42 @@ import { describe, it } from "node:test"; import { validateConfigShape } from "../../src/config/validateConfig.js"; describe("validateConfigShape", () => { + it("validates feature-flag access and registry configuration", () => { + const valid = validateConfigShape({ + featureFlags: { + accessPatterns: [{ callee: "featureClient.isEnabled", keyArgument: 1 }], + registryGlobs: ["src/flags/**"], + constantNamePatterns: ["^rollout[A-Z]"], + }, + }); + const invalid = validateConfigShape({ + featureFlags: { + accessPatterns: [{ callee: "", keyArgument: -1 }], + constantNamePatterns: ["["], + }, + }); + + assert.equal(valid.valid, true, valid.errors.join("; ")); + assert.equal(invalid.valid, false); + assert.ok(invalid.errors.some((error) => error.includes("callee"))); + assert.ok(invalid.errors.some((error) => error.includes("keyArgument"))); + assert.ok(invalid.errors.some((error) => error.includes("regular expression"))); + }); + + it("rejects empty feature-flag registry globs and constant-name patterns", () => { + const result = validateConfigShape({ + featureFlags: { + registryGlobs: ["", " "], + constantNamePatterns: ["", "\t"], + }, + }); + + assert.equal(result.valid, false); + assert.match(result.errors.join("\n"), /featureFlags\.registryGlobs\[0\] must be a non-empty string/); + assert.match(result.errors.join("\n"), /featureFlags\.registryGlobs\[1\] must be a non-empty string/); + assert.match(result.errors.join("\n"), /featureFlags\.constantNamePatterns\[0\] must be a non-empty string/); + assert.match(result.errors.join("\n"), /featureFlags\.constantNamePatterns\[1\] must be a non-empty string/); + }); it("accepts comma-separated built-in packs in config", () => { const result = validateConfigShape({ pack: "vue,svelte,kotlin,compose" }); diff --git a/tests/core/parallelScan.test.ts b/tests/core/parallelScan.test.ts index 65c794d..feedf43 100644 --- a/tests/core/parallelScan.test.ts +++ b/tests/core/parallelScan.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; -import { resolveConcurrency, shardFiles } from "../../src/core/parallelScan.js"; +import { defaultConcurrency, resolveConcurrency, shardFiles } from "../../src/core/parallelScan.js"; describe("parallel scan helpers", () => { it("shards files deterministically", async () => { @@ -10,8 +10,9 @@ describe("parallel scan helpers", () => { assert.deepEqual(shards[1], ["b.ts", "d.ts"]); }); - it("defaults concurrency to 1 unless configured", () => { + it("uses serial execution by default and a bounded CPU default for --parallel", () => { assert.equal(resolveConcurrency({ concurrency: 3 } as never), 3); assert.equal(resolveConcurrency({} as never), 1); + assert.equal(resolveConcurrency({ parallel: true } as never), defaultConcurrency()); }); }); diff --git a/tests/core/scan.cache.test.ts b/tests/core/scan.cache.test.ts index f0d3b33..f8035b2 100644 --- a/tests/core/scan.cache.test.ts +++ b/tests/core/scan.cache.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, it } from "node:test"; @@ -182,6 +182,42 @@ describe("scan cache", () => { } }); + it("reuses a restored cache after the checkout root changes", async () => { + const parent = mkdtempSync(join(tmpdir(), "debtlens-portable-cache-")); + const firstRoot = join(parent, "runner-one", "checkout"); + const secondRoot = join(parent, "runner-two", "checkout"); + const cacheDir = join(parent, "restored-cache"); + try { + for (const root of [firstRoot, secondRoot]) { + mkdirSync(join(root, "src"), { recursive: true }); + writeFileSync(join(root, "src", "app.ts"), "// TODO portable cache\nexport const value = 1;\n"); + } + const makeOptions = (root: string) => ({ + cwd: root, + target: root, + include: defaultConfig.include, + exclude: defaultConfig.exclude, + minSeverity: "low" as const, + rules: ["todo-comment"], + thresholds: defaultConfig.thresholds, + cache: true, + cacheDir, + }); + + const first = await scan(makeOptions(firstRoot)); + const restored = await scan(makeOptions(secondRoot)); + + assert.equal(first.summary.performance?.cache?.hit, false); + assert.equal(restored.summary.performance?.cache?.hit, true); + assert.equal(restored.options.target, secondRoot); + assert.equal(JSON.stringify(restored.issues), JSON.stringify(first.issues)); + const cacheText = readFileSync(join(cacheDir, "cache.json"), "utf8"); + assert.doesNotMatch(cacheText, /runner-one/); + } finally { + rmSync(parent, { recursive: true, force: true }); + } + }); + it("disables scan caching when plugin detectors are loaded", async () => { const dir = mkdtempSync(join(tmpdir(), "debtlens-scan-plugin-cache-")); try { diff --git a/tests/core/scan.parallel.test.ts b/tests/core/scan.parallel.test.ts index f37c05c..43a55d0 100644 --- a/tests/core/scan.parallel.test.ts +++ b/tests/core/scan.parallel.test.ts @@ -24,13 +24,28 @@ describe("scan parallel dispatch", () => { const serial = await scan(baseOptions); const parallel = await scan({ ...baseOptions, parallel: true }); - assert.deepEqual( - parallel.issues.map((issue) => [issue.ruleId, issue.file, issue.location?.startLine]), - serial.issues.map((issue) => [issue.ruleId, issue.file, issue.location?.startLine]), - ); + assert.equal(JSON.stringify(parallel.issues), JSON.stringify(serial.issues)); assert.equal(parallel.summary.performance?.parallel, true); }); + it("keeps --concurrency 1 on the serial execution path", async () => { + const cwd = process.cwd(); + const result = await scan({ + cwd, + target: resolve("examples/react"), + include: defaultConfig.include, + exclude: [], + minSeverity: "medium", + rules: ["duplicate-logic", "duplicated-literal", "import-cycle"], + thresholds: {}, + maxFiles: defaultConfig.maxFiles, + concurrency: 1, + }); + + assert.equal(result.summary.performance?.parallel, undefined); + assert.equal(result.summary.performance, undefined); + }); + it("keeps concurrency-based dispatch equivalent to serial dispatch", async () => { const cwd = process.cwd(); const baseOptions = { @@ -54,6 +69,70 @@ describe("scan parallel dispatch", () => { assert.equal(parallel.summary.performance?.parallel, true); }); + it("runs cross-file aggregation once with complete repository context", async () => { + const dir = mkdtempSync(join(tmpdir(), "debtlens-cross-file-parallel-")); + try { + mkdirSync(join(dir, "src")); + writeFileSync(join(dir, "src", "a.ts"), `import "./b";\nexport const a = "shared-domain-literal";\n`); + writeFileSync(join(dir, "src", "b.ts"), `import "./a";\nexport const b = "shared-domain-literal";\n`); + writeFileSync(join(dir, "src", "c.ts"), `export const c = "shared-domain-literal";\n`); + const baseOptions = { + cwd: dir, + target: dir, + include: defaultConfig.include, + exclude: defaultConfig.exclude, + minSeverity: "low" as const, + rules: ["duplicated-literal", "import-cycle"], + thresholds: defaultConfig.thresholds, + }; + + const serial = await scan({ ...baseOptions, concurrency: 1 }); + const parallel = await scan({ ...baseOptions, concurrency: 3 }); + + assert.ok(serial.issues.some((issue) => issue.ruleId === "duplicated-literal")); + assert.ok(serial.issues.some((issue) => issue.ruleId === "import-cycle")); + assert.equal(JSON.stringify(parallel.issues), JSON.stringify(serial.issues)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("keeps cross-file feature flag references byte-identical in parallel scans", async () => { + const dir = mkdtempSync(join(tmpdir(), "debtlens-feature-flag-parallel-")); + try { + mkdirSync(join(dir, "src")); + writeFileSync(join(dir, "src", "flags.ts"), "export const enableCheckout = true;\n"); + writeFileSync( + join(dir, "src", "app.ts"), + 'import { enableCheckout } from "./flags";\nexport const route = enableCheckout ? "/new" : "/old";\n', + ); + const baseOptions = { + cwd: dir, + target: dir, + include: defaultConfig.include, + exclude: defaultConfig.exclude, + minSeverity: "low" as const, + rules: ["stale-feature-flag"], + thresholds: defaultConfig.thresholds, + featureFlags: { + registryGlobs: ["src/flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }; + + const serial = await scan({ ...baseOptions, concurrency: 1 }); + const parallel = await scan({ ...baseOptions, concurrency: 2 }); + + assert.equal(serial.issues.length, 1); + assert.match(serial.issues[0]?.message ?? "", /hardcoded to true/); + assert.doesNotMatch(serial.issues[0]?.message ?? "", /never referenced/); + assert.equal(JSON.stringify(parallel.issues), JSON.stringify(serial.issues)); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("caps concurrent detector dispatch when concurrency is set", async () => { const dir = mkdtempSync(join(tmpdir(), "debtlens-scan-concurrency-cap-")); let active = 0; diff --git a/tests/core/scan.test.ts b/tests/core/scan.test.ts index 5fdf164..b7b5784 100644 --- a/tests/core/scan.test.ts +++ b/tests/core/scan.test.ts @@ -10,6 +10,22 @@ import { resolveFilePaths } from "../../src/core/resolveFiles.js"; import { scan } from "../../src/core/scan.js"; describe("scan integration", () => { + it("keeps opt-in detectors out of default scans while explicit rules and packs include them", async () => { + const dir = mkdtempSync(join(tmpdir(), "debtlens-opt-in-detector-")); + try { + writeFileSync(join(dir, "flags.ts"), "const enableCheckout = true;\nif (enableCheckout) launch();\n"); + const defaultResult = await scan(mergeConfig(".", {}, { cwd: dir })); + const explicitResult = await scan(mergeConfig(".", {}, { cwd: dir, rules: ["stale-feature-flag"] })); + const packResult = await scan(mergeConfig(".", { pack: "feature-flags" }, { cwd: dir })); + + assert.equal(defaultResult.summary.byRule["stale-feature-flag"], undefined); + assert.equal(explicitResult.summary.byRule["stale-feature-flag"], 1); + assert.equal(packResult.summary.byRule["stale-feature-flag"], 1); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("runs the full pipeline against examples/react", async () => { const cwd = process.cwd(); const result = await scan({ diff --git a/tests/core/scanCache.test.ts b/tests/core/scanCache.test.ts index 0cda247..d954929 100644 --- a/tests/core/scanCache.test.ts +++ b/tests/core/scanCache.test.ts @@ -53,6 +53,43 @@ describe("scan cache", () => { assert.equal(payload.packageVersion, packageVersion); assert.match(key, /^[a-f0-9]{64}$/); assert.equal(buildScanCacheKey(options, [stubDetector]), key); + assert.notEqual( + buildScanCacheKey({ + ...options, + featureFlags: { accessPatterns: [{ callee: "customFlags.enabled", keyArgument: 1 }] }, + }, [stubDetector]), + key, + ); + }); + + it("keeps keys portable across checkout roots and invalidates config or scanner versions", () => { + const makeOptions = (root: string): ScanOptions => ({ + cwd: root, + target: join(root, "packages", "app"), + include: defaultConfig.include, + exclude: defaultConfig.exclude, + minSeverity: "low", + rules: ["todo-comment"], + thresholds: defaultConfig.thresholds, + changedFiles: [join(root, "packages", "app", "src", "index.ts")], + }); + const first = makeOptions("/runner/one/checkout"); + const restored = makeOptions("/different/root/checkout"); + + assert.equal(buildScanCacheKey(first, [stubDetector]), buildScanCacheKey(restored, [stubDetector])); + assert.notEqual( + buildScanCacheKey(first, [stubDetector]), + buildScanCacheKey({ ...first, thresholds: { ...first.thresholds, "todo-comment.limit": 2 } }, [stubDetector]), + ); + assert.notEqual( + buildScanCacheKey(first, [stubDetector], [], "0.4.0"), + buildScanCacheKey(first, [stubDetector], [], "0.5.0"), + ); + const source = join(first.target, "src", "index.ts"); + assert.notEqual( + buildScanCacheKey(first, [stubDetector], [{ absolutePath: source, cacheIdentity: "src/index.ts", content: "one", hash: "one" }]), + buildScanCacheKey(first, [stubDetector], [{ absolutePath: source, cacheIdentity: "src/index.ts", content: "two", hash: "two" }]), + ); }); it("writes cache files atomically without leaving temp files behind", () => { diff --git a/tests/detectors/featureFlagDebt.test.ts b/tests/detectors/featureFlagDebt.test.ts index 15be93a..5ea699f 100644 --- a/tests/detectors/featureFlagDebt.test.ts +++ b/tests/detectors/featureFlagDebt.test.ts @@ -4,46 +4,225 @@ import { featureFlagDebtDetector } from "../../src/detectors/featureFlagDebt.js" import { runDetector } from "../helpers/runDetector.js"; describe("stale-feature-flag detector", () => { - it("flags hardcoded boolean feature flags", async () => { - const src = ` + it("flags a hardcoded flag constant used in conditional control flow", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "checkout.ts": ` const enableNewCheckout = true; export function render() { return enableNewCheckout ? "new" : "old"; } -`; - const issues = await runDetector(featureFlagDebtDetector, { "flags.ts": src }); +`, + }); + assert.equal(issues.length, 1); assert.equal(issues[0]?.ruleId, "stale-feature-flag"); + assert.match(issues[0]?.message ?? "", /hardcoded to true/); }); - it("flags unused boolean feature flags", async () => { - const src = ` -const enableBetaFeature = true; -export const version = 1; -`; - const issues = await runDetector(featureFlagDebtDetector, { "flags.ts": src }); - assert.equal(issues.length, 1); - assert.match(issues[0]?.message ?? "", /never referenced/); + it("ignores flag-like booleans that do not control a branch", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "telemetry.ts": ` +export const enabledTelemetry = true; +console.log(enabledTelemetry); +`, + }); + + assert.equal(issues.length, 0); }); - it("tracks same-named flags per file", async () => { + it("finds hardcoded and unreferenced keys in configured registry files", async () => { const issues = await runDetector(featureFlagDebtDetector, { - "one.ts": "const enableCheckout = true;\nexport const one = enableCheckout;\n", - "two.ts": "const enableCheckout = true;\nexport const two = enableCheckout;\n", + "src/flags/registry.ts": ` +export const featureFlags = { + "new-checkout": true, + abandonedSearch: false, +}; +`, + "src/checkout.ts": ` +export function checkout() { + if (featureClient.enabled("tenant", "new-checkout")) return "new"; + return "old"; +} +`, + }, { + featureFlags: { + registryGlobs: ["src/flags/**"], + accessPatterns: [{ callee: "featureClient.enabled", keyArgument: 1 }], + constantNamePatterns: [], + }, }); assert.equal(issues.length, 2); - assert.deepEqual(issues.map((issue) => issue.file).sort(), ["one.ts", "two.ts"]); + assert.ok(issues.some((issue) => /new-checkout is hardcoded to true/.test(issue.message))); + assert.ok(issues.some((issue) => /abandonedSearch.*never referenced/.test(issue.message))); }); - it("ignores local boolean helpers that only look like flags", async () => { - const src = ` -export function render(enabled) { - const enableButton = true; - return enabled && enableButton; -} -`; - const issues = await runDetector(featureFlagDebtDetector, { "local.ts": src }); + it("aggregates exported constant references across files", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "src/flags.ts": "export const enableCheckout = true;\n", + "src/app.ts": ` +import { enableCheckout } from "./flags"; +export const route = enableCheckout ? "/new" : "/old"; +`, + }, { + featureFlags: { + registryGlobs: ["src/flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 1); + assert.match(issues[0]?.message ?? "", /hardcoded/); + assert.doesNotMatch(issues[0]?.message ?? "", /never referenced/); + }); + + it("combines configured-key references with registry constant symbols", async () => { + const conditionalIssues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const checkout = true;\n", + "app.ts": "if (isEnabled(\"checkout\")) launch();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [{ callee: "isEnabled" }], + constantNamePatterns: [], + }, + }); + const nonConditionalIssues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const checkout = true;\n", + "app.ts": "export const active = isEnabled(\"checkout\");\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [{ callee: "isEnabled" }], + constantNamePatterns: [], + }, + }); + + assert.equal(conditionalIssues.length, 1); + assert.match(conditionalIssues[0]?.message ?? "", /hardcoded to true/); + assert.doesNotMatch(conditionalIssues[0]?.message ?? "", /never referenced/); + assert.equal(nonConditionalIssues.length, 0); + }); + + it("recognizes direct registry property checks", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: false };\n", + "app.ts": "if (flags.checkout) launch();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 1); + assert.match(issues[0]?.message ?? "", /checkout is hardcoded to false/); + }); + + it("does not confuse same-named properties on unrelated receivers with registry access", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: true };\n", + "app.ts": "const cart = { checkout: false };\nif (cart.checkout) purchase();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 1); + assert.match(issues[0]?.message ?? "", /checkout.*never referenced/); + }); + + it("does not let unrelated element access suppress unreferenced registry keys", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: true };\n", + "app.ts": "const values = [1];\nconsole.log(values[0]);\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 1); + assert.match(issues[0]?.message ?? "", /checkout.*never referenced/); + }); + + it("resolves literal property access through an imported registry alias", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: true };\n", + "app.ts": "import { flags as rolloutFlags } from './flags';\nif (rolloutFlags.checkout) launch();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 1); + assert.match(issues[0]?.message ?? "", /checkout is hardcoded to true/); + }); + + it("scopes dynamic element access through an imported alias to that registry receiver", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: true };\n", + "otherFlags.ts": "export const otherFlags = { search: false };\n", + "app.ts": "import { flags as rolloutFlags } from './flags';\nif (rolloutFlags[currentFlag]) launch();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts", "otherFlags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 1); + assert.match(issues[0]?.message ?? "", /search.*never referenced/); + }); + + it("suppresses unreferenced claims when configured access uses a dynamic key", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: true };\n", + "app.ts": "if (isEnabled(flagName)) launch();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [{ callee: "isEnabled" }], + constantNamePatterns: [], + }, + }); + assert.equal(issues.length, 0); }); + + it("suppresses unreferenced claims when direct element access uses a dynamic key", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "flags.ts": "export const flags = { checkout: true, search: false };\n", + "app.ts": "if (flags[currentFlag]) launch();\n", + }, { + featureFlags: { + registryGlobs: ["flags.ts"], + accessPatterns: [], + constantNamePatterns: [], + }, + }); + + assert.equal(issues.length, 0); + }); + + it("keeps same-named non-registry constants scoped conservatively", async () => { + const issues = await runDetector(featureFlagDebtDetector, { + "one.ts": "const enableCheckout = true;\nexport const one = enableCheckout ? 1 : 0;\n", + "two.ts": "const enableCheckout = true;\nexport const two = enableCheckout ? 2 : 0;\n", + }); + + assert.equal(issues.length, 2); + assert.deepEqual(issues.map((issue) => issue.file).sort(), ["one.ts", "two.ts"]); + }); }); diff --git a/tests/helpers/runDetector.ts b/tests/helpers/runDetector.ts index c24ad42..833dc86 100644 --- a/tests/helpers/runDetector.ts +++ b/tests/helpers/runDetector.ts @@ -1,6 +1,6 @@ import { Project, ScriptTarget, ts } from "ts-morph"; import { parseSourceFile } from "../../src/core/languages.js"; -import type { DebtIssue, Detector, ScanOptions, ScanThresholds, Severity, SourceFileInfo, SourceLanguage } from "../../src/core/types.js"; +import type { DebtIssue, Detector, FeatureFlagsConfig, ScanOptions, ScanThresholds, Severity, SourceFileInfo, SourceLanguage } from "../../src/core/types.js"; import { compileTodoCommentMarkers } from "../../src/detectors/todoComment.js"; export interface RunDetectorOptions { @@ -34,6 +34,8 @@ export interface RunDetectorOptions { changedFiles?: string[]; /** Override file contents keyed by relative path. */ fileContents?: Record; + /** Feature-flag access patterns, registry globs, and constant-name patterns. */ + featureFlags?: FeatureFlagsConfig; } function inferSourceLanguage(relativePath: string, override?: SourceLanguage): SourceLanguage { @@ -102,6 +104,7 @@ export async function runDetector( : undefined, changedFiles: options.changedFiles, fileContents: options.fileContents, + featureFlags: options.featureFlags, }; const issues = await detector.detect({