diff --git a/README.md b/README.md index befd121..8d02dff 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) +### `--max-warnings` + +`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 +``` + ### `complexity` ```sh @@ -316,6 +328,8 @@ 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() + +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/src/checks/external.test.ts b/src/checks/external.test.ts index 62d8a69..ab7ee62 100644 --- a/src/checks/external.test.ts +++ b/src/checks/external.test.ts @@ -1,6 +1,7 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' -import { appendArgs, defineExternalCheck, externalFailureHint, selectCommand } from './external.ts' +import { runCaptured } from '../shared/output.ts' +import { appendArgs, defineExternalCheck, externalFailureHint, runCountedBudget, selectCommand } from './external.ts' const fixable = { checkCommand: 'oxfmt --check .', fixCommand: 'oxfmt .' } const notFixable = { checkCommand: 'tsc --noEmit' } @@ -79,3 +80,42 @@ describe('defineExternalCheck', () => { expect(knip.scaffold.script).toBe('verifyx unused-code') }) }) + +describe('runCountedBudget', () => { + const spec = { name: 'duplicate-code', bin: 'jscpd', checkCommand: 'jscpd src', docs: 'https://x' } + 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(5), 5, [], {})).toEqual({ name: 'duplicate-code', ok: true }) + }) + + 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 { + 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 = { + 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, [], {}) + 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..e43230f 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, 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,7 @@ 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 + 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 +67,38 @@ export function externalFailureHint(spec: Pick +type CountableSpec = Pick + +export async function runCountedBudget( + spec: CountableSpec, + budget: CountBudget, + maxWarnings: number, + extraArgs: string[], + env: Record, +): Promise { + let counted: { count: number; report: string } + try { + counted = await budget.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 } + } + 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}.`)) + if (report) emit(spec.transformOutput ? spec.transformOutput(report) : report) + console.error(color.dim(externalFailureHint(spec, appendArgs(spec.checkCommand, extraArgs)))) + 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 +106,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 +114,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,12 +123,18 @@ 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 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 }) - // 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()) + return runReport(appendArgs(selectCommand(spec, resolveMode()), [...extraArgs, ...budget.toArgs(maxWarnings)])) + } + return runReport(appendArgs(selectCommand(spec, resolveMode()), extraArgs)) }, } } diff --git a/src/checks/maxWarnings.test.ts b/src/checks/maxWarnings.test.ts new file mode 100644 index 0000000..d5b1256 --- /dev/null +++ b/src/checks/maxWarnings.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest' + +import { countJscpdClones, withinBudget } from './maxWarnings.ts' + +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) + }) + + 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', () => { + 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..3cf852b --- /dev/null +++ b/src/checks/maxWarnings.ts @@ -0,0 +1,47 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +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 } + +export function withinBudget(count: number, maxWarnings: number): boolean { + return count <= maxWarnings +} + +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)') +} + +// 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}"` + 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()}` : ''}`) + 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 }) + } +} diff --git a/src/checks/registry.ts b/src/checks/registry.ts index b1638c0..a89826f 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 } 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: { strategy: 'flag', toArgs: (n) => ['--max-issues', String(n)] }, }), 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: { strategy: 'count', unit: 'clone', count: jscpdCount }, }), ] diff --git a/src/checks/types.ts b/src/checks/types.ts index bc43239..0a248fd 100644 --- a/src/checks/types.ts +++ b/src/checks/types.ts @@ -15,6 +15,7 @@ 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[] + maxWarnings?: number } /** A single verification. Native checks run in-process; external checks shell out to a tool. */ @@ -24,6 +25,7 @@ export type Check = { kind: CheckKind /** Whether `verifyx init` preselects this check as a recommended default. */ recommended: boolean + 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..78bd4ac --- /dev/null +++ b/src/commands/registerChecks.test.ts @@ -0,0 +1,31 @@ +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/) + }) + + 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 c1b1dbe..8ef879c 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,14 @@ function collect(value: string, previous: string[]): string[] { return [...previous, value] } +export function parseMaxWarnings(raw: string): number { + 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`, …). */ export function registerChecks(program: Command): void { program @@ -71,14 +79,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..df141ed 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' +// Keep passthrough globs unquoted for shell expansion. +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,25 @@ export function runCommand(command: string, opts: RunCommandOptions = {}): Promi child.on('error', () => resolve(127)) }) } + +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'], + 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: '' })) + }) +}