From 4eb4791d2db12bc9b8774c3c5d898c5a9a652424 Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Tue, 14 Jul 2026 15:22:29 +1000 Subject: [PATCH 1/7] docs: design spec for --max-warnings on unused-code and duplicate-code --- ...14-max-warnings-unused-duplicate-design.md | 202 ++++++++++++++++++ 1 file changed, 202 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md diff --git a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md new file mode 100644 index 0000000..27ded6d --- /dev/null +++ b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md @@ -0,0 +1,202 @@ +# Design: `--max-warnings` for `unused-code` and `duplicate-code` + +**Date:** 2026-07-14 +**Status:** Approved + +## Summary + +Add a `--max-warnings ` flag to the `unused-code` (knip) and `duplicate-code` +(jscpd) checks. It is an ESLint-style tolerance gate: the check passes while the +number of findings stays at or below `n`, and fails when it exceeds `n`. This lets +a project adopt either check on a codebase that already has debt — set the budget +at (or just below) the current count and ratchet it down over time, without letting +new findings slip in. + +## Motivation + +Today both checks are all-or-nothing: knip fails if there is *any* unused +file/export/dependency, and jscpd fails on *any* duplication. On an existing +codebase that means the check is red from day one and stays red until every finding +is fixed, so teams tend not to turn it on at all. A tolerance budget makes gradual +adoption possible. + +Neither underlying tool has a native count-based gate: + +- **knip** exits non-zero whenever there is any finding; it has no "max issues" flag. +- **jscpd** only offers `--threshold ` (a duplication *percentage*), not a + count of clones. + +So `verifyx` itself must count findings from each tool's machine-readable output and +decide pass/fail. That is the core of this design. + +## Behavior + +- New flag `--max-warnings ` on `verifyx unused-code` and `verifyx duplicate-code`. + `n` is a non-negative integer. +- **Without the flag:** unchanged — the current exit-code path, zero tolerance. +- **With the flag:** `verifyx` runs the tool in a machine-readable "count" mode, + tallies findings, and: + - `count <= n` → **pass**, silent (per the project's quiet-on-success rule). + - `count > n` → **fail**, printing a one-line count summary, then the tool's normal + findings report, then the existing external-failure hint. +- **Count unit:** + - `unused-code` (knip): total unused *items* — every unused file, export, exported + type, and dependency, summed into a single number. + - `duplicate-code` (jscpd): the number of detected duplicate *clones*. + +## How it flows through `verifyx` + +`--max-warnings` is a subcommand flag, so it rides the existing curated-gate pattern. +A consumer wires a budget into their `package.json`: + +```jsonc +{ + "scripts": { + "verify:unused-code": "verifyx unused-code --max-warnings 5", + "verify:duplicate-code": "verifyx duplicate-code --max-warnings 10" + } +} +``` + +- **Bare `verifyx`** (the curated gate) runs these `verify:*` scripts verbatim, so the + budget applies. +- **`verifyx all`** runs each built-in via `runDefault()` with *no* arguments, so a + built-in with no override runs at its **default: zero tolerance** (today's behavior). + A `verify:` script that matches a built-in **overrides** it under `verifyx all`, + so the same one-line script above is also how you apply a budget in "run everything" + mode. `verifyx all` never adds flags on its own. +- **Passthrough is preserved:** `verifyx unused-code --max-warnings 5 -- --production` + forwards `--production` to knip, and `--max-warnings` is consumed by `verifyx`. + +## Implementation + +The design reuses `defineExternalCheck` rather than adding bespoke check types. + +### `ExternalCheckSpec` gains an optional counting capability + +Only `unused-code` and `duplicate-code` set it; every other external check is untouched +and rejects `--max-warnings` (the flag is not registered for them). + +```ts +export type MaxWarningsSupport = { + /** Run the tool in machine-readable mode and return the total finding count. */ + count: (ctx: { extraArgs: string[]; env: Record }) => Promise +} +``` + +`ExternalCheckSpec` gets `maxWarnings?: MaxWarningsSupport`. + +### `RunDefaultOptions` gains `maxWarnings?: number` + +```ts +export type RunDefaultOptions = { + extraArgs?: string[] + /** When set, the check counts findings and passes iff count <= maxWarnings (external checks that support it). */ + maxWarnings?: number +} +``` + +### `runDefault` branch + +`defineExternalCheck`'s `runDefault` takes the counting path **only** when +`maxWarnings` is provided *and* the spec has a `maxWarnings` capability; otherwise it +runs exactly as today. The bin-present and `canRun` guards run first, unchanged (so a +missing tool still skips gracefully before any counting). + +Counting path: + +1. `count = await spec.maxWarnings.count({ extraArgs, env: envWithLocalBin() })`. +2. `count <= maxWarnings` → return `{ ok: true }`, print nothing. +3. `count > maxWarnings` → print a one-line summary (e.g. + `duplicate-code: 12 clones found, exceeds --max-warnings 10`), then re-run the + tool's normal `checkCommand` (buffered) so the familiar findings report is shown, + then the existing `externalFailureHint`. Return `{ ok: false }`. + +In count mode the tool's own exit code is **ignored** for the verdict (knip exits +non-zero on any finding, even within budget); the verdict is purely the parsed count. + +### New spawn helper + +`runCommand` currently returns only the exit code and emits output to the buffer. Add a +sibling that returns captured stdout for parsing, leaving `runCommand` untouched: + +```ts +export function captureCommand(command: string, opts?: RunCommandOptions): Promise<{ code: number; stdout: string }> +``` + +### Per-tool count functions + +Extracted as exported **pure parsers** so they are unit-testable without the real tools; +the `count` functions wire the parser to a real invocation. + +- **knip** — `knip --reporter json --no-progress [extraArgs]`, capture stdout, then + `countKnipFindings(json)`: `files.length` plus the summed lengths of every issue array + across `issues[]` (dependencies, devDependencies, optionalPeerDependencies, unlisted, + binaries, unresolved, exports, nsExports, types, nsTypes, enumMembers, duplicates, + and any other array-valued issue fields). The exact knip JSON shape is confirmed + against the installed knip during implementation and pinned with a fixture test. +- **jscpd** — jscpd has no stdout JSON reporter, so run it with the JSON reporter into a + fresh OS temp dir (`fs.mkdtemp` under `os.tmpdir()`), read `jscpd-report.json`, then + `countJscpdClones(json)`: `statistics.total.clones` (falling back to + `duplicates.length`). The temp dir is removed afterward (including on error). jscpd's + console reporters are suppressed for the count run. + +### `registerChecks` + +`unused-code` and `duplicate-code` are registered explicitly (out of the generic +external loop) so they can declare `--max-warnings ` alongside the existing +`[toolArgs...]` passthrough: + +```ts +program + .command('unused-code') + .description(...) + .option('--max-warnings ', 'tolerate up to n findings before failing', Number) + .argument('[toolArgs...]', 'extra arguments passed through to the underlying tool (after `--`)') + .action(async (toolArgs, opts) => { + finish((await getCheck('unused-code')!.runDefault({ extraArgs: toolArgs, maxWarnings: opts.maxWarnings })).ok) + }) +``` + +All other external checks keep the generic registration loop. + +## Errors and edge cases + +- **Invalid `n`** (negative, non-integer, or `NaN` from a non-numeric value) → a clear + error message and non-zero exit, before running any tool. +- **`--max-warnings 0`** is valid and equivalent to today's zero-tolerance behavior + (routed through the count path). +- **Count command crashes or emits unparseable output** → fail loudly (never silently + pass): surface the raw output and the failure hint. A parse failure is a check failure. +- **Tool not installed / `canRun` false** → skip, exactly as today (guards run before + counting). + +## Docs and programmatic API + +- **README:** document `--max-warnings` under the `unused-code` and `duplicate-code` + sections — the count semantics per tool, the `verify:` override needed for + `verifyx all`, and that the default is zero tolerance. Note it in the built-in-checks + overview where relevant. +- **Programmatic:** `getCheck('unused-code')?.runDefault({ maxWarnings: 5 })` works via + the existing registry entry point; no new export beyond the count parsers. + +## Testing + +Following the existing `external.test.ts` pure-function style: + +- `countKnipFindings` against a JSON fixture with mixed issue types → expected total. +- `countJscpdClones` against a JSON fixture → expected clone count. +- The decision logic: `count <= n` → ok, `count > n` → fail (spec with a stubbed + `count`), including the `n = 0` boundary. +- `--max-warnings` argument validation (rejects negative / non-integer). + +The `count` functions themselves (which spawn the real tools) are covered indirectly; +the parsers hold the logic that needs pinning. + +## Out of scope + +- Applying a budget automatically under `verifyx all` without a `verify:` override. +- Persisting the budget in `verify.config.json` (external checks are configured through + their own tool config; the flag lives on the script, consistent with the existing model). +- Per-category budgets for knip (a single total count is the agreed unit). +- Adding `--max-warnings` to other external checks. From 113b76009c2f47c3765b83512b9053d9ef44352d Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Tue, 14 Jul 2026 15:48:44 +1000 Subject: [PATCH 2/7] feat(checks): add --max-warnings budget to unused-code and duplicate-code Both checks gain an ESLint-style `--max-warnings ` tolerance flag: verifyx counts findings from the tool's machine-readable output and passes while the count stays at or below n, failing only when it exceeds n. Without the flag both stay zero-tolerance, exactly as before. - unused-code (knip): counts every unused item (files, exports, types, deps) via the JSON reporter on stdout. - duplicate-code (jscpd): counts clones via a JSON report written to a temp dir (jscpd has no stdout JSON reporter), reusing the check command's scan config. Reuses defineExternalCheck: a per-tool maxWarnings capability on the spec, a runDefault counting branch, and a captureCommand spawn helper. On over-budget the check prints a count summary, the tool's normal report, and the usual hint; a counting error fails loudly rather than passing. Invalid n is rejected with a clean CLI error. The flag flows through the curated gate and verify: overrides; verifyx all with no override stays zero-tolerance. --- README.md | 15 +++++ ...14-max-warnings-unused-duplicate-design.md | 18 ++--- src/checks/external.test.ts | 43 +++++++++++- src/checks/external.ts | 59 +++++++++++++++-- src/checks/maxWarnings.test.ts | 65 +++++++++++++++++++ src/checks/maxWarnings.ts | 65 +++++++++++++++++++ src/checks/registry.ts | 3 + src/checks/types.ts | 7 ++ src/commands/registerChecks.test.ts | 27 ++++++++ src/commands/registerChecks.ts | 21 ++++-- src/shared/spawn.test.ts | 22 +++++++ src/shared/spawn.ts | 28 ++++++++ 12 files changed, 349 insertions(+), 24 deletions(-) create mode 100644 src/checks/maxWarnings.test.ts create mode 100644 src/checks/maxWarnings.ts create mode 100644 src/commands/registerChecks.test.ts create mode 100644 src/shared/spawn.test.ts diff --git a/README.md b/README.md index befd121..d3f39c0 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,18 @@ Each external check is configured through its **tool's own config file**, exactl - `circular-deps`: [skott options](https://github.com/antoine-coulon/skott) - `duplicate-code`: [`.jscpd.json` or `package.json#jscpd`](https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config) +**Tolerating findings with `--max-warnings`.** `unused-code` and `duplicate-code` accept `--max-warnings `, an ESLint-style tolerance budget: the check counts its findings and passes while the count stays **at or below** `n`, failing only when it exceeds `n`. It's for turning a check on over a codebase that already has debt — pin the budget at the current count and ratchet it down, without letting new findings slip in. Neither tool has a native count gate, so `verifyx` counts for them (from knip's JSON reporter and jscpd's JSON report). + +- `unused-code` counts every unused item knip reports (files, exports, exported types, dependencies) as one. +- `duplicate-code` counts each duplicate clone jscpd reports as one. + +```sh +verifyx unused-code --max-warnings 5 +verifyx duplicate-code --max-warnings 10 +``` + +Without the flag both checks stay **zero-tolerance** (fail on any finding), exactly as before. When the budget is exceeded the check prints how many findings it found, the tool's normal report, and the usual failure hint. Because it's a flag on the script, it applies wherever the script carries it: put it in a `verify:` script and both the bare `verifyx` gate and `verifyx all` pick it up (a `verify:` script [overrides](#verifyx-all-run-everything) the matching built-in). Under `verifyx all` with no such script, the built-in runs at its zero-tolerance default. Passthrough still composes: `verifyx unused-code --max-warnings 5 -- --production`. + ### `complexity` ```sh @@ -316,6 +328,9 @@ const { failing } = analyzeComplexity({ pattern: 'src/**/*.ts', threshold: 50 }) // Run any single check by name, including the ones that shell out to an external tool. const lint = await getCheck('lint')?.runDefault() + +// unused-code / duplicate-code accept a max-warnings budget (pass iff findings <= maxWarnings). +const unused = await getCheck('unused-code')?.runDefault({ maxWarnings: 5 }) ``` Native checks also expose a direct runner (`runComplexity`, `runComments`, `runHardcodedColors`, `runForbiddenStrings`); external checks (`lint`, `format`, `check-types`, `unused-code`, `circular-deps`, `duplicate-code`) have no standalone function and are run via the registry (`getCheck(name)?.runDefault()`) or the orchestrators. diff --git a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md index 27ded6d..fbdfda3 100644 --- a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md +++ b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md @@ -14,8 +14,8 @@ new findings slip in. ## Motivation -Today both checks are all-or-nothing: knip fails if there is *any* unused -file/export/dependency, and jscpd fails on *any* duplication. On an existing +Today both checks are all-or-nothing: knip fails if there is _any_ unused +file/export/dependency, and jscpd fails on _any_ duplication. On an existing codebase that means the check is red from day one and stays red until every finding is fixed, so teams tend not to turn it on at all. A tolerance budget makes gradual adoption possible. @@ -23,7 +23,7 @@ adoption possible. Neither underlying tool has a native count-based gate: - **knip** exits non-zero whenever there is any finding; it has no "max issues" flag. -- **jscpd** only offers `--threshold ` (a duplication *percentage*), not a +- **jscpd** only offers `--threshold ` (a duplication _percentage_), not a count of clones. So `verifyx` itself must count findings from each tool's machine-readable output and @@ -40,9 +40,9 @@ decide pass/fail. That is the core of this design. - `count > n` → **fail**, printing a one-line count summary, then the tool's normal findings report, then the existing external-failure hint. - **Count unit:** - - `unused-code` (knip): total unused *items* — every unused file, export, exported + - `unused-code` (knip): total unused _items_ — every unused file, export, exported type, and dependency, summed into a single number. - - `duplicate-code` (jscpd): the number of detected duplicate *clones*. + - `duplicate-code` (jscpd): the number of detected duplicate _clones_. ## How it flows through `verifyx` @@ -53,14 +53,14 @@ A consumer wires a budget into their `package.json`: { "scripts": { "verify:unused-code": "verifyx unused-code --max-warnings 5", - "verify:duplicate-code": "verifyx duplicate-code --max-warnings 10" - } + "verify:duplicate-code": "verifyx duplicate-code --max-warnings 10", + }, } ``` - **Bare `verifyx`** (the curated gate) runs these `verify:*` scripts verbatim, so the budget applies. -- **`verifyx all`** runs each built-in via `runDefault()` with *no* arguments, so a +- **`verifyx all`** runs each built-in via `runDefault()` with _no_ arguments, so a built-in with no override runs at its **default: zero tolerance** (today's behavior). A `verify:` script that matches a built-in **overrides** it under `verifyx all`, so the same one-line script above is also how you apply a budget in "run everything" @@ -99,7 +99,7 @@ export type RunDefaultOptions = { ### `runDefault` branch `defineExternalCheck`'s `runDefault` takes the counting path **only** when -`maxWarnings` is provided *and* the spec has a `maxWarnings` capability; otherwise it +`maxWarnings` is provided _and_ the spec has a `maxWarnings` capability; otherwise it runs exactly as today. The bin-present and `canRun` guards run first, unchanged (so a missing tool still skips gracefully before any counting). diff --git a/src/checks/external.test.ts b/src/checks/external.test.ts index 62d8a69..b176e1a 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -1,6 +1,6 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' -import { appendArgs, defineExternalCheck, externalFailureHint, selectCommand } from './external.ts' +import { appendArgs, defineExternalCheck, externalFailureHint, runWithMaxWarnings, selectCommand } from './external.ts' const fixable = { checkCommand: 'oxfmt --check .', fixCommand: 'oxfmt .' } const notFixable = { checkCommand: 'tsc --noEmit' } @@ -79,3 +79,42 @@ describe('defineExternalCheck', () => { expect(knip.scaffold.script).toBe('verifyx unused-code') }) }) + +describe('runWithMaxWarnings', () => { + const baseSpec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } + + it('passes when the finding count is at or below the budget, without running the display command', async () => { + const spec = { ...baseSpec, maxWarnings: { unit: 'clone', count: async () => 5 } } + expect(await runWithMaxWarnings(spec, 5, [], {})).toEqual({ name: 'duplicate-code', ok: true }) + }) + + it('fails when the finding count exceeds the budget', async () => { + const spec = { ...baseSpec, checkCommand: 'node -e ""', maxWarnings: { unit: 'clone', count: async () => 6 } } + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + expect((await runWithMaxWarnings(spec, 5, [], {})).ok).toBe(false) + } finally { + spy.mockRestore() + } + }) + + it('fails loudly when counting throws instead of silently passing', async () => { + const spec = { + ...baseSpec, + maxWarnings: { + unit: 'clone', + count: async () => { + throw new Error('knip crashed') + }, + }, + } + const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) + try { + const result = await runWithMaxWarnings(spec, 5, [], {}) + expect(result.ok).toBe(false) + expect(spy).toHaveBeenCalled() + } finally { + spy.mockRestore() + } + }) +}) diff --git a/src/checks/external.ts b/src/checks/external.ts index 9baec11..f194fc6 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -3,9 +3,13 @@ import path from 'node:path' import { color } from '../shared/color.ts' import { resolveMode } from '../shared/mode.ts' -import { runCommand } from '../shared/spawn.ts' +import { emit } from '../shared/output.ts' +import { appendArgs, captureCommand, runCommand } from '../shared/spawn.ts' +import { type MaxWarningsSupport, withinBudget } from './maxWarnings.ts' import type { Check, CheckMode, CheckResult, RunDefaultOptions } from './types.ts' +export { appendArgs } from '../shared/spawn.ts' + const BIN_EXTENSIONS = ['', '.cmd', '.ps1', '.exe'] /** True when a project-local binary is installed under node_modules/.bin (cross-platform). */ @@ -46,11 +50,8 @@ export type ExternalCheckSpec = { * script so a consumer can see and tweak them. Not baked into `runDefault`; only the scaffolded script carries them. */ scaffoldArgs?: string -} - -/** Append user/scaffold `extraArgs` to an external tool's command, preserving shell semantics (globs unquoted). */ -export function appendArgs(command: string, extraArgs: readonly string[] = []): string { - return extraArgs.length > 0 ? `${command} ${extraArgs.join(' ')}` : command + /** Present only for checks that support `--max-warnings`; defines how to count findings for the tolerance gate. */ + maxWarnings?: MaxWarningsSupport } /** Pick the command for the run mode: the fix command only in fix mode and only when the check is fixable. */ @@ -67,6 +68,46 @@ export function externalFailureHint(spec: Pick & { + maxWarnings: MaxWarningsSupport +} + +/** + * Run a countable external check under a `--max-warnings` budget: count findings via the tool's machine-readable + * output and pass while the count stays at or below the budget. On over-budget, print a one-line summary, then the + * tool's normal report (re-run and shown regardless of its own exit code), then the failure hint. A counting error + * fails loudly rather than passing silently. + */ +export async function runWithMaxWarnings( + spec: CountableSpec, + maxWarnings: number, + extraArgs: string[], + env: Record, +): Promise { + let count: number + try { + count = await spec.maxWarnings.count({ extraArgs, env, checkCommand: spec.checkCommand }) + } catch (error) { + const docs = spec.docs ? ` — ${spec.docs}` : '' + console.error( + color.dim( + `↳ ${spec.name}: could not count ${spec.bin} findings for --max-warnings (${String(error)}). Configure ${spec.bin}${docs}.`, + ), + ) + return { name: spec.name, ok: false } + } + if (withinBudget(count, maxWarnings)) return { name: spec.name, ok: true } + + const unit = count === 1 ? spec.maxWarnings.unit : `${spec.maxWarnings.unit}s` + console.error(color.dim(`↳ ${spec.name}: ${count} ${unit} found, exceeds --max-warnings ${maxWarnings}.`)) + const command = appendArgs(selectCommand(spec, resolveMode()), extraArgs) + const { stdout, stderr } = await captureCommand(command, { env }) + const report = stdout || stderr + if (report) emit(spec.transformOutput ? spec.transformOutput(report) : report) + console.error(color.dim(externalFailureHint(spec, command))) + return { name: spec.name, ok: false } +} + /** Build a Check that shells out to an external tool, skipping gracefully when the tool cannot run. */ export function defineExternalCheck(spec: ExternalCheckSpec): Check { return { @@ -74,6 +115,7 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { description: spec.description, kind: 'external', recommended: spec.recommended ?? false, + supportsMaxWarnings: !!spec.maxWarnings, // Scaffold as a call into this CLI so fix-vs-check lives in one place, not the consumer's script. scaffold: { script: spec.scaffoldArgs ? `verifyx ${spec.name} -- ${spec.scaffoldArgs}` : `verifyx ${spec.name}`, @@ -81,7 +123,7 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { }, // `verifyx eject ` inlines these raw commands into the consumer's verify:* scripts. eject: { check: spec.checkCommand, fix: spec.fixCommand }, - async runDefault({ extraArgs }: RunDefaultOptions = {}): Promise { + async runDefault({ extraArgs, maxWarnings }: RunDefaultOptions = {}): Promise { if (!hasLocalBin(spec.bin)) { console.log(color.dim(`${spec.name}: ${spec.bin} not installed — skipping (add it with \`npx verifyx init\`)`)) return { name: spec.name, ok: true, skipped: true } @@ -90,6 +132,9 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { console.log(color.dim(`${spec.name}: not applicable here — skipping`)) return { name: spec.name, ok: true, skipped: true } } + if (maxWarnings !== undefined && spec.maxWarnings) { + return runWithMaxWarnings({ ...spec, maxWarnings: spec.maxWarnings }, maxWarnings, extraArgs ?? [], envWithLocalBin()) + } const command = appendArgs(selectCommand(spec, resolveMode()), extraArgs) // quiet: buffer the tool's output and flush only on failure (streamed live under --verbose). const code = await runCommand(command, { env: envWithLocalBin(), quiet: true, transform: spec.transformOutput }) diff --git a/src/checks/maxWarnings.test.ts b/src/checks/maxWarnings.test.ts new file mode 100644 index 0000000..a0e6b78 --- /dev/null +++ b/src/checks/maxWarnings.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from 'vitest' + +import { countJscpdClones, countKnipFindings, withinBudget } from './maxWarnings.ts' + +describe('countKnipFindings', () => { + it('sums the lengths of every finding array across all issue rows', () => { + const report = { + issues: [ + { file: 'src/unused.ts', files: ['src/unused.ts'], exports: [], dependencies: [] }, + { + file: 'src/foo.ts', + exports: [{ name: 'a' }, { name: 'b' }], + types: [{ name: 'T' }], + duplicates: [[{ name: 'x' }, { name: 'y' }]], + }, + { file: 'package.json', dependencies: [{ name: 'lodash' }], devDependencies: [{ name: 'jest' }], unlisted: [{ name: 'react' }] }, + ], + } + expect(countKnipFindings(report)).toBe(8) + }) + + it('excludes the file string and owners metadata from the count', () => { + const report = { issues: [{ file: 'src/foo.ts', owners: [{ name: 'team-a' }, { name: 'team-b' }], exports: [{ name: 'unused' }] }] } + expect(countKnipFindings(report)).toBe(1) + }) + + it('returns 0 for a clean report', () => { + expect(countKnipFindings({ issues: [] })).toBe(0) + }) + + it('returns 0 when the issues key is absent', () => { + expect(countKnipFindings({})).toBe(0) + }) +}) + +describe('countJscpdClones', () => { + it('reads the total clone count from statistics', () => { + const report = { duplicates: [{}, {}, {}], statistics: { total: { clones: 3 } } } + expect(countJscpdClones(report)).toBe(3) + }) + + it('falls back to the duplicates array length when statistics are missing', () => { + expect(countJscpdClones({ duplicates: [{}, {}] })).toBe(2) + }) + + it('returns 0 for a clean report', () => { + expect(countJscpdClones({ duplicates: [], statistics: { total: { clones: 0 } } })).toBe(0) + }) +}) + +describe('withinBudget', () => { + it('passes when the count is at or below the budget (budget is inclusive)', () => { + expect(withinBudget(3, 5)).toBe(true) + expect(withinBudget(5, 5)).toBe(true) + }) + + it('fails when the count exceeds the budget', () => { + expect(withinBudget(6, 5)).toBe(false) + }) + + it('treats a budget of 0 as zero tolerance', () => { + expect(withinBudget(0, 0)).toBe(true) + expect(withinBudget(1, 0)).toBe(false) + }) +}) diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts new file mode 100644 index 0000000..0b1ac2a --- /dev/null +++ b/src/checks/maxWarnings.ts @@ -0,0 +1,65 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +import { appendArgs, captureCommand } from '../shared/spawn.ts' + +/** Context handed to a counting run: the user's passthrough args, the PATH-augmented env, and the check's own command. */ +type MaxWarningsCountContext = { extraArgs: string[]; env: Record; checkCommand: string } + +/** Opt-in capability that lets an external check tolerate up to `--max-warnings` findings by counting them itself. */ +export type MaxWarningsSupport = { + /** Singular label for one finding, pluralised in the over-budget summary, e.g. 'clone' → '6 clones found'. */ + unit: string + /** Run the tool in machine-readable mode and return the total finding count. */ + count: (ctx: MaxWarningsCountContext) => Promise +} + +/** knip's JSON reporter attaches ownership metadata (not a finding) under this key; exclude it from the count. */ +const KNIP_NON_FINDING_KEYS = new Set(['owners']) + +/** + * Total unused items in a knip `--reporter json` report: every finding array (files, exports, types, + * dependencies, duplicate groups, …) summed across all issue rows. Robust to knip adding new issue types, + * since any array-valued field counts; only ownership metadata and the row's `file` string are excluded. + */ +export function countKnipFindings(report: { issues?: Array> }): number { + let total = 0 + for (const row of report.issues ?? []) { + for (const [key, value] of Object.entries(row)) { + if (!KNIP_NON_FINDING_KEYS.has(key) && Array.isArray(value)) total += value.length + } + } + return total +} + +/** Number of duplicate clones in a jscpd JSON report: the authoritative total, else the duplicates array length. */ +export function countJscpdClones(report: { statistics?: { total?: { clones?: number } }; duplicates?: unknown[] }): number { + return report.statistics?.total?.clones ?? report.duplicates?.length ?? 0 +} + +/** A counted check passes while findings stay at or below the budget; the budget itself is tolerated (inclusive). */ +export function withinBudget(count: number, maxWarnings: number): boolean { + return count <= maxWarnings +} + +/** Count knip's findings by asking it for a JSON report on stdout (config-hint errors are irrelevant to the count). */ +export async function knipCount(ctx: MaxWarningsCountContext): Promise { + const command = `${appendArgs('knip --no-progress', ctx.extraArgs)} --reporter json` + const { stdout } = await captureCommand(command, { env: ctx.env }) + return countKnipFindings(JSON.parse(stdout)) +} + +/** + * Count jscpd's clones. jscpd has no stdout JSON reporter, so run the check command (reusing its scan config) with a + * JSON reporter written into a throwaway temp dir; the appended reporter flags override the command's console reporter. + */ +export async function jscpdCount(ctx: MaxWarningsCountContext): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verifyx-jscpd-')) + try { + await captureCommand(`${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}" --silent`, { env: ctx.env }) + return countJscpdClones(JSON.parse(fs.readFileSync(path.join(dir, 'jscpd-report.json'), 'utf8'))) + } finally { + fs.rmSync(dir, { recursive: true, force: true }) + } +} diff --git a/src/checks/registry.ts b/src/checks/registry.ts index b1638c0..c7640ff 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -6,6 +6,7 @@ import { runComplexity } from './complexity.ts' import { defineExternalCheck } from './external.ts' import { runForbiddenStrings } from './forbidden-strings.ts' import { runHardcodedColors } from './hardcoded-colors.ts' +import { jscpdCount, knipCount } from './maxWarnings.ts' import type { Check, CheckResult } from './types.ts' function nativeCheck(name: string, description: string, recommended: boolean, run: () => CheckResult, script = `verifyx ${name}`): Check { @@ -69,6 +70,7 @@ export const CHECKS: Check[] = [ checkCommand: 'knip --no-progress --treat-config-hints-as-errors', devDeps: ['knip'], docs: 'https://knip.dev/reference/configuration', + maxWarnings: { unit: 'unused item', count: knipCount }, }), defineExternalCheck({ name: 'circular-deps', @@ -90,6 +92,7 @@ export const CHECKS: Check[] = [ // NO_COLOR/FORCE_COLOR, so strip the red foreground from its output; the table renders in the default colour. transformOutput: withoutRed, docs: 'https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config', + maxWarnings: { unit: 'clone', count: jscpdCount }, }), ] diff --git a/src/checks/types.ts b/src/checks/types.ts index bc43239..b86897a 100644 --- a/src/checks/types.ts +++ b/src/checks/types.ts @@ -15,6 +15,11 @@ export type CheckResult = { export type RunDefaultOptions = { /** Extra arguments appended verbatim to an external check's underlying command (e.g. `verifyx circular-deps -- src/*.ts`). */ extraArgs?: string[] + /** + * When set, an external check that supports counting (currently `unused-code`, `duplicate-code`) tolerates up to + * this many findings, failing only when the count exceeds it. Ignored by checks without a counting capability. + */ + maxWarnings?: number } /** A single verification. Native checks run in-process; external checks shell out to a tool. */ @@ -24,6 +29,8 @@ export type Check = { kind: CheckKind /** Whether `verifyx init` preselects this check as a recommended default. */ recommended: boolean + /** Whether this check accepts `--max-warnings ` (counts findings and tolerates up to n). External checks only. */ + supportsMaxWarnings?: boolean /** Run the check with its default options and print its own report. Resolves to the outcome. */ runDefault: (options?: RunDefaultOptions) => Promise /** How `verify init` wires this check into a consuming project. */ diff --git a/src/commands/registerChecks.test.ts b/src/commands/registerChecks.test.ts new file mode 100644 index 0000000..f7c5fd9 --- /dev/null +++ b/src/commands/registerChecks.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' + +import { parseMaxWarnings } from './registerChecks.ts' + +describe('parseMaxWarnings', () => { + it('parses a non-negative integer', () => { + expect(parseMaxWarnings('0')).toBe(0) + expect(parseMaxWarnings('5')).toBe(5) + expect(parseMaxWarnings(' 7 ')).toBe(7) + }) + + it('rejects a negative number', () => { + expect(() => parseMaxWarnings('-1')).toThrow(/non-negative integer/) + }) + + it('rejects a non-integer', () => { + expect(() => parseMaxWarnings('2.5')).toThrow(/non-negative integer/) + }) + + it('rejects a non-numeric value', () => { + expect(() => parseMaxWarnings('abc')).toThrow(/non-negative integer/) + }) + + it('rejects an empty value', () => { + expect(() => parseMaxWarnings('')).toThrow(/non-negative integer/) + }) +}) diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index c1b1dbe..51c9528 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -1,4 +1,4 @@ -import type { Command } from 'commander' +import { type Command, InvalidArgumentError } from 'commander' import { runComments } from '../checks/comments.ts' import { runComplexity } from '../checks/complexity.ts' @@ -14,6 +14,12 @@ function collect(value: string, previous: string[]): string[] { return [...previous, value] } +/** Commander arg parser for `--max-warnings `: a non-negative integer, else a clean CLI error. */ +export function parseMaxWarnings(raw: string): number { + if (!/^\d+$/.test(raw.trim())) throw new InvalidArgumentError('--max-warnings must be a non-negative integer.') + return Number(raw.trim()) +} + /** Register a directly-invocable subcommand for every built-in check (`verifyx complexity`, `verifyx knip`, …). */ export function registerChecks(program: Command): void { program @@ -71,14 +77,17 @@ export function registerChecks(program: Command): void { // Mode flows via the VERIFY_MODE env / CI, not per-subcommand flags (which collide with the root's --check). for (const check of CHECKS.filter((c) => c.kind === 'external')) { - program + const command = program .command(check.name) .description(check.description) // Everything after `--` is forwarded verbatim to the underlying tool (e.g. `verifyx circular-deps -- src/*.ts`). .argument('[toolArgs...]', 'extra arguments passed through to the underlying tool (after `--`)') - .action(async (toolArgs: string[]) => { - const result = await check.runDefault({ extraArgs: toolArgs }) - finish(result.ok) - }) + if (check.supportsMaxWarnings) { + command.option('--max-warnings ', 'tolerate up to n findings before failing (counts findings)', parseMaxWarnings) + } + command.action(async (toolArgs: string[], opts: { maxWarnings?: number }) => { + const result = await check.runDefault({ extraArgs: toolArgs, maxWarnings: opts.maxWarnings }) + finish(result.ok) + }) } } diff --git a/src/shared/spawn.test.ts b/src/shared/spawn.test.ts new file mode 100644 index 0000000..ee7b992 --- /dev/null +++ b/src/shared/spawn.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest' + +import { captureCommand } from './spawn.ts' + +describe('captureCommand', () => { + it('returns the captured stdout and a zero exit code for a successful command', async () => { + const { code, stdout } = await captureCommand(`node -e "process.stdout.write('captured-output')"`) + expect(code).toBe(0) + expect(stdout).toBe('captured-output') + }) + + it('surfaces a non-zero exit code without throwing', async () => { + const { code } = await captureCommand(`node -e "process.exit(3)"`) + expect(code).toBe(3) + }) + + it('captures stderr separately from stdout', async () => { + const { stdout, stderr } = await captureCommand(`node -e "process.stderr.write('to-stderr')"`) + expect(stdout).toBe('') + expect(stderr).toContain('to-stderr') + }) +}) diff --git a/src/shared/spawn.ts b/src/shared/spawn.ts index 988a1bd..356a656 100644 --- a/src/shared/spawn.ts +++ b/src/shared/spawn.ts @@ -2,6 +2,11 @@ import { spawn } from 'node:child_process' import { emit, isCapturing } from './output.ts' +/** Append user/scaffold `extraArgs` to a command, preserving shell semantics (globs unquoted). */ +export function appendArgs(command: string, extraArgs: readonly string[] = []): string { + return extraArgs.length > 0 ? `${command} ${extraArgs.join(' ')}` : command +} + let verboseMode = false /** When verbose, per-command output is always streamed; otherwise it is buffered and only shown on failure. */ @@ -56,3 +61,26 @@ export function runCommand(command: string, opts: RunCommandOptions = {}): Promi child.on('error', () => resolve(127)) }) } + +/** + * Run a command and return its captured stdout/stderr as strings (never emitted to the output buffer), for callers + * that need to parse a tool's machine-readable output — e.g. counting findings for `--max-warnings`. + */ +export function captureCommand(command: string, opts: RunCommandOptions = {}): Promise<{ code: number; stdout: string; stderr: string }> { + return new Promise((resolve) => { + const child = spawn(command, [], { + stdio: ['ignore', 'pipe', 'pipe'], + shell: true, + cwd: opts.cwd ?? process.cwd(), + env: opts.env ? { ...process.env, ...opts.env } : undefined, + }) + const out: Buffer[] = [] + const err: Buffer[] = [] + child.stdout?.on('data', (data: Buffer) => out.push(data)) + child.stderr?.on('data', (data: Buffer) => err.push(data)) + child.on('close', (code) => { + resolve({ code: code ?? 1, stdout: Buffer.concat(out).toString(), stderr: Buffer.concat(err).toString() }) + }) + child.on('error', () => resolve({ code: 127, stdout: '', stderr: '' })) + }) +} From d87c7d3c0e40a0530dd27bf98ebd5127ad14b420 Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Tue, 14 Jul 2026 16:08:01 +1000 Subject: [PATCH 3/7] fix(checks): address Codex review of --max-warnings - unused-code now applies the budget via knip's own `--max-issues` (flag strategy) instead of verifyx-side JSON counting. knip enforces config-hint and configuration/operational errors (exit 2) independently of the budget, so a tolerance no longer suppresses unrelated failures; it also removes the need to parse knip JSON and fixes a passthrough `--reporter` conflict. (review #1, #2, #4) - duplicate-code keeps counting (jscpd has no native clone-count gate), but countJscpdClones now throws on an unrecognised report shape instead of counting it as zero, so a degraded/version-shifted report fails loudly. (review #2) - runCountedBudget no longer discards stderr when showing the over-budget report (stdout + stderr), so an actionable error on stderr isn't hidden. (review #5) - parseMaxWarnings rejects unsafe integers, so a huge digit string can't round to an effectively infinite budget. (review #6) Introduces a two-strategy maxWarnings capability (flag | count) on the external check spec. Not addressed: shell-string passthrough (review #3) is the codebase's deliberate, pre-existing behaviour (globs are intentionally unquoted, e.g. `circular-deps -- src/*.ts`); changing it is out of scope for this feature. --- README.md | 6 ++-- src/checks/external.test.ts | 46 ++++++++++++++++---------- src/checks/external.ts | 34 +++++++++++--------- src/checks/maxWarnings.test.ts | 43 +++++++------------------ src/checks/maxWarnings.ts | 50 ++++++++++------------------- src/checks/registry.ts | 8 +++-- src/commands/registerChecks.test.ts | 4 +++ src/commands/registerChecks.ts | 9 ++++-- 8 files changed, 94 insertions(+), 106 deletions(-) diff --git a/README.md b/README.md index d3f39c0..ccea0f3 100644 --- a/README.md +++ b/README.md @@ -142,10 +142,10 @@ Each external check is configured through its **tool's own config file**, exactl - `circular-deps`: [skott options](https://github.com/antoine-coulon/skott) - `duplicate-code`: [`.jscpd.json` or `package.json#jscpd`](https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config) -**Tolerating findings with `--max-warnings`.** `unused-code` and `duplicate-code` accept `--max-warnings `, an ESLint-style tolerance budget: the check counts its findings and passes while the count stays **at or below** `n`, failing only when it exceeds `n`. It's for turning a check on over a codebase that already has debt — pin the budget at the current count and ratchet it down, without letting new findings slip in. Neither tool has a native count gate, so `verifyx` counts for them (from knip's JSON reporter and jscpd's JSON report). +**Tolerating findings with `--max-warnings`.** `unused-code` and `duplicate-code` accept `--max-warnings `, an ESLint-style tolerance budget: the check passes while its finding count stays **at or below** `n`, failing only when it exceeds `n`. It's for turning a check on over a codebase that already has debt — pin the budget at the current count and ratchet it down, without letting new findings slip in. -- `unused-code` counts every unused item knip reports (files, exports, exported types, dependencies) as one. -- `duplicate-code` counts each duplicate clone jscpd reports as one. +- `unused-code` maps the budget to knip's own [`--max-issues`](https://knip.dev/reference/cli#--max-issues), so it counts each error-level finding (unused files, exports, types, dependencies) as one and honours your knip rule severities. Config-hint and configuration errors still fail regardless of the budget. +- `duplicate-code` has no native count gate in jscpd (only a duplication-percentage `--threshold`), so `verifyx` counts each duplicate clone jscpd reports as one. ```sh verifyx unused-code --max-warnings 5 diff --git a/src/checks/external.test.ts b/src/checks/external.test.ts index b176e1a..1b727cf 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest' -import { appendArgs, defineExternalCheck, externalFailureHint, runWithMaxWarnings, selectCommand } from './external.ts' +import { appendArgs, defineExternalCheck, externalFailureHint, runCountedBudget, selectCommand } from './external.ts' const fixable = { checkCommand: 'oxfmt --check .', fixCommand: 'oxfmt .' } const notFixable = { checkCommand: 'tsc --noEmit' } @@ -80,37 +80,49 @@ describe('defineExternalCheck', () => { }) }) -describe('runWithMaxWarnings', () => { - const baseSpec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } +describe('runCountedBudget', () => { + const spec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } + const budget = (count: () => Promise) => ({ strategy: 'count' as const, unit: 'clone', count: () => count() }) it('passes when the finding count is at or below the budget, without running the display command', async () => { - const spec = { ...baseSpec, maxWarnings: { unit: 'clone', count: async () => 5 } } - expect(await runWithMaxWarnings(spec, 5, [], {})).toEqual({ name: 'duplicate-code', ok: true }) + expect( + await runCountedBudget( + spec, + budget(async () => 5), + 5, + [], + {}, + ), + ).toEqual({ name: 'duplicate-code', ok: true }) }) it('fails when the finding count exceeds the budget', async () => { - const spec = { ...baseSpec, checkCommand: 'node -e ""', maxWarnings: { unit: 'clone', count: async () => 6 } } + const displaySpec = { ...spec, checkCommand: 'node -e ""' } const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { - expect((await runWithMaxWarnings(spec, 5, [], {})).ok).toBe(false) + expect( + ( + await runCountedBudget( + displaySpec, + budget(async () => 6), + 5, + [], + {}, + ) + ).ok, + ).toBe(false) } finally { spy.mockRestore() } }) it('fails loudly when counting throws instead of silently passing', async () => { - const spec = { - ...baseSpec, - maxWarnings: { - unit: 'clone', - count: async () => { - throw new Error('knip crashed') - }, - }, - } + const throwing = budget(async () => { + throw new Error('jscpd crashed') + }) const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { - const result = await runWithMaxWarnings(spec, 5, [], {}) + const result = await runCountedBudget(spec, throwing, 5, [], {}) expect(result.ok).toBe(false) expect(spy).toHaveBeenCalled() } finally { diff --git a/src/checks/external.ts b/src/checks/external.ts index f194fc6..60fd924 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -68,25 +68,25 @@ export function externalFailureHint(spec: Pick & { - maxWarnings: MaxWarningsSupport -} +type CountBudget = Extract +type CountableSpec = Pick /** - * Run a countable external check under a `--max-warnings` budget: count findings via the tool's machine-readable - * output and pass while the count stays at or below the budget. On over-budget, print a one-line summary, then the - * tool's normal report (re-run and shown regardless of its own exit code), then the failure hint. A counting error - * fails loudly rather than passing silently. + * Run a check whose tool has no native budget under a `--max-warnings` budget: count findings via the tool's + * machine-readable output and pass while the count stays at or below the budget. On over-budget, print a one-line + * summary, then the tool's normal report (re-run and shown regardless of its own exit code), then the failure hint. + * A counting error fails loudly rather than passing silently. */ -export async function runWithMaxWarnings( +export async function runCountedBudget( spec: CountableSpec, + budget: CountBudget, maxWarnings: number, extraArgs: string[], env: Record, ): Promise { let count: number try { - count = await spec.maxWarnings.count({ extraArgs, env, checkCommand: spec.checkCommand }) + count = await budget.count({ extraArgs, env, checkCommand: spec.checkCommand }) } catch (error) { const docs = spec.docs ? ` — ${spec.docs}` : '' console.error( @@ -98,11 +98,12 @@ export async function runWithMaxWarnings( } if (withinBudget(count, maxWarnings)) return { name: spec.name, ok: true } - const unit = count === 1 ? spec.maxWarnings.unit : `${spec.maxWarnings.unit}s` + const unit = count === 1 ? budget.unit : `${budget.unit}s` console.error(color.dim(`↳ ${spec.name}: ${count} ${unit} found, exceeds --max-warnings ${maxWarnings}.`)) const command = appendArgs(selectCommand(spec, resolveMode()), extraArgs) + // Keep both channels: the report is on stdout, but an actionable config/module error may be on stderr. const { stdout, stderr } = await captureCommand(command, { env }) - const report = stdout || stderr + const report = stdout + stderr if (report) emit(spec.transformOutput ? spec.transformOutput(report) : report) console.error(color.dim(externalFailureHint(spec, command))) return { name: spec.name, ok: false } @@ -123,7 +124,7 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { }, // `verifyx eject ` inlines these raw commands into the consumer's verify:* scripts. eject: { check: spec.checkCommand, fix: spec.fixCommand }, - async runDefault({ extraArgs, maxWarnings }: RunDefaultOptions = {}): Promise { + async runDefault({ extraArgs = [], maxWarnings }: RunDefaultOptions = {}): Promise { if (!hasLocalBin(spec.bin)) { console.log(color.dim(`${spec.name}: ${spec.bin} not installed — skipping (add it with \`npx verifyx init\`)`)) return { name: spec.name, ok: true, skipped: true } @@ -132,10 +133,13 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { console.log(color.dim(`${spec.name}: not applicable here — skipping`)) return { name: spec.name, ok: true, skipped: true } } - if (maxWarnings !== undefined && spec.maxWarnings) { - return runWithMaxWarnings({ ...spec, maxWarnings: spec.maxWarnings }, maxWarnings, extraArgs ?? [], envWithLocalBin()) + const budget = maxWarnings !== undefined ? spec.maxWarnings : undefined + if (budget?.strategy === 'count') { + return runCountedBudget(spec, budget, maxWarnings as number, extraArgs, envWithLocalBin()) } - const command = appendArgs(selectCommand(spec, resolveMode()), extraArgs) + // A `flag`-strategy budget just augments the normal command with the tool's own budget option. + const budgetArgs = budget?.strategy === 'flag' ? budget.toArgs(maxWarnings as number) : [] + const command = appendArgs(selectCommand(spec, resolveMode()), [...extraArgs, ...budgetArgs]) // quiet: buffer the tool's output and flush only on failure (streamed live under --verbose). const code = await runCommand(command, { env: envWithLocalBin(), quiet: true, transform: spec.transformOutput }) // Only on failure — passing runs stay silent to save tokens. diff --git a/src/checks/maxWarnings.test.ts b/src/checks/maxWarnings.test.ts index a0e6b78..d5b1256 100644 --- a/src/checks/maxWarnings.test.ts +++ b/src/checks/maxWarnings.test.ts @@ -1,37 +1,6 @@ import { describe, expect, it } from 'vitest' -import { countJscpdClones, countKnipFindings, withinBudget } from './maxWarnings.ts' - -describe('countKnipFindings', () => { - it('sums the lengths of every finding array across all issue rows', () => { - const report = { - issues: [ - { file: 'src/unused.ts', files: ['src/unused.ts'], exports: [], dependencies: [] }, - { - file: 'src/foo.ts', - exports: [{ name: 'a' }, { name: 'b' }], - types: [{ name: 'T' }], - duplicates: [[{ name: 'x' }, { name: 'y' }]], - }, - { file: 'package.json', dependencies: [{ name: 'lodash' }], devDependencies: [{ name: 'jest' }], unlisted: [{ name: 'react' }] }, - ], - } - expect(countKnipFindings(report)).toBe(8) - }) - - it('excludes the file string and owners metadata from the count', () => { - const report = { issues: [{ file: 'src/foo.ts', owners: [{ name: 'team-a' }, { name: 'team-b' }], exports: [{ name: 'unused' }] }] } - expect(countKnipFindings(report)).toBe(1) - }) - - it('returns 0 for a clean report', () => { - expect(countKnipFindings({ issues: [] })).toBe(0) - }) - - it('returns 0 when the issues key is absent', () => { - expect(countKnipFindings({})).toBe(0) - }) -}) +import { countJscpdClones, withinBudget } from './maxWarnings.ts' describe('countJscpdClones', () => { it('reads the total clone count from statistics', () => { @@ -46,6 +15,16 @@ describe('countJscpdClones', () => { it('returns 0 for a clean report', () => { expect(countJscpdClones({ duplicates: [], statistics: { total: { clones: 0 } } })).toBe(0) }) + + it('throws on an unrecognised shape rather than counting it as zero', () => { + expect(() => countJscpdClones({})).toThrow(/unrecognised/) + expect(() => countJscpdClones({ statistics: { total: {} } })).toThrow(/unrecognised/) + }) + + it('rejects a non-integer or negative clone count when there is no duplicates array', () => { + expect(() => countJscpdClones({ statistics: { total: { clones: -1 } } })).toThrow(/unrecognised/) + expect(() => countJscpdClones({ statistics: { total: { clones: 2.5 } } })).toThrow(/unrecognised/) + }) }) describe('withinBudget', () => { diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts index 0b1ac2a..7d6e5bd 100644 --- a/src/checks/maxWarnings.ts +++ b/src/checks/maxWarnings.ts @@ -7,47 +7,31 @@ import { appendArgs, captureCommand } from '../shared/spawn.ts' /** Context handed to a counting run: the user's passthrough args, the PATH-augmented env, and the check's own command. */ type MaxWarningsCountContext = { extraArgs: string[]; env: Record; checkCommand: string } -/** Opt-in capability that lets an external check tolerate up to `--max-warnings` findings by counting them itself. */ -export type MaxWarningsSupport = { - /** Singular label for one finding, pluralised in the over-budget summary, e.g. 'clone' → '6 clones found'. */ - unit: string - /** Run the tool in machine-readable mode and return the total finding count. */ - count: (ctx: MaxWarningsCountContext) => Promise -} - -/** knip's JSON reporter attaches ownership metadata (not a finding) under this key; exclude it from the count. */ -const KNIP_NON_FINDING_KEYS = new Set(['owners']) - /** - * Total unused items in a knip `--reporter json` report: every finding array (files, exports, types, - * dependencies, duplicate groups, …) summed across all issue rows. Robust to knip adding new issue types, - * since any array-valued field counts; only ownership metadata and the row's `file` string are excluded. + * How a check applies a `--max-warnings ` budget: + * - `flag`: the tool has its own budget option, so map n to the args to append — the tool's exit code is the verdict, + * which keeps any independent failures (e.g. knip config hints, operational errors) enforced regardless of n. + * - `count`: the tool has no native gate, so verifyx counts findings from its machine-readable output and compares to n. */ -export function countKnipFindings(report: { issues?: Array> }): number { - let total = 0 - for (const row of report.issues ?? []) { - for (const [key, value] of Object.entries(row)) { - if (!KNIP_NON_FINDING_KEYS.has(key) && Array.isArray(value)) total += value.length - } - } - return total -} - -/** Number of duplicate clones in a jscpd JSON report: the authoritative total, else the duplicates array length. */ -export function countJscpdClones(report: { statistics?: { total?: { clones?: number } }; duplicates?: unknown[] }): number { - return report.statistics?.total?.clones ?? report.duplicates?.length ?? 0 -} +export type MaxWarningsSupport = + | { strategy: 'flag'; toArgs: (maxWarnings: number) => string[] } + | { strategy: 'count'; unit: string; count: (ctx: MaxWarningsCountContext) => Promise } /** A counted check passes while findings stay at or below the budget; the budget itself is tolerated (inclusive). */ export function withinBudget(count: number, maxWarnings: number): boolean { return count <= maxWarnings } -/** Count knip's findings by asking it for a JSON report on stdout (config-hint errors are irrelevant to the count). */ -export async function knipCount(ctx: MaxWarningsCountContext): Promise { - const command = `${appendArgs('knip --no-progress', ctx.extraArgs)} --reporter json` - const { stdout } = await captureCommand(command, { env: ctx.env }) - return countKnipFindings(JSON.parse(stdout)) +/** + * Number of duplicate clones in a jscpd JSON report. A valid report always carries a non-negative integer + * `statistics.total.clones` (or at least a `duplicates` array); any other shape is unrecognised and throws, so a + * degraded or version-shifted report fails loudly rather than silently counting as zero. + */ +export function countJscpdClones(report: { statistics?: { total?: { clones?: unknown } }; duplicates?: unknown }): number { + const clones = report.statistics?.total?.clones + if (typeof clones === 'number' && Number.isInteger(clones) && clones >= 0) return clones + if (Array.isArray(report.duplicates)) return report.duplicates.length + throw new Error('unrecognised jscpd JSON report (no non-negative integer statistics.total.clones and no duplicates array)') } /** diff --git a/src/checks/registry.ts b/src/checks/registry.ts index c7640ff..9b4cb76 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -6,7 +6,7 @@ import { runComplexity } from './complexity.ts' import { defineExternalCheck } from './external.ts' import { runForbiddenStrings } from './forbidden-strings.ts' import { runHardcodedColors } from './hardcoded-colors.ts' -import { jscpdCount, knipCount } from './maxWarnings.ts' +import { jscpdCount } from './maxWarnings.ts' import type { Check, CheckResult } from './types.ts' function nativeCheck(name: string, description: string, recommended: boolean, run: () => CheckResult, script = `verifyx ${name}`): Check { @@ -70,7 +70,8 @@ export const CHECKS: Check[] = [ checkCommand: 'knip --no-progress --treat-config-hints-as-errors', devDeps: ['knip'], docs: 'https://knip.dev/reference/configuration', - maxWarnings: { unit: 'unused item', count: knipCount }, + // knip has a native budget (--max-issues counts error-level findings); config hints / errors still fail regardless. + maxWarnings: { strategy: 'flag', toArgs: (n) => ['--max-issues', String(n)] }, }), defineExternalCheck({ name: 'circular-deps', @@ -92,7 +93,8 @@ export const CHECKS: Check[] = [ // NO_COLOR/FORCE_COLOR, so strip the red foreground from its output; the table renders in the default colour. transformOutput: withoutRed, docs: 'https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config', - maxWarnings: { unit: 'clone', count: jscpdCount }, + // jscpd has no native clone-count gate (only a duplication-percentage --threshold), so verifyx counts clones. + maxWarnings: { strategy: 'count', unit: 'clone', count: jscpdCount }, }), ] diff --git a/src/commands/registerChecks.test.ts b/src/commands/registerChecks.test.ts index f7c5fd9..78bd4ac 100644 --- a/src/commands/registerChecks.test.ts +++ b/src/commands/registerChecks.test.ts @@ -24,4 +24,8 @@ describe('parseMaxWarnings', () => { it('rejects an empty value', () => { expect(() => parseMaxWarnings('')).toThrow(/non-negative integer/) }) + + it('rejects an unsafe integer that would round to an effectively infinite budget', () => { + expect(() => parseMaxWarnings('999999999999999999999999999999999999')).toThrow(/non-negative integer/) + }) }) diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index 51c9528..9e76178 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -14,10 +14,13 @@ function collect(value: string, previous: string[]): string[] { return [...previous, value] } -/** Commander arg parser for `--max-warnings `: a non-negative integer, else a clean CLI error. */ +/** Commander arg parser for `--max-warnings `: a safe non-negative integer, else a clean CLI error. */ export function parseMaxWarnings(raw: string): number { - if (!/^\d+$/.test(raw.trim())) throw new InvalidArgumentError('--max-warnings must be a non-negative integer.') - return Number(raw.trim()) + const value = Number(raw.trim()) + if (!/^\d+$/.test(raw.trim()) || !Number.isSafeInteger(value)) { + throw new InvalidArgumentError('--max-warnings must be a non-negative integer.') + } + return value } /** Register a directly-invocable subcommand for every built-in check (`verifyx complexity`, `verifyx knip`, …). */ From d15578384e4095368c07697a7f95e5f80d4886e4 Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Tue, 14 Jul 2026 16:21:27 +1000 Subject: [PATCH 4/7] fix(checks): address Fable review of --max-warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - jscpdCount now surfaces jscpd's own stderr and exit code when it produces no report (e.g. a passthrough flag colliding with the appended --output/--silent), so the "could not count" error is actionable instead of a bare ENOENT. (review M1) - Temp-report cleanup uses maxRetries: 3 so a transient Windows lock (AV/indexer) on the just-written report can't flip a green check red. (review L3) - runDefault drops the `as number` casts by narrowing under the maxWarnings guard and sharing the run/report logic; captureCommand's options type no longer advertises quiet/transform it ignores. (review L8) - README notes the duplicate-code passthrough caveat (--reporters/--output/--silent conflict with the count run). (review M2) - Spec doc gets a post-review update note recording the knip --max-issues pivot and the two-variant capability. (review L6) - Fixed an over-claiming test title. (review L7c) Not changed: latent fix-mode mismatch in runCountedBudget (L4, unreachable — no fixable counted check exists), verbose streaming on the count path (L5, minor by-design), and real-tool-spawn test gaps (L7a/b/d, covered by e2e per the repo's convention of not unit-testing live tool invocations). --- README.md | 2 +- ...14-max-warnings-unused-duplicate-design.md | 17 ++++++++++++++ src/checks/external.test.ts | 2 +- src/checks/external.ts | 23 ++++++++++--------- src/checks/maxWarnings.ts | 12 +++++++--- src/shared/spawn.ts | 5 +++- 6 files changed, 44 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index ccea0f3..30bf5c3 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ verifyx unused-code --max-warnings 5 verifyx duplicate-code --max-warnings 10 ``` -Without the flag both checks stay **zero-tolerance** (fail on any finding), exactly as before. When the budget is exceeded the check prints how many findings it found, the tool's normal report, and the usual failure hint. Because it's a flag on the script, it applies wherever the script carries it: put it in a `verify:` script and both the bare `verifyx` gate and `verifyx all` pick it up (a `verify:` script [overrides](#verifyx-all-run-everything) the matching built-in). Under `verifyx all` with no such script, the built-in runs at its zero-tolerance default. Passthrough still composes: `verifyx unused-code --max-warnings 5 -- --production`. +Without the flag both checks stay **zero-tolerance** (fail on any finding), exactly as before. When the budget is exceeded the check prints how many findings it found, the tool's normal report, and the usual failure hint. Because it's a flag on the script, it applies wherever the script carries it: put it in a `verify:` script and both the bare `verifyx` gate and `verifyx all` pick it up (a `verify:` script [overrides](#verifyx-all-run-everything) the matching built-in). Under `verifyx all` with no such script, the built-in runs at its zero-tolerance default. Passthrough still composes (e.g. `verifyx unused-code --max-warnings 5 -- --production`); the one exception is `duplicate-code`, where `--max-warnings` drives jscpd's reporter for counting, so also passing `--reporters`/`--output`/`--silent` after `--` conflicts (it fails with jscpd's own error rather than passing). ### `complexity` diff --git a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md index fbdfda3..2d34fe5 100644 --- a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md +++ b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md @@ -200,3 +200,20 @@ the parsers hold the logic that needs pinning. their own tool config; the flag lives on the script, consistent with the existing model). - Per-category budgets for knip (a single total count is the agreed unit). - Adding `--max-warnings` to other external checks. + +## Update (post-implementation review) + +A Codex review found that knip **does** have a native budget flag, +[`--max-issues`](https://knip.dev/reference/cli#--max-issues) — this design's premise +that "knip has no max-issues flag" was wrong. The implementation therefore diverged: + +- **`unused-code` uses a `flag` strategy** — it appends knip's own `--max-issues ` and + takes the verdict from knip's exit code, rather than parsing knip JSON (`countKnipFindings` + is gone). This is more correct: knip counts error-level findings honouring rule severity, + and enforces config-hint / operational (exit 2) failures independently of the budget, so a + tolerance no longer suppresses unrelated errors. +- **`duplicate-code` keeps the `count` strategy** (jscpd has no native clone-count gate). + `countJscpdClones` now throws on an unrecognised report shape instead of returning zero. + +So `MaxWarningsSupport` is a two-variant union (`flag` | `count`), not the single `count` +shape sketched above. The counting-path runner is `runCountedBudget`. diff --git a/src/checks/external.test.ts b/src/checks/external.test.ts index 1b727cf..3e0b0c0 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -84,7 +84,7 @@ describe('runCountedBudget', () => { const spec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } const budget = (count: () => Promise) => ({ strategy: 'count' as const, unit: 'clone', count: () => count() }) - it('passes when the finding count is at or below the budget, without running the display command', async () => { + it('passes when the finding count is at or below the budget', async () => { expect( await runCountedBudget( spec, diff --git a/src/checks/external.ts b/src/checks/external.ts index 60fd924..e93734e 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -133,18 +133,19 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { console.log(color.dim(`${spec.name}: not applicable here — skipping`)) return { name: spec.name, ok: true, skipped: true } } - const budget = maxWarnings !== undefined ? spec.maxWarnings : undefined - if (budget?.strategy === 'count') { - return runCountedBudget(spec, budget, maxWarnings as number, extraArgs, envWithLocalBin()) - } - // A `flag`-strategy budget just augments the normal command with the tool's own budget option. - const budgetArgs = budget?.strategy === 'flag' ? budget.toArgs(maxWarnings as number) : [] - const command = appendArgs(selectCommand(spec, resolveMode()), [...extraArgs, ...budgetArgs]) // quiet: buffer the tool's output and flush only on failure (streamed live under --verbose). - const code = await runCommand(command, { env: envWithLocalBin(), quiet: true, transform: spec.transformOutput }) - // Only on failure — passing runs stay silent to save tokens. - if (code !== 0) console.error(color.dim(externalFailureHint(spec, command))) - return { name: spec.name, ok: code === 0 } + const runReport = async (command: string): Promise => { + const code = await runCommand(command, { env: envWithLocalBin(), quiet: true, transform: spec.transformOutput }) + if (code !== 0) console.error(color.dim(externalFailureHint(spec, command))) + return { name: spec.name, ok: code === 0 } + } + if (maxWarnings !== undefined && spec.maxWarnings) { + const budget = spec.maxWarnings + if (budget.strategy === 'count') return runCountedBudget(spec, budget, maxWarnings, extraArgs, envWithLocalBin()) + // A `flag`-strategy budget just augments the normal command with the tool's own budget option (budget wins last). + return runReport(appendArgs(selectCommand(spec, resolveMode()), [...extraArgs, ...budget.toArgs(maxWarnings)])) + } + return runReport(appendArgs(selectCommand(spec, resolveMode()), extraArgs)) }, } } diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts index 7d6e5bd..e987733 100644 --- a/src/checks/maxWarnings.ts +++ b/src/checks/maxWarnings.ts @@ -41,9 +41,15 @@ export function countJscpdClones(report: { statistics?: { total?: { clones?: unk export async function jscpdCount(ctx: MaxWarningsCountContext): Promise { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verifyx-jscpd-')) try { - await captureCommand(`${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}" --silent`, { env: ctx.env }) - return countJscpdClones(JSON.parse(fs.readFileSync(path.join(dir, 'jscpd-report.json'), 'utf8'))) + const command = `${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}" --silent` + const { code, stderr } = await captureCommand(command, { env: ctx.env }) + const reportPath = path.join(dir, 'jscpd-report.json') + // No report means the run itself failed (e.g. a passthrough flag colliding with the appended ones) — surface the + // tool's own stderr so the "could not count" error is actionable, rather than a bare ENOENT from reading the file. + if (!fs.existsSync(reportPath)) throw new Error(`jscpd produced no report (exit ${code})${stderr.trim() ? `: ${stderr.trim()}` : ''}`) + return countJscpdClones(JSON.parse(fs.readFileSync(reportPath, 'utf8'))) } finally { - fs.rmSync(dir, { recursive: true, force: true }) + // maxRetries covers a transient Windows lock (AV/indexer) on the just-written report so cleanup can't fail the check. + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3 }) } } diff --git a/src/shared/spawn.ts b/src/shared/spawn.ts index 356a656..9dbc6de 100644 --- a/src/shared/spawn.ts +++ b/src/shared/spawn.ts @@ -66,7 +66,10 @@ export function runCommand(command: string, opts: RunCommandOptions = {}): Promi * Run a command and return its captured stdout/stderr as strings (never emitted to the output buffer), for callers * that need to parse a tool's machine-readable output — e.g. counting findings for `--max-warnings`. */ -export function captureCommand(command: string, opts: RunCommandOptions = {}): Promise<{ code: number; stdout: string; stderr: string }> { +export function captureCommand( + command: string, + opts: { cwd?: string; env?: Record } = {}, +): Promise<{ code: number; stdout: string; stderr: string }> { return new Promise((resolve) => { const child = spawn(command, [], { stdio: ['ignore', 'pipe', 'pipe'], From ab72c2152692c7dab458b1debb54658cce1db495 Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Wed, 15 Jul 2026 09:59:24 +1000 Subject: [PATCH 5/7] chore: remove docs --- ...14-max-warnings-unused-duplicate-design.md | 219 ------------------ 1 file changed, 219 deletions(-) delete mode 100644 docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md diff --git a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md b/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md deleted file mode 100644 index 2d34fe5..0000000 --- a/docs/superpowers/specs/2026-07-14-max-warnings-unused-duplicate-design.md +++ /dev/null @@ -1,219 +0,0 @@ -# Design: `--max-warnings` for `unused-code` and `duplicate-code` - -**Date:** 2026-07-14 -**Status:** Approved - -## Summary - -Add a `--max-warnings ` flag to the `unused-code` (knip) and `duplicate-code` -(jscpd) checks. It is an ESLint-style tolerance gate: the check passes while the -number of findings stays at or below `n`, and fails when it exceeds `n`. This lets -a project adopt either check on a codebase that already has debt — set the budget -at (or just below) the current count and ratchet it down over time, without letting -new findings slip in. - -## Motivation - -Today both checks are all-or-nothing: knip fails if there is _any_ unused -file/export/dependency, and jscpd fails on _any_ duplication. On an existing -codebase that means the check is red from day one and stays red until every finding -is fixed, so teams tend not to turn it on at all. A tolerance budget makes gradual -adoption possible. - -Neither underlying tool has a native count-based gate: - -- **knip** exits non-zero whenever there is any finding; it has no "max issues" flag. -- **jscpd** only offers `--threshold ` (a duplication _percentage_), not a - count of clones. - -So `verifyx` itself must count findings from each tool's machine-readable output and -decide pass/fail. That is the core of this design. - -## Behavior - -- New flag `--max-warnings ` on `verifyx unused-code` and `verifyx duplicate-code`. - `n` is a non-negative integer. -- **Without the flag:** unchanged — the current exit-code path, zero tolerance. -- **With the flag:** `verifyx` runs the tool in a machine-readable "count" mode, - tallies findings, and: - - `count <= n` → **pass**, silent (per the project's quiet-on-success rule). - - `count > n` → **fail**, printing a one-line count summary, then the tool's normal - findings report, then the existing external-failure hint. -- **Count unit:** - - `unused-code` (knip): total unused _items_ — every unused file, export, exported - type, and dependency, summed into a single number. - - `duplicate-code` (jscpd): the number of detected duplicate _clones_. - -## How it flows through `verifyx` - -`--max-warnings` is a subcommand flag, so it rides the existing curated-gate pattern. -A consumer wires a budget into their `package.json`: - -```jsonc -{ - "scripts": { - "verify:unused-code": "verifyx unused-code --max-warnings 5", - "verify:duplicate-code": "verifyx duplicate-code --max-warnings 10", - }, -} -``` - -- **Bare `verifyx`** (the curated gate) runs these `verify:*` scripts verbatim, so the - budget applies. -- **`verifyx all`** runs each built-in via `runDefault()` with _no_ arguments, so a - built-in with no override runs at its **default: zero tolerance** (today's behavior). - A `verify:` script that matches a built-in **overrides** it under `verifyx all`, - so the same one-line script above is also how you apply a budget in "run everything" - mode. `verifyx all` never adds flags on its own. -- **Passthrough is preserved:** `verifyx unused-code --max-warnings 5 -- --production` - forwards `--production` to knip, and `--max-warnings` is consumed by `verifyx`. - -## Implementation - -The design reuses `defineExternalCheck` rather than adding bespoke check types. - -### `ExternalCheckSpec` gains an optional counting capability - -Only `unused-code` and `duplicate-code` set it; every other external check is untouched -and rejects `--max-warnings` (the flag is not registered for them). - -```ts -export type MaxWarningsSupport = { - /** Run the tool in machine-readable mode and return the total finding count. */ - count: (ctx: { extraArgs: string[]; env: Record }) => Promise -} -``` - -`ExternalCheckSpec` gets `maxWarnings?: MaxWarningsSupport`. - -### `RunDefaultOptions` gains `maxWarnings?: number` - -```ts -export type RunDefaultOptions = { - extraArgs?: string[] - /** When set, the check counts findings and passes iff count <= maxWarnings (external checks that support it). */ - maxWarnings?: number -} -``` - -### `runDefault` branch - -`defineExternalCheck`'s `runDefault` takes the counting path **only** when -`maxWarnings` is provided _and_ the spec has a `maxWarnings` capability; otherwise it -runs exactly as today. The bin-present and `canRun` guards run first, unchanged (so a -missing tool still skips gracefully before any counting). - -Counting path: - -1. `count = await spec.maxWarnings.count({ extraArgs, env: envWithLocalBin() })`. -2. `count <= maxWarnings` → return `{ ok: true }`, print nothing. -3. `count > maxWarnings` → print a one-line summary (e.g. - `duplicate-code: 12 clones found, exceeds --max-warnings 10`), then re-run the - tool's normal `checkCommand` (buffered) so the familiar findings report is shown, - then the existing `externalFailureHint`. Return `{ ok: false }`. - -In count mode the tool's own exit code is **ignored** for the verdict (knip exits -non-zero on any finding, even within budget); the verdict is purely the parsed count. - -### New spawn helper - -`runCommand` currently returns only the exit code and emits output to the buffer. Add a -sibling that returns captured stdout for parsing, leaving `runCommand` untouched: - -```ts -export function captureCommand(command: string, opts?: RunCommandOptions): Promise<{ code: number; stdout: string }> -``` - -### Per-tool count functions - -Extracted as exported **pure parsers** so they are unit-testable without the real tools; -the `count` functions wire the parser to a real invocation. - -- **knip** — `knip --reporter json --no-progress [extraArgs]`, capture stdout, then - `countKnipFindings(json)`: `files.length` plus the summed lengths of every issue array - across `issues[]` (dependencies, devDependencies, optionalPeerDependencies, unlisted, - binaries, unresolved, exports, nsExports, types, nsTypes, enumMembers, duplicates, - and any other array-valued issue fields). The exact knip JSON shape is confirmed - against the installed knip during implementation and pinned with a fixture test. -- **jscpd** — jscpd has no stdout JSON reporter, so run it with the JSON reporter into a - fresh OS temp dir (`fs.mkdtemp` under `os.tmpdir()`), read `jscpd-report.json`, then - `countJscpdClones(json)`: `statistics.total.clones` (falling back to - `duplicates.length`). The temp dir is removed afterward (including on error). jscpd's - console reporters are suppressed for the count run. - -### `registerChecks` - -`unused-code` and `duplicate-code` are registered explicitly (out of the generic -external loop) so they can declare `--max-warnings ` alongside the existing -`[toolArgs...]` passthrough: - -```ts -program - .command('unused-code') - .description(...) - .option('--max-warnings ', 'tolerate up to n findings before failing', Number) - .argument('[toolArgs...]', 'extra arguments passed through to the underlying tool (after `--`)') - .action(async (toolArgs, opts) => { - finish((await getCheck('unused-code')!.runDefault({ extraArgs: toolArgs, maxWarnings: opts.maxWarnings })).ok) - }) -``` - -All other external checks keep the generic registration loop. - -## Errors and edge cases - -- **Invalid `n`** (negative, non-integer, or `NaN` from a non-numeric value) → a clear - error message and non-zero exit, before running any tool. -- **`--max-warnings 0`** is valid and equivalent to today's zero-tolerance behavior - (routed through the count path). -- **Count command crashes or emits unparseable output** → fail loudly (never silently - pass): surface the raw output and the failure hint. A parse failure is a check failure. -- **Tool not installed / `canRun` false** → skip, exactly as today (guards run before - counting). - -## Docs and programmatic API - -- **README:** document `--max-warnings` under the `unused-code` and `duplicate-code` - sections — the count semantics per tool, the `verify:` override needed for - `verifyx all`, and that the default is zero tolerance. Note it in the built-in-checks - overview where relevant. -- **Programmatic:** `getCheck('unused-code')?.runDefault({ maxWarnings: 5 })` works via - the existing registry entry point; no new export beyond the count parsers. - -## Testing - -Following the existing `external.test.ts` pure-function style: - -- `countKnipFindings` against a JSON fixture with mixed issue types → expected total. -- `countJscpdClones` against a JSON fixture → expected clone count. -- The decision logic: `count <= n` → ok, `count > n` → fail (spec with a stubbed - `count`), including the `n = 0` boundary. -- `--max-warnings` argument validation (rejects negative / non-integer). - -The `count` functions themselves (which spawn the real tools) are covered indirectly; -the parsers hold the logic that needs pinning. - -## Out of scope - -- Applying a budget automatically under `verifyx all` without a `verify:` override. -- Persisting the budget in `verify.config.json` (external checks are configured through - their own tool config; the flag lives on the script, consistent with the existing model). -- Per-category budgets for knip (a single total count is the agreed unit). -- Adding `--max-warnings` to other external checks. - -## Update (post-implementation review) - -A Codex review found that knip **does** have a native budget flag, -[`--max-issues`](https://knip.dev/reference/cli#--max-issues) — this design's premise -that "knip has no max-issues flag" was wrong. The implementation therefore diverged: - -- **`unused-code` uses a `flag` strategy** — it appends knip's own `--max-issues ` and - takes the verdict from knip's exit code, rather than parsing knip JSON (`countKnipFindings` - is gone). This is more correct: knip counts error-level findings honouring rule severity, - and enforces config-hint / operational (exit 2) failures independently of the budget, so a - tolerance no longer suppresses unrelated errors. -- **`duplicate-code` keeps the `count` strategy** (jscpd has no native clone-count gate). - `countJscpdClones` now throws on an unrecognised report shape instead of returning zero. - -So `MaxWarningsSupport` is a two-variant union (`flag` | `count`), not the single `count` -shape sketched above. The counting-path runner is `runCountedBudget`. From 9bf4525d792dc7a4aaa80e99a8ce761f0364690c Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Wed, 15 Jul 2026 10:04:41 +1000 Subject: [PATCH 6/7] chore: clean up docs --- README.md | 11 +++++------ src/checks/external.ts | 9 --------- src/checks/maxWarnings.ts | 22 ++-------------------- src/checks/registry.ts | 2 -- src/checks/types.ts | 5 ----- src/commands/registerChecks.ts | 1 - src/shared/spawn.ts | 6 +----- 7 files changed, 8 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 30bf5c3..8d02dff 100644 --- a/README.md +++ b/README.md @@ -142,18 +142,18 @@ Each external check is configured through its **tool's own config file**, exactl - `circular-deps`: [skott options](https://github.com/antoine-coulon/skott) - `duplicate-code`: [`.jscpd.json` or `package.json#jscpd`](https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config) -**Tolerating findings with `--max-warnings`.** `unused-code` and `duplicate-code` accept `--max-warnings `, an ESLint-style tolerance budget: the check passes while its finding count stays **at or below** `n`, failing only when it exceeds `n`. It's for turning a check on over a codebase that already has debt — pin the budget at the current count and ratchet it down, without letting new findings slip in. +### `--max-warnings` -- `unused-code` maps the budget to knip's own [`--max-issues`](https://knip.dev/reference/cli#--max-issues), so it counts each error-level finding (unused files, exports, types, dependencies) as one and honours your knip rule severities. Config-hint and configuration errors still fail regardless of the budget. -- `duplicate-code` has no native count gate in jscpd (only a duplication-percentage `--threshold`), so `verifyx` counts each duplicate clone jscpd reports as one. +`unused-code` and `duplicate-code` accept `--max-warnings ` and fail when findings exceed `n`. + +- `unused-code` passes the value to knip's [`--max-issues`](https://knip.dev/reference/cli#--max-issues). Knip configuration errors still fail. +- `duplicate-code` counts jscpd clones. Do not pass jscpd's `--reporters`, `--output`, or `--silent` options with `--max-warnings`; `verifyx` sets them to produce the count. ```sh verifyx unused-code --max-warnings 5 verifyx duplicate-code --max-warnings 10 ``` -Without the flag both checks stay **zero-tolerance** (fail on any finding), exactly as before. When the budget is exceeded the check prints how many findings it found, the tool's normal report, and the usual failure hint. Because it's a flag on the script, it applies wherever the script carries it: put it in a `verify:` script and both the bare `verifyx` gate and `verifyx all` pick it up (a `verify:` script [overrides](#verifyx-all-run-everything) the matching built-in). Under `verifyx all` with no such script, the built-in runs at its zero-tolerance default. Passthrough still composes (e.g. `verifyx unused-code --max-warnings 5 -- --production`); the one exception is `duplicate-code`, where `--max-warnings` drives jscpd's reporter for counting, so also passing `--reporters`/`--output`/`--silent` after `--` conflicts (it fails with jscpd's own error rather than passing). - ### `complexity` ```sh @@ -329,7 +329,6 @@ const { failing } = analyzeComplexity({ pattern: 'src/**/*.ts', threshold: 50 }) // Run any single check by name, including the ones that shell out to an external tool. const lint = await getCheck('lint')?.runDefault() -// unused-code / duplicate-code accept a max-warnings budget (pass iff findings <= maxWarnings). const unused = await getCheck('unused-code')?.runDefault({ maxWarnings: 5 }) ``` diff --git a/src/checks/external.ts b/src/checks/external.ts index e93734e..0870f1f 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -50,7 +50,6 @@ export type ExternalCheckSpec = { * script so a consumer can see and tweak them. Not baked into `runDefault`; only the scaffolded script carries them. */ scaffoldArgs?: string - /** Present only for checks that support `--max-warnings`; defines how to count findings for the tolerance gate. */ maxWarnings?: MaxWarningsSupport } @@ -71,12 +70,6 @@ export function externalFailureHint(spec: Pick type CountableSpec = Pick -/** - * Run a check whose tool has no native budget under a `--max-warnings` budget: count findings via the tool's - * machine-readable output and pass while the count stays at or below the budget. On over-budget, print a one-line - * summary, then the tool's normal report (re-run and shown regardless of its own exit code), then the failure hint. - * A counting error fails loudly rather than passing silently. - */ export async function runCountedBudget( spec: CountableSpec, budget: CountBudget, @@ -101,7 +94,6 @@ export async function runCountedBudget( const unit = count === 1 ? budget.unit : `${budget.unit}s` console.error(color.dim(`↳ ${spec.name}: ${count} ${unit} found, exceeds --max-warnings ${maxWarnings}.`)) const command = appendArgs(selectCommand(spec, resolveMode()), extraArgs) - // Keep both channels: the report is on stdout, but an actionable config/module error may be on stderr. const { stdout, stderr } = await captureCommand(command, { env }) const report = stdout + stderr if (report) emit(spec.transformOutput ? spec.transformOutput(report) : report) @@ -142,7 +134,6 @@ export function defineExternalCheck(spec: ExternalCheckSpec): Check { if (maxWarnings !== undefined && spec.maxWarnings) { const budget = spec.maxWarnings if (budget.strategy === 'count') return runCountedBudget(spec, budget, maxWarnings, extraArgs, envWithLocalBin()) - // A `flag`-strategy budget just augments the normal command with the tool's own budget option (budget wins last). return runReport(appendArgs(selectCommand(spec, resolveMode()), [...extraArgs, ...budget.toArgs(maxWarnings)])) } return runReport(appendArgs(selectCommand(spec, resolveMode()), extraArgs)) diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts index e987733..69696f9 100644 --- a/src/checks/maxWarnings.ts +++ b/src/checks/maxWarnings.ts @@ -4,29 +4,16 @@ import path from 'node:path' import { appendArgs, captureCommand } from '../shared/spawn.ts' -/** Context handed to a counting run: the user's passthrough args, the PATH-augmented env, and the check's own command. */ type MaxWarningsCountContext = { extraArgs: string[]; env: Record; checkCommand: string } -/** - * How a check applies a `--max-warnings ` budget: - * - `flag`: the tool has its own budget option, so map n to the args to append — the tool's exit code is the verdict, - * which keeps any independent failures (e.g. knip config hints, operational errors) enforced regardless of n. - * - `count`: the tool has no native gate, so verifyx counts findings from its machine-readable output and compares to n. - */ export type MaxWarningsSupport = | { strategy: 'flag'; toArgs: (maxWarnings: number) => string[] } | { strategy: 'count'; unit: string; count: (ctx: MaxWarningsCountContext) => Promise } -/** A counted check passes while findings stay at or below the budget; the budget itself is tolerated (inclusive). */ export function withinBudget(count: number, maxWarnings: number): boolean { return count <= maxWarnings } -/** - * Number of duplicate clones in a jscpd JSON report. A valid report always carries a non-negative integer - * `statistics.total.clones` (or at least a `duplicates` array); any other shape is unrecognised and throws, so a - * degraded or version-shifted report fails loudly rather than silently counting as zero. - */ export function countJscpdClones(report: { statistics?: { total?: { clones?: unknown } }; duplicates?: unknown }): number { const clones = report.statistics?.total?.clones if (typeof clones === 'number' && Number.isInteger(clones) && clones >= 0) return clones @@ -34,22 +21,17 @@ export function countJscpdClones(report: { statistics?: { total?: { clones?: unk throw new Error('unrecognised jscpd JSON report (no non-negative integer statistics.total.clones and no duplicates array)') } -/** - * Count jscpd's clones. jscpd has no stdout JSON reporter, so run the check command (reusing its scan config) with a - * JSON reporter written into a throwaway temp dir; the appended reporter flags override the command's console reporter. - */ +// jscpd writes JSON reports to disk; later reporter flags override its configured console reporter. export async function jscpdCount(ctx: MaxWarningsCountContext): Promise { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verifyx-jscpd-')) try { const command = `${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}" --silent` const { code, stderr } = await captureCommand(command, { env: ctx.env }) const reportPath = path.join(dir, 'jscpd-report.json') - // No report means the run itself failed (e.g. a passthrough flag colliding with the appended ones) — surface the - // tool's own stderr so the "could not count" error is actionable, rather than a bare ENOENT from reading the file. if (!fs.existsSync(reportPath)) throw new Error(`jscpd produced no report (exit ${code})${stderr.trim() ? `: ${stderr.trim()}` : ''}`) return countJscpdClones(JSON.parse(fs.readFileSync(reportPath, 'utf8'))) } finally { - // maxRetries covers a transient Windows lock (AV/indexer) on the just-written report so cleanup can't fail the check. + // Retry transient Windows locks on the new report. fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3 }) } } diff --git a/src/checks/registry.ts b/src/checks/registry.ts index 9b4cb76..a89826f 100644 --- a/src/checks/registry.ts +++ b/src/checks/registry.ts @@ -70,7 +70,6 @@ export const CHECKS: Check[] = [ checkCommand: 'knip --no-progress --treat-config-hints-as-errors', devDeps: ['knip'], docs: 'https://knip.dev/reference/configuration', - // knip has a native budget (--max-issues counts error-level findings); config hints / errors still fail regardless. maxWarnings: { strategy: 'flag', toArgs: (n) => ['--max-issues', String(n)] }, }), defineExternalCheck({ @@ -93,7 +92,6 @@ export const CHECKS: Check[] = [ // NO_COLOR/FORCE_COLOR, so strip the red foreground from its output; the table renders in the default colour. transformOutput: withoutRed, docs: 'https://github.com/kucherenko/jscpd/tree/master/apps/jscpd#config', - // jscpd has no native clone-count gate (only a duplication-percentage --threshold), so verifyx counts clones. maxWarnings: { strategy: 'count', unit: 'clone', count: jscpdCount }, }), ] diff --git a/src/checks/types.ts b/src/checks/types.ts index b86897a..0a248fd 100644 --- a/src/checks/types.ts +++ b/src/checks/types.ts @@ -15,10 +15,6 @@ export type CheckResult = { export type RunDefaultOptions = { /** Extra arguments appended verbatim to an external check's underlying command (e.g. `verifyx circular-deps -- src/*.ts`). */ extraArgs?: string[] - /** - * When set, an external check that supports counting (currently `unused-code`, `duplicate-code`) tolerates up to - * this many findings, failing only when the count exceeds it. Ignored by checks without a counting capability. - */ maxWarnings?: number } @@ -29,7 +25,6 @@ export type Check = { kind: CheckKind /** Whether `verifyx init` preselects this check as a recommended default. */ recommended: boolean - /** Whether this check accepts `--max-warnings ` (counts findings and tolerates up to n). External checks only. */ supportsMaxWarnings?: boolean /** Run the check with its default options and print its own report. Resolves to the outcome. */ runDefault: (options?: RunDefaultOptions) => Promise diff --git a/src/commands/registerChecks.ts b/src/commands/registerChecks.ts index 9e76178..8ef879c 100644 --- a/src/commands/registerChecks.ts +++ b/src/commands/registerChecks.ts @@ -14,7 +14,6 @@ function collect(value: string, previous: string[]): string[] { return [...previous, value] } -/** Commander arg parser for `--max-warnings `: a safe non-negative integer, else a clean CLI error. */ export function parseMaxWarnings(raw: string): number { const value = Number(raw.trim()) if (!/^\d+$/.test(raw.trim()) || !Number.isSafeInteger(value)) { diff --git a/src/shared/spawn.ts b/src/shared/spawn.ts index 9dbc6de..df141ed 100644 --- a/src/shared/spawn.ts +++ b/src/shared/spawn.ts @@ -2,7 +2,7 @@ import { spawn } from 'node:child_process' import { emit, isCapturing } from './output.ts' -/** Append user/scaffold `extraArgs` to a command, preserving shell semantics (globs unquoted). */ +// Keep passthrough globs unquoted for shell expansion. export function appendArgs(command: string, extraArgs: readonly string[] = []): string { return extraArgs.length > 0 ? `${command} ${extraArgs.join(' ')}` : command } @@ -62,10 +62,6 @@ export function runCommand(command: string, opts: RunCommandOptions = {}): Promi }) } -/** - * Run a command and return its captured stdout/stderr as strings (never emitted to the output buffer), for callers - * that need to parse a tool's machine-readable output — e.g. counting findings for `--max-warnings`. - */ export function captureCommand( command: string, opts: { cwd?: string; env?: Record } = {}, From 5924e7ef7eccd194dfc45bfdb9413756e2769fe9 Mon Sep 17 00:00:00 2001 From: Hoang Dinh <> Date: Wed, 15 Jul 2026 13:40:29 +1000 Subject: [PATCH 7/7] fix(checks): run jscpd once for a failing duplicate-code --max-warnings jscpd reporter flags accumulate, so the counting run can keep the configured console reporter alongside the appended json reporter: capture its output during the count and print it on the failure path instead of re-invoking the full check command. --- src/checks/external.test.ts | 41 ++++++++++++++----------------------- src/checks/external.ts | 14 ++++++------- src/checks/maxWarnings.ts | 22 ++++++++++++++------ 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/src/checks/external.test.ts b/src/checks/external.test.ts index 3e0b0c0..ab7ee62 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' +import { runCaptured } from '../shared/output.ts' import { appendArgs, defineExternalCheck, externalFailureHint, runCountedBudget, selectCommand } from './external.ts' const fixable = { checkCommand: 'oxfmt --check .', fixCommand: 'oxfmt .' } @@ -82,44 +83,32 @@ describe('defineExternalCheck', () => { describe('runCountedBudget', () => { const spec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } - const budget = (count: () => Promise) => ({ strategy: 'count' as const, unit: 'clone', count: () => count() }) + const budget = (count: number, report = '') => ({ strategy: 'count' as const, unit: 'clone', count: async () => ({ count, report }) }) it('passes when the finding count is at or below the budget', async () => { - expect( - await runCountedBudget( - spec, - budget(async () => 5), - 5, - [], - {}, - ), - ).toEqual({ name: 'duplicate-code', ok: true }) + expect(await runCountedBudget(spec, budget(5), 5, [], {})).toEqual({ name: 'duplicate-code', ok: true }) }) - it('fails when the finding count exceeds the budget', async () => { - const displaySpec = { ...spec, checkCommand: 'node -e ""' } + it('fails over budget, printing the counting run’s report instead of re-running the tool', async () => { + const transformingSpec = { ...spec, transformOutput: (output: string) => output.toUpperCase() } const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { - expect( - ( - await runCountedBudget( - displaySpec, - budget(async () => 6), - 5, - [], - {}, - ) - ).ok, - ).toBe(false) + const { result, output } = await runCaptured(() => runCountedBudget(transformingSpec, budget(6, 'clone report'), 5, [], {})) + expect(result.ok).toBe(false) + expect(output).toBe('CLONE REPORT') } finally { spy.mockRestore() } }) it('fails loudly when counting throws instead of silently passing', async () => { - const throwing = budget(async () => { - throw new Error('jscpd crashed') - }) + const throwing = { + strategy: 'count' as const, + unit: 'clone', + count: async (): Promise<{ count: number; report: string }> => { + throw new Error('jscpd crashed') + }, + } const spy = vi.spyOn(console, 'error').mockImplementation(() => {}) try { const result = await runCountedBudget(spec, throwing, 5, [], {}) diff --git a/src/checks/external.ts b/src/checks/external.ts index 0870f1f..e43230f 100644 --- a/src/checks/external.ts +++ b/src/checks/external.ts @@ -4,7 +4,7 @@ import path from 'node:path' import { color } from '../shared/color.ts' import { resolveMode } from '../shared/mode.ts' import { emit } from '../shared/output.ts' -import { appendArgs, captureCommand, runCommand } from '../shared/spawn.ts' +import { appendArgs, runCommand } from '../shared/spawn.ts' import { type MaxWarningsSupport, withinBudget } from './maxWarnings.ts' import type { Check, CheckMode, CheckResult, RunDefaultOptions } from './types.ts' @@ -68,7 +68,7 @@ export function externalFailureHint(spec: Pick -type CountableSpec = Pick +type CountableSpec = Pick export async function runCountedBudget( spec: CountableSpec, @@ -77,9 +77,9 @@ export async function runCountedBudget( extraArgs: string[], env: Record, ): Promise { - let count: number + let counted: { count: number; report: string } try { - count = await budget.count({ extraArgs, env, checkCommand: spec.checkCommand }) + counted = await budget.count({ extraArgs, env, checkCommand: spec.checkCommand }) } catch (error) { const docs = spec.docs ? ` — ${spec.docs}` : '' console.error( @@ -89,15 +89,13 @@ export async function runCountedBudget( ) return { name: spec.name, ok: false } } + const { count, report } = counted if (withinBudget(count, maxWarnings)) return { name: spec.name, ok: true } const unit = count === 1 ? budget.unit : `${budget.unit}s` console.error(color.dim(`↳ ${spec.name}: ${count} ${unit} found, exceeds --max-warnings ${maxWarnings}.`)) - const command = appendArgs(selectCommand(spec, resolveMode()), extraArgs) - const { stdout, stderr } = await captureCommand(command, { env }) - const report = stdout + stderr if (report) emit(spec.transformOutput ? spec.transformOutput(report) : report) - console.error(color.dim(externalFailureHint(spec, command))) + console.error(color.dim(externalFailureHint(spec, appendArgs(spec.checkCommand, extraArgs)))) return { name: spec.name, ok: false } } diff --git a/src/checks/maxWarnings.ts b/src/checks/maxWarnings.ts index 69696f9..3cf852b 100644 --- a/src/checks/maxWarnings.ts +++ b/src/checks/maxWarnings.ts @@ -6,9 +6,12 @@ import { appendArgs, captureCommand } from '../shared/spawn.ts' type MaxWarningsCountContext = { extraArgs: string[]; env: Record; checkCommand: string } +/** The finding count plus the tool's rendered console report, so a failing budget can print it without a second run. */ +export type CountResult = { count: number; report: string } + export type MaxWarningsSupport = | { strategy: 'flag'; toArgs: (maxWarnings: number) => string[] } - | { strategy: 'count'; unit: string; count: (ctx: MaxWarningsCountContext) => Promise } + | { strategy: 'count'; unit: string; count: (ctx: MaxWarningsCountContext) => Promise } export function withinBudget(count: number, maxWarnings: number): boolean { return count <= maxWarnings @@ -21,15 +24,22 @@ export function countJscpdClones(report: { statistics?: { total?: { clones?: unk throw new Error('unrecognised jscpd JSON report (no non-negative integer statistics.total.clones and no duplicates array)') } -// jscpd writes JSON reports to disk; later reporter flags override its configured console reporter. -export async function jscpdCount(ctx: MaxWarningsCountContext): Promise { +// jscpd reporter flags accumulate, so appending `--reporters json` keeps the configured console reporter running: +// one invocation yields both the clone count (JSON on disk) and the console report (captured for the failure path). +export async function jscpdCount(ctx: MaxWarningsCountContext): Promise { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'verifyx-jscpd-')) try { - const command = `${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}" --silent` - const { code, stderr } = await captureCommand(command, { env: ctx.env }) + const command = `${appendArgs(ctx.checkCommand, ctx.extraArgs)} --reporters json --output "${dir}"` + const { code, stdout, stderr } = await captureCommand(command, { env: ctx.env }) const reportPath = path.join(dir, 'jscpd-report.json') if (!fs.existsSync(reportPath)) throw new Error(`jscpd produced no report (exit ${code})${stderr.trim() ? `: ${stderr.trim()}` : ''}`) - return countJscpdClones(JSON.parse(fs.readFileSync(reportPath, 'utf8'))) + const count = countJscpdClones(JSON.parse(fs.readFileSync(reportPath, 'utf8'))) + // Drop the json reporter's "report saved to " line — the temp dir is deleted before anyone could read it. + const report = (stdout + stderr) + .split('\n') + .filter((line) => !line.includes(dir)) + .join('\n') + return { count, report } } finally { // Retry transient Windows locks on the new report. fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3 })