diff --git a/docs/architecture.md b/docs/architecture.md index a3e05b6..336fd8b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -95,6 +95,7 @@ runOperation(operation, codebaseGraph, input, context) - **Type/Shape facts**: Full-program parsing stores compact parameter/return/type-parameter facts on parsed symbols. `file`, `symbol`, and `search` JSON expose those facts additively; search indexes consumed/produced shape tokens so agents can ask which symbols touch a shape without a new command. - **Duplication families**: Parser stores deterministic function-body token streams on symbols. `duplicates` / `find_duplicates` groups symbols into strict, renamed, and near-miss clone families, with optional trace evidence for AI agents before refactors. - **Dead-code gates**: `check` can opt into unused-file, unused-type, unused-member, and dependency hygiene rules. These rules use graph metrics plus local TypeScript AST facts and emit confidence/evidence in JSON and SARIF. +- **Suppression hygiene**: `check` reports active and stale `ci-ignore-*` / `@expected-unused` suppressions, emits stale suppression findings via `no-stale-suppressions`, and treats `@public` exported type declarations as intentional public API while `@internal` stays checkable. - **Shared graph-load pipeline**: CLI commands and MCP stdio startup both use `src/graph-loader/` for path checks, legacy cache migration, cache reuse, parse/build/analyze, optional persistence, and stderr progress events. - **graphology**: In-memory graph with O(1) neighbor lookup. PageRank and betweenness computed via graphology-metrics. - **Batch git churn**: Single `git log --all --name-only` call, parsed for all files. Avoids O(n) subprocess spawning. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index c36476c..03f74cd 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -192,9 +192,9 @@ codebase-intelligence check [--config ] [--format ] [--fail-on **Gate modes:** `all` (default), `new-only`. -**Output:** pass/warn/fail verdict, findings, and summary counts. Findings include stable `fingerprint`; cleanup findings may also include `kind`, `confidence`, and `evidence`. Exit code `0` on pass, `1` when the configured gate fails, `2` for invalid config or arguments. +**Output:** pass/warn/fail verdict, findings, suppression ledger, and summary counts. Findings include stable `fingerprint`; cleanup findings may also include `kind`, `confidence`, and `evidence`. Summary counts include `suppressed` and `staleSuppressions`; `--summary` prints those counts when present. Exit code `0` on pass, `1` when the configured gate fails, `2` for invalid config or arguments. -**Rules:** `no-comments` (off by default), `no-circular-deps` (error), `no-dead-exports` (warn), plus opt-in cleanup gates: `no-dead-files`, `no-unused-types`, `no-unused-members`, `no-unused-deps`. Configure severities in `codebase-intelligence.json`. +**Rules:** `no-comments` (off by default), `no-circular-deps` (error), `no-dead-exports` (warn), `no-stale-suppressions` (warn), plus opt-in cleanup gates: `no-dead-files`, `no-unused-types`, `no-unused-members`, `no-unused-deps`. `ci-ignore-file`, `ci-ignore-next-line`, and JSDoc `@expected-unused` suppressions are reported as active/stale; JSDoc `@public` protects exported cleanup declarations while `@internal` remains checkable. Configure severities in `codebase-intelligence.json`. ### init diff --git a/docs/data-model.md b/docs/data-model.md index d510d98..09937cf 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -173,8 +173,9 @@ CacheFacts { ## Check Findings -`check --json` and MCP `check` return deterministic findings. `kind`, `confidence`, -and `evidence` are additive fields for rules that can explain cleanup confidence. +`check --json` and MCP `check` return deterministic findings plus a suppression +ledger. `kind`, `confidence`, and `evidence` are additive fields for rules that +can explain cleanup confidence. ```typescript Finding { @@ -192,4 +193,32 @@ Finding { actions?: FindingAction[] fingerprint: string } + +CheckSuppression { + directive: "ci-ignore-file" | "ci-ignore-next-line" | "@expected-unused" + status: "active" | "stale" + file: string + line: number + targetLine?: number + ruleIds: string[] + matchesAllRules: boolean + suppressed: number + message: string +} + +CheckSummary { + error: number + warn: number + suppressed: number + staleSuppressions: number + rules: Record +} + +CheckResult { + findings: Finding[] + suppressions: CheckSuppression[] + summary: CheckSummary + verdict: "pass" | "warn" | "fail" + configPath: string | null +} ``` diff --git a/docs/mcp-tools.md b/docs/mcp-tools.md index 46f1e60..4a11877 100644 --- a/docs/mcp-tools.md +++ b/docs/mcp-tools.md @@ -184,9 +184,9 @@ Community-detected clusters of related files. Run the configurable rules engine and gate on findings. **Input:** `{}` (uses the loaded graph + discovered config) -**Returns:** `{ verdict: "pass"|"warn"|"fail", summary: { error, warn, rules }, configPath, findings[] }`. Each finding has ruleId, severity, file, line, column, message, fingerprint, optional `kind`, optional `confidence`, optional `evidence[]`, and optional advisory `actions[]` (the tool is read-only — actions are hints, never applied). +**Returns:** `{ verdict: "pass"|"warn"|"fail", summary: { error, warn, suppressed, staleSuppressions, rules }, configPath, suppressions[], findings[] }`. Each finding has ruleId, severity, file, line, column, message, fingerprint, optional `kind`, optional `confidence`, optional `evidence[]`, and optional advisory `actions[]` (the tool is read-only — actions are hints, never applied). Each suppression records directive, status, file, line, targetLine, matched rule IDs, and suppressed count. -Rules: `no-comments` (off by default), `no-circular-deps` (error), `no-dead-exports` (warn), and opt-in cleanup gates `no-dead-files`, `no-unused-types`, `no-unused-members`, `no-unused-deps`. Configure severities and options in `codebase-intelligence.json` (validated by `schema.json`). +Rules: `no-comments` (off by default), `no-circular-deps` (error), `no-dead-exports` (warn), `no-stale-suppressions` (warn), and opt-in cleanup gates `no-dead-files`, `no-unused-types`, `no-unused-members`, `no-unused-deps`. `ci-ignore-file`, `ci-ignore-next-line`, and JSDoc `@expected-unused` suppressions are reported as active/stale; JSDoc `@public` protects exported cleanup declarations while `@internal` remains checkable. Configure severities and options in `codebase-intelligence.json` (validated by `schema.json`). **Use when:** Linting a codebase or enforcing a CI gate. "What rule violations exist?" **Not for:** Architecture metrics (use analyze_forces). diff --git a/llms-full.txt b/llms-full.txt index 4e090df..1024710 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -315,7 +315,7 @@ Entry point execution flows. Input: `{ entryPoint?: string, limit?: number }`. R Community-detected file clusters. Input: `{ minFiles?: number }`. Returns: clusters with cohesion. ## 18. check -Rules-engine gate. Input: `{}`. Returns: pass/warn/fail verdict, findings, config path, and summary counts. Findings always include ruleId, severity, file, line, column, message, and fingerprint; cleanup findings may also include kind, confidence, and evidence. +Rules-engine gate. Input: `{}`. Returns: pass/warn/fail verdict, findings, suppression ledger, config path, and summary counts. Findings always include ruleId, severity, file, line, column, message, and fingerprint; cleanup findings may also include kind, confidence, and evidence. Suppressions include directive, active/stale status, file, line, targetLine, ruleIds, and suppressed count. ## Tool Selection Guide @@ -470,7 +470,7 @@ content between the `codebase-intelligence:start`/`:end` markers is ever touched codebase-intelligence check [--config ] [--format ] [--fail-on ] [--gate ] [--base ] [--quiet] [--summary] [--json] [--force] ``` Rules-engine gate for CI. Formats: text, json, sarif. Fail-on: error, warn, never. -Gate modes: all, new-only. Returns pass/warn/fail verdict, findings, and summary counts. Built-in rules include no-comments, no-circular-deps, no-dead-exports, and opt-in no-dead-files, no-unused-types, no-unused-members, no-unused-deps. +Gate modes: all, new-only. Returns pass/warn/fail verdict, findings, suppression ledger, and summary counts. Built-in rules include no-comments, no-circular-deps, no-dead-exports, no-stale-suppressions, and opt-in no-dead-files, no-unused-types, no-unused-members, no-unused-deps. Reports active/stale `ci-ignore-*` and `@expected-unused` suppressions; `@public` protects exported cleanup declarations while `@internal` remains checkable. ## Global Behavior diff --git a/llms.txt b/llms.txt index d7c25a3..bfee888 100644 --- a/llms.txt +++ b/llms.txt @@ -36,7 +36,7 @@ codebase-intelligence impact ./src getUserById codebase-intelligence rename ./src oldName newName codebase-intelligence processes ./src --entry main codebase-intelligence clusters ./src --min-files 3 -codebase-intelligence check ./src # rules gate for CI; JSON findings may include kind/confidence/evidence +codebase-intelligence check ./src # rules gate for CI; JSON includes findings plus active/stale suppressions codebase-intelligence init . # make AI agents use CI ``` diff --git a/roadmap.md b/roadmap.md index f045a6e..7c0bbd5 100644 --- a/roadmap.md +++ b/roadmap.md @@ -333,7 +333,6 @@ Extend deletion intelligence beyond exported symbols. **Remaining:** -- Add stale suppression reporting and `@public` / `@expected-unused` semantics in CH-P1-06. - Add workspace/package-section dependency policy beyond single-package `package.json`. - Add CSS/template/framework plugin coverage only after `doctor` can explain supported surfaces. @@ -343,11 +342,17 @@ Finish suppression management without hiding stale debt. **Already exists:** `ci-ignore-next-line` and `ci-ignore-file`. -**To do:** +**Foundation slice:** + +- Report active and stale `ci-ignore-file`, `ci-ignore-next-line`, and JSDoc `@expected-unused` suppressions in JSON. +- Add `no-stale-suppressions` warnings so stale suppressions can fail CI with `--fail-on warn`. +- Include `suppressed` and `staleSuppressions` in check summaries and `--summary` output when relevant. +- Add JSDoc `@public` and `@internal` semantics for cleanup declarations: public exported types are protected, internal exported types remain checkable. +- Add CH-P1-06 coverage for active suppression, stale suppression drift, JSDoc cleanup semantics, JSON summary, CLI summary, and SARIF properties. + +**Remaining:** -- Add JSDoc `@public`, `@internal`, and `@expected-unused` semantics. -- Add stale-suppression detection. -- Report suppressions in JSON and CI summaries. +- No open CH-P1-06 work on the current TypeScript cleanup surface. --- diff --git a/schema.json b/schema.json index 161d62a..bd80c18 100644 --- a/schema.json +++ b/schema.json @@ -83,6 +83,10 @@ "$ref": "#/$defs/severity", "description": "Report package.json dependencies that are never imported." }, + "no-stale-suppressions": { + "$ref": "#/$defs/severity", + "description": "Report ci-ignore and @expected-unused suppressions that no longer hide any finding." + }, "no-duplication": { "description": "Report duplicated code (clone families).", "oneOf": [ diff --git a/src/rules/check.ts b/src/rules/check.ts index 7251afc..7302c2a 100644 --- a/src/rules/check.ts +++ b/src/rules/check.ts @@ -4,25 +4,32 @@ import path from "path"; import type { CheckResult, CheckSummary, + CheckSuppression, CodebaseGraph, CodebaseIntelligenceConfig, Finding, Verdict, } from "../types/index.js"; import { loadConfig, type ConfigOverrides } from "../config/index.js"; -import { runEngine, type RuleContext } from "./engine.js"; +import { runEngineWithSuppressions, type RuleContext } from "./engine.js"; import { ALL_RULES } from "./registry.js"; -function summarize(findings: Finding[]): CheckSummary { +function summarize(findings: Finding[], suppressions: CheckSuppression[]): CheckSummary { const rules: Record = {}; let error = 0; let warn = 0; + let suppressed = 0; + let staleSuppressions = 0; for (const f of findings) { rules[f.ruleId] = (rules[f.ruleId] ?? 0) + 1; if (f.severity === "error") error++; else warn++; } - return { error, warn, rules }; + for (const suppression of suppressions) { + if (suppression.status === "active") suppressed += suppression.suppressed; + else staleSuppressions += 1; + } + return { error, warn, suppressed, staleSuppressions, rules }; } function computeVerdict(summary: CheckSummary, config: CodebaseIntelligenceConfig): Verdict { @@ -105,7 +112,9 @@ export function runCheck( }; const ctx: RuleContext = { graph, rootDir: resolvedRoot, config, fileRelPaths, sourceOf }; - let findings = runEngine(ctx, ALL_RULES, config); + const engineResult = runEngineWithSuppressions(ctx, ALL_RULES, config); + let findings = engineResult.findings; + let suppressions = engineResult.suppressions; if (config.ci?.gate === "new-only") { const base = config.ci.base; @@ -117,13 +126,14 @@ export function runCheck( process.stderr.write(`Warning: could not diff against '${base}'; running full check.\n`); } else { findings = findings.filter((f) => changed.has(toPosix(f.file))); + suppressions = suppressions.filter((suppression) => changed.has(toPosix(suppression.file))); } } } - const summary = summarize(findings); + const summary = summarize(findings, suppressions); const verdict = computeVerdict(summary, config); - return { findings, summary, verdict, configPath }; + return { findings, suppressions, summary, verdict, configPath }; } /** Process exit code for a check result: 1 on fail, 0 otherwise. (Config errors → 2, handled by the CLI.) */ diff --git a/src/rules/dead-code.ts b/src/rules/dead-code.ts index 7a500b5..afed6a1 100644 --- a/src/rules/dead-code.ts +++ b/src/rules/dead-code.ts @@ -14,6 +14,7 @@ interface TypeDeclarationFact { line: number; column: number; exported: boolean; + jsDocTags: Set; } interface MemberDeclarationFact { @@ -24,6 +25,7 @@ interface MemberDeclarationFact { line: number; column: number; used: boolean; + jsDocTags: Set; } interface DependencyDeclaration { @@ -131,6 +133,7 @@ function collectUnusedTypeFindings(ctx: RuleContext): ReportedFinding[] { for (const typeFact of facts.types) { const metrics = ctx.graph.fileMetrics.get(typeFact.file); if (typeFact.exported) { + if (hasJsDocTag(typeFact, "public")) continue; if (!metrics?.deadExports.includes(typeFact.name)) continue; if (entrypointReason(ctx, typeFact.file, metrics)) continue; findings.push({ @@ -310,6 +313,7 @@ function parseSourceFacts(ctx: RuleContext, file: string): ParsedSourceFacts | n kind: "type", ...locationOf(sourceFile, node.name), exported: isExported(node), + jsDocTags: jsDocTagNames(node), }); } else if (ts.isInterfaceDeclaration(node)) { typeDeclarationPositions.add(node.name.getStart(sourceFile)); @@ -319,6 +323,7 @@ function parseSourceFacts(ctx: RuleContext, file: string): ParsedSourceFacts | n kind: "interface", ...locationOf(sourceFile, node.name), exported: isExported(node), + jsDocTags: jsDocTagNames(node), }); } else if (ts.isEnumDeclaration(node) && !isExported(node)) { typeDeclarationPositions.add(node.name.getStart(sourceFile)); @@ -334,6 +339,7 @@ function parseSourceFacts(ctx: RuleContext, file: string): ParsedSourceFacts | n kind: "enum-member", ...locationOf(sourceFile, member.name), used: false, + jsDocTags: jsDocTagNames(member), }); } } else if (ts.isClassDeclaration(node) && node.name) { @@ -350,6 +356,7 @@ function parseSourceFacts(ctx: RuleContext, file: string): ParsedSourceFacts | n kind: "private-class-member", ...locationOf(sourceFile, member.name), used: false, + jsDocTags: jsDocTagNames(member), }); } } @@ -603,6 +610,14 @@ function isExported(node: ts.Node): boolean { return ts.canHaveModifiers(node) && (ts.getModifiers(node)?.some((modifier) => modifier.kind === ts.SyntaxKind.ExportKeyword) ?? false); } +function jsDocTagNames(node: ts.Node): Set { + return new Set(ts.getJSDocTags(node).map((tag) => tag.tagName.getText().replace(/^@/, ""))); +} + +function hasJsDocTag(fact: { jsDocTags: Set }, tag: string): boolean { + return fact.jsDocTags.has(tag); +} + function isPrivateClassMember(member: ts.ClassElement): member is ts.PropertyDeclaration | ts.MethodDeclaration | ts.GetAccessorDeclaration | ts.SetAccessorDeclaration { if (!("name" in member) || !member.name) return false; if (ts.isPrivateIdentifier(member.name)) return true; diff --git a/src/rules/engine.ts b/src/rules/engine.ts index d5fd3e5..bbae46a 100644 --- a/src/rules/engine.ts +++ b/src/rules/engine.ts @@ -2,6 +2,7 @@ import { createHash } from "crypto"; import type { CodebaseGraph, CodebaseIntelligenceConfig, + CheckSuppression, Finding, FindingAction, FindingConfidence, @@ -69,39 +70,140 @@ function fingerprint(ruleId: string, file: string, line: number, message: string type RuleSet = "all" | Set; +interface SuppressionDirectiveState { + directive: CheckSuppression["directive"]; + file: string; + line: number; + targetLine?: number; + rules: RuleSet; + suppressed: number; + suppressedRuleIds: Set; +} + interface FileSuppressions { - fileRules: RuleSet | null; - lineRules: Map; + directives: SuppressionDirectiveState[]; + fileDirectives: SuppressionDirectiveState[]; + lineDirectives: Map; } // Matches both line (`// ci-ignore-...`) and block (`/* ci-ignore-... */`) forms. const SUPPRESS_RE = /(?:\/\/|\/\*)\s*ci-ignore-(file|next-line)\b([^\n*]*)/; +const EXPECTED_UNUSED_RULES = new Set([ + "no-dead-exports", + "no-dead-files", + "no-unused-types", + "no-unused-members", +]); +const STALE_SUPPRESSION_RULE_ID = "no-stale-suppressions"; -function mergeRuleSet(existing: RuleSet | null, incoming: RuleSet): RuleSet { - if (existing === null) return incoming; - if (existing === "all" || incoming === "all") return "all"; - return new Set([...existing, ...incoming]); +export interface EngineResult { + findings: Finding[]; + suppressions: CheckSuppression[]; } -function parseSuppressions(source: string): FileSuppressions { +function parseRuleSet(list: string): RuleSet { + const trimmed = list.trim(); + return trimmed.length === 0 ? "all" : new Set(trimmed.split(/[\s,]+/).filter(Boolean)); +} + +function addLineDirective(parsed: FileSuppressions, directive: SuppressionDirectiveState): void { + const existing = parsed.lineDirectives.get(directive.targetLine ?? directive.line) ?? []; + existing.push(directive); + parsed.lineDirectives.set(directive.targetLine ?? directive.line, existing); + parsed.directives.push(directive); +} + +function parseSuppressions(source: string, file: string): FileSuppressions { const lines = source.split(/\r?\n/); - let fileRules: RuleSet | null = null; - const lineRules = new Map(); + const parsed: FileSuppressions = { directives: [], fileDirectives: [], lineDirectives: new Map() }; lines.forEach((text, idx) => { const match = SUPPRESS_RE.exec(text); if (!match) return; - const list = match[2].trim(); - const set: RuleSet = list.length === 0 ? "all" : new Set(list.split(/[\s,]+/).filter(Boolean)); + const set = parseRuleSet(match[2]); + const line = idx + 1; if (match[1] === "file") { - fileRules = mergeRuleSet(fileRules, set); - } else { - // ci-ignore-next-line suppresses the finding on the following source line (1-based). - lineRules.set(idx + 2, mergeRuleSet(lineRules.get(idx + 2) ?? null, set)); + parsed.fileDirectives.push({ + directive: "ci-ignore-file", + file, + line, + rules: set, + suppressed: 0, + suppressedRuleIds: new Set(), + }); + parsed.directives.push(parsed.fileDirectives[parsed.fileDirectives.length - 1]); + return; } + + addLineDirective(parsed, { + directive: "ci-ignore-next-line", + file, + line, + targetLine: line + 1, + rules: set, + suppressed: 0, + suppressedRuleIds: new Set(), + }); + }); + + for (const directive of parseExpectedUnusedSuppressions(lines, file)) { + addLineDirective(parsed, directive); + } + + return parsed; +} + +function parseExpectedUnusedSuppressions(lines: string[], file: string): SuppressionDirectiveState[] { + const directives: SuppressionDirectiveState[] = []; + const seenBlocks = new Set(); + + lines.forEach((text, idx) => { + if (!text.includes("@expected-unused")) return; + const block = jsDocBlockBounds(lines, idx); + if (!block) return; + + const key = `${String(block.start)}:${String(block.end)}`; + if (seenBlocks.has(key)) return; + seenBlocks.add(key); + + const targetLine = nextCodeLine(lines, block.end + 1); + if (targetLine === null) return; + directives.push({ + directive: "@expected-unused", + file, + line: idx + 1, + targetLine, + rules: new Set(EXPECTED_UNUSED_RULES), + suppressed: 0, + suppressedRuleIds: new Set(), + }); }); - return { fileRules, lineRules }; + return directives; +} + +function jsDocBlockBounds(lines: string[], tagLineIndex: number): { start: number; end: number } | null { + let start = tagLineIndex; + while (start >= 0 && !lines[start].includes("/**")) { + if (lines[start].includes("*/")) return null; + start -= 1; + } + if (start < 0) return null; + + let end = start; + while (end < lines.length && !lines[end].includes("*/")) end += 1; + if (end >= lines.length || tagLineIndex > end) return null; + return { start, end }; +} + +function nextCodeLine(lines: string[], startIndex: number): number | null { + for (let idx = startIndex; idx < lines.length; idx += 1) { + const trimmed = lines[idx].trim(); + if (trimmed.length === 0) continue; + if (trimmed.startsWith("//") || trimmed.startsWith("/*") || trimmed.startsWith("*") || trimmed.startsWith("*/")) continue; + return idx + 1; + } + return null; } function ruleSetMatches(set: RuleSet | null | undefined, ruleId: string): boolean { @@ -109,6 +211,46 @@ function ruleSetMatches(set: RuleSet | null | undefined, ruleId: string): boolea return set === "all" || set.has(ruleId); } +function matchingDirective(supp: FileSuppressions, line: number, ruleId: string): SuppressionDirectiveState | null { + const lineDirectives = supp.lineDirectives.get(line) ?? []; + const lineDirective = lineDirectives.find((directive) => ruleSetMatches(directive.rules, ruleId)); + if (lineDirective) return lineDirective; + return supp.fileDirectives.find((directive) => ruleSetMatches(directive.rules, ruleId)) ?? null; +} + +function ruleSetToIds(set: RuleSet): string[] { + if (set === "all") return []; + return [...set].sort((left, right) => left.localeCompare(right)); +} + +function suppressionMessage(directive: SuppressionDirectiveState, status: CheckSuppression["status"]): string { + if (status === "stale") { + return `Stale suppression: ${directive.directive} does not suppress any finding`; + } + return `${directive.directive} suppressed ${String(directive.suppressed)} finding(s)`; +} + +function toCheckSuppression( + directive: SuppressionDirectiveState, + status: CheckSuppression["status"], +): CheckSuppression { + return { + directive: directive.directive, + status, + file: directive.file, + line: directive.line, + targetLine: directive.targetLine, + ruleIds: status === "active" ? ruleSetToIds(directive.suppressedRuleIds) : ruleSetToIds(directive.rules), + matchesAllRules: directive.rules === "all", + suppressed: directive.suppressed, + message: suppressionMessage(directive, status), + }; +} + +function staleSuppressionSeverity(config: CodebaseIntelligenceConfig): Severity { + return resolveSetting(config.rules?.[STALE_SUPPRESSION_RULE_ID], "warn").severity; +} + /** * Run every enabled rule and return findings with severity + fingerprint applied, * after dropping anything covered by ci-ignore suppressions. Deterministically sorted. @@ -118,12 +260,23 @@ export function runEngine( rules: Rule[], config: CodebaseIntelligenceConfig, ): Finding[] { + return runEngineWithSuppressions(ctx, rules, config).findings; +} + +/** + * Run enabled rules and return findings plus active/stale suppression metadata. + */ +export function runEngineWithSuppressions( + ctx: RuleContext, + rules: Rule[], + config: CodebaseIntelligenceConfig, +): EngineResult { const suppressionCache = new Map(); const suppressionsFor = (file: string): FileSuppressions => { const cached = suppressionCache.get(file); if (cached) return cached; const source = ctx.sourceOf(file); - const parsed = source ? parseSuppressions(source) : { fileRules: null, lineRules: new Map() }; + const parsed = source ? parseSuppressions(source, file) : { directives: [], fileDirectives: [], lineDirectives: new Map() }; suppressionCache.set(file, parsed); return parsed; }; @@ -147,8 +300,12 @@ export function runEngine( for (const r of reported) { const supp = suppressionsFor(r.file); - if (ruleSetMatches(supp.fileRules, rule.id)) continue; - if (ruleSetMatches(supp.lineRules.get(r.line), rule.id)) continue; + const directive = matchingDirective(supp, r.line, rule.id); + if (directive) { + directive.suppressed += 1; + directive.suppressedRuleIds.add(rule.id); + continue; + } out.push({ ruleId: rule.id, @@ -171,5 +328,39 @@ export function runEngine( out.sort( (a, b) => a.file.localeCompare(b.file) || a.line - b.line || a.ruleId.localeCompare(b.ruleId), ); - return out; + + for (const file of ctx.fileRelPaths) suppressionsFor(file); + + const staleSeverity = staleSuppressionSeverity(config); + const suppressions = [...suppressionCache.values()] + .flatMap((supp) => supp.directives) + .sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line || a.directive.localeCompare(b.directive)) + .map((directive) => { + const status: CheckSuppression["status"] = directive.suppressed > 0 ? "active" : "stale"; + if (status === "stale" && staleSeverity !== "off") { + const message = suppressionMessage(directive, status); + out.push({ + ruleId: STALE_SUPPRESSION_RULE_ID, + severity: staleSeverity === "error" ? "error" : "warn", + kind: "stale-suppression", + confidence: "high", + file: directive.file, + line: directive.line, + column: 1, + message, + evidence: [ + `directive=${directive.directive}`, + directive.targetLine ? `targetLine=${String(directive.targetLine)}` : "scope=file", + directive.rules === "all" ? "rules=all" : `rules=${ruleSetToIds(directive.rules).join(",")}`, + ], + fingerprint: fingerprint(STALE_SUPPRESSION_RULE_ID, directive.file, directive.line, message), + }); + } + return toCheckSuppression(directive, status); + }); + + out.sort( + (a, b) => a.file.localeCompare(b.file) || a.line - b.line || a.ruleId.localeCompare(b.ruleId), + ); + return { findings: out, suppressions }; } diff --git a/src/rules/format.ts b/src/rules/format.ts index 11c60dc..ee34b15 100644 --- a/src/rules/format.ts +++ b/src/rules/format.ts @@ -2,10 +2,14 @@ import type { CheckResult, OutputFormat } from "../types/index.js"; import { ALL_RULES } from "./registry.js"; const RULE_DESCRIPTIONS = new Map(ALL_RULES.map((r) => [r.id, r.meta.description])); +RULE_DESCRIPTIONS.set("no-stale-suppressions", "Report suppressions that no longer hide any finding."); -/** "N error(s), M warning(s) — VERDICT" — shared by text output and the CLI --summary flag. */ +/** Shared by text output and the CLI --summary flag. */ export function formatSummaryLine(result: CheckResult): string { - return `${String(result.summary.error)} error(s), ${String(result.summary.warn)} warning(s) — ${result.verdict.toUpperCase()}`; + const suppressionText = result.summary.suppressed > 0 || result.summary.staleSuppressions > 0 + ? `, ${String(result.summary.suppressed)} suppressed, ${String(result.summary.staleSuppressions)} stale suppression(s)` + : ""; + return `${String(result.summary.error)} error(s), ${String(result.summary.warn)} warning(s)${suppressionText} — ${result.verdict.toUpperCase()}`; } export function formatJson(result: CheckResult): string { @@ -14,6 +18,7 @@ export function formatJson(result: CheckResult): string { verdict: result.verdict, summary: result.summary, configPath: result.configPath, + suppressions: result.suppressions, findings: result.findings, }, null, diff --git a/src/types/index.ts b/src/types/index.ts index 0e54f4c..85b47d3 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -385,9 +385,26 @@ export interface Finding { fingerprint: string; } +export type SuppressionDirective = "ci-ignore-file" | "ci-ignore-next-line" | "@expected-unused"; +export type SuppressionStatus = "active" | "stale"; + +export interface CheckSuppression { + directive: SuppressionDirective; + status: SuppressionStatus; + file: string; + line: number; + targetLine?: number; + ruleIds: string[]; + matchesAllRules: boolean; + suppressed: number; + message: string; +} + export interface CheckSummary { error: number; warn: number; + suppressed: number; + staleSuppressions: number; rules: Record; } @@ -395,6 +412,7 @@ export type Verdict = "pass" | "warn" | "fail"; export interface CheckResult { findings: Finding[]; + suppressions: CheckSuppression[]; summary: CheckSummary; verdict: Verdict; configPath: string | null; diff --git a/tests/cli-check.e2e.test.ts b/tests/cli-check.e2e.test.ts index e9a4331..6b84258 100644 --- a/tests/cli-check.e2e.test.ts +++ b/tests/cli-check.e2e.test.ts @@ -220,6 +220,83 @@ describe("check command (e2e)", () => { expect(stdout).toContain("No findings."); }); + it("CH-P1-06: reports active and stale suppressions with JSDoc cleanup semantics", async () => { + const activeDir = makeProject( + { + "src/contracts.ts": [ + "/** @expected-unused */", + "type Intentional = { value: string };", + "/** @public */", + "export type PublicContract = { id: string };", + "/** @internal */", + "export type InternalDraft = { id: string };", + "", + ].join("\n"), + }, + { rules: { "no-circular-deps": "off", "no-dead-exports": "off", "no-unused-types": "warn" } }, + ); + const activeRun = await run(["check", activeDir, "--json"]); + expect(activeRun.status).toBe(0); + const active = JSON.parse(activeRun.stdout) as { + summary: { suppressed: number; staleSuppressions: number }; + suppressions: Array<{ directive: string; status: string; targetLine?: number; suppressed: number }>; + findings: Array<{ ruleId: string; message: string }>; + }; + expect(active.summary.suppressed).toBe(1); + expect(active.summary.staleSuppressions).toBe(0); + expect(active.suppressions).toContainEqual( + expect.objectContaining({ directive: "@expected-unused", status: "active", targetLine: 2, suppressed: 1 }), + ); + expect(active.findings.some((finding) => finding.message.includes("Intentional"))).toBe(false); + expect(active.findings.some((finding) => finding.message.includes("PublicContract"))).toBe(false); + expect(active.findings.some((finding) => finding.message.includes("InternalDraft"))).toBe(true); + + const activeSummary = await run(["check", activeDir, "--summary"]); + expect(activeSummary.status).toBe(0); + expect(activeSummary.stdout).toContain("1 suppressed"); + + const staleDir = makeProject( + { + "src/contracts.ts": [ + "/** @expected-unused */", + "type Used = { value: string };", + "export const value: Used = { value: 'ok' };", + "", + ].join("\n"), + }, + { rules: { "no-circular-deps": "off", "no-dead-exports": "off", "no-unused-types": "warn" } }, + ); + const staleRun = await run(["check", staleDir, "--json", "--fail-on", "warn"]); + expect(staleRun.status).toBe(1); + const stale = JSON.parse(staleRun.stdout) as { + summary: { warn: number; suppressed: number; staleSuppressions: number }; + suppressions: Array<{ directive: string; status: string; targetLine?: number; suppressed: number }>; + findings: Array<{ ruleId: string; kind?: string; message: string }>; + }; + expect(stale.summary.warn).toBe(1); + expect(stale.summary.suppressed).toBe(0); + expect(stale.summary.staleSuppressions).toBe(1); + expect(stale.suppressions).toContainEqual( + expect.objectContaining({ directive: "@expected-unused", status: "stale", targetLine: 2, suppressed: 0 }), + ); + expect(stale.findings).toContainEqual( + expect.objectContaining({ ruleId: "no-stale-suppressions", kind: "stale-suppression" }), + ); + + const staleSummary = await run(["check", staleDir, "--summary", "--fail-on", "warn"]); + expect(staleSummary.status).toBe(1); + expect(staleSummary.stdout).toContain("1 stale suppression(s)"); + + const staleSarifRun = await run(["check", staleDir, "--format", "sarif", "--fail-on", "warn"]); + expect(staleSarifRun.status).toBe(1); + const staleSarif = JSON.parse(staleSarifRun.stdout) as { + runs: Array<{ results: Array<{ ruleId: string; properties?: { kind?: string; evidence?: string[] } }> }>; + }; + const staleFinding = staleSarif.runs[0].results.find((finding) => finding.ruleId === "no-stale-suppressions"); + expect(staleFinding?.properties?.kind).toBe("stale-suppression"); + expect(staleFinding?.properties?.evidence).toContain("directive=@expected-unused"); + }, 90_000); + it("default failOn=error keeps a warn-only project green (exit 0)", async () => { const dir = makeProject(DEAD, { rules: { "no-circular-deps": "off" } }); const { status } = await run(["check", dir]); diff --git a/tests/rules-engine.test.ts b/tests/rules-engine.test.ts index a599041..60784e5 100644 --- a/tests/rules-engine.test.ts +++ b/tests/rules-engine.test.ts @@ -99,13 +99,26 @@ describe("no-comments rule", () => { describe("suppressions", () => { it("ci-ignore-next-line suppresses the following line", () => { - const findings = project( + const result = projectResult( { "src/x.ts": "export const x = 1;\n// ci-ignore-next-line no-comments\nexport const y = x; // hidden\n", }, { rules: { "no-comments": "error", "no-dead-exports": "off" } }, ); + const findings = result.findings; expect(ids(findings)).not.toContain("no-comments"); + expect(result.suppressions).toMatchObject([ + { + directive: "ci-ignore-next-line", + status: "active", + file: "src/x.ts", + line: 2, + targetLine: 3, + ruleIds: ["no-comments"], + suppressed: 1, + }, + ]); + expect(result.summary.suppressed).toBe(1); }); it("ci-ignore-file suppresses the whole file", () => { @@ -115,6 +128,92 @@ describe("suppressions", () => { ); expect(ids(findings)).not.toContain("no-comments"); }); + + it("reports stale ci-ignore suppressions as warnings", () => { + const result = projectResult( + { + "src/x.ts": "export const x = 1;\n// ci-ignore-next-line no-comments\nexport const y = x;\n", + }, + { rules: { "no-comments": "error", "no-dead-exports": "off" } }, + ); + expect(result.suppressions[0]).toMatchObject({ + directive: "ci-ignore-next-line", + status: "stale", + file: "src/x.ts", + line: 2, + targetLine: 3, + ruleIds: ["no-comments"], + suppressed: 0, + }); + expect(result.findings).toEqual([ + expect.objectContaining({ + ruleId: "no-stale-suppressions", + severity: "warn", + kind: "stale-suppression", + file: "src/x.ts", + line: 2, + }), + ]); + expect(result.summary.staleSuppressions).toBe(1); + }); + + it("@expected-unused suppresses declaration cleanup and becomes stale when the type is used", () => { + const active = projectResult( + { + "src/x.ts": "/** @expected-unused */\ntype Intentional = { value: string };\n", + }, + { rules: { "no-circular-deps": "off", "no-dead-exports": "off", "no-unused-types": "warn" } }, + ); + expect(active.findings.some((finding) => finding.message.includes("Intentional"))).toBe(false); + expect(active.suppressions[0]).toMatchObject({ + directive: "@expected-unused", + status: "active", + file: "src/x.ts", + line: 1, + targetLine: 2, + ruleIds: ["no-unused-types"], + suppressed: 1, + }); + + const stale = projectResult( + { + "src/x.ts": "/** @expected-unused */\ntype Used = { value: string };\nexport const value: Used = { value: 'ok' };\n", + }, + { rules: { "no-circular-deps": "off", "no-dead-exports": "off", "no-unused-types": "warn" } }, + ); + expect(stale.findings).toEqual([ + expect.objectContaining({ ruleId: "no-stale-suppressions", file: "src/x.ts", line: 1 }), + ]); + expect(stale.suppressions[0]).toMatchObject({ directive: "@expected-unused", status: "stale" }); + }); + + it("@expected-unused is only parsed inside JSDoc blocks", () => { + const result = projectResult( + { + "src/x.ts": "export const marker = '@expected-unused';\n// @expected-unused is docs text, not a directive\nexport const ok = marker;\n", + }, + { rules: { "no-circular-deps": "off", "no-dead-exports": "off", "no-unused-types": "warn" } }, + ); + expect(result.suppressions).toEqual([]); + expect(ids(result.findings)).not.toContain("no-stale-suppressions"); + }); + + it("@public protects exported type declarations while @internal remains checkable", () => { + const findings = project( + { + "src/contracts.ts": [ + "/** @public */", + "export type PublicContract = { id: string };", + "/** @internal */", + "export type InternalDraft = { id: string };", + "", + ].join("\n"), + }, + { rules: { "no-circular-deps": "off", "no-dead-exports": "off", "no-unused-types": "warn" } }, + ); + expect(findings.some((finding) => finding.message.includes("PublicContract"))).toBe(false); + expect(findings.some((finding) => finding.message.includes("InternalDraft"))).toBe(true); + }); }); describe("graph-backed rules", () => {