Support max-warnings for unused code and duplicate code#16
Merged
Conversation
added 6 commits
July 14, 2026 15:22
…code Both checks gain an ESLint-style `--max-warnings <n>` 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:<name> overrides; verifyx all with no override stays zero-tolerance.
- 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.
- 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).
staff0rd
reviewed
Jul 15, 2026
staff0rd
left a comment
staff0rd
left a comment
Member
There was a problem hiding this comment.
Code review summary
Findings: 1 (blocker 0, major 0, minor 0, nit 1)
This is a focused, well-tested change adding a --max-warnings budget to the unused-code and duplicate-code checks, cleanly separating the pass-through flag strategy (knip's --max-issues) from the count strategy (jscpd JSON report parsing). Both reviewers found no blocking, major, or minor issues; the code fails loudly rather than silently on counting errors, validates report shapes, and rejects invalid budget inputs. The only note is a minor efficiency trade-off on the failure path.
Findings
- nit: jscpd runs twice on a failing
duplicate-code --max-warnings
staff0rd
approved these changes
Jul 15, 2026
staff0rd
left a comment
staff0rd
left a comment
Member
There was a problem hiding this comment.
nice one - this approach works well for legacy projects where the cost of adding verify and refactoring is too high because of existing non-compliance.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
--max-warnings <n>support to the unused-code and duplicate-code checks.