Skip to content

Commit 9e57b44

Browse files
author
bgagent
committed
chore(ci): harden dead-code ratchet + comment accuracy (#282 review nits)
Non-blocking follow-ups from krokoko's review: - Pin knip exactly (6.20.0, drop caret) and make the ratchet fail loud instead of open: if a future knip reshapes its JSON so `issues` is not an array, countIssues now exits 2 rather than silently counting 0 and reporting a passing build (which would disable the gate). Needed before the gate flips to blocking. - COUNTED_KEYS comment: note the list is complete for the pinned knip and that nsExports/nsTypes/classMembers are not part of its schema. - validation.ts: attribute the `void` to tsconfig noUnusedLocals specifically (no-unused-vars already exempts the ^_ binding).
1 parent 68df6c8 commit 9e57b44

4 files changed

Lines changed: 24 additions & 5 deletions

File tree

cdk/src/handlers/shared/validation.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,9 @@ type _AssertAttachmentExhaustive = Exclude<AttachmentType, (typeof ATTACHMENT_TY
290290
// ATTACHMENT_TYPE_LIST, _AssertAttachmentExhaustive resolves to `never` and
291291
// assigning `true` is a hard compile error. (A `true as never` assertion would
292292
// NOT error — assertions are permitted whenever either side is assignable — so
293-
// it must stay an assignment.) `void` consumes the binding so noUnusedLocals
294-
// and @typescript-eslint/no-unused-vars stay satisfied without weakening it.
293+
// it must stay an assignment.) `void` consumes the binding to satisfy
294+
// tsconfig noUnusedLocals, which — unlike @typescript-eslint/no-unused-vars,
295+
// already exempt via its ^_ varsIgnorePattern — does not honor the _ prefix.
295296
const _attachmentExhaustiveCheck: _AssertAttachmentExhaustive = true;
296297
void _attachmentExhaustiveCheck;
297298
const VALID_ATTACHMENT_TYPES = new Set<string>(ATTACHMENT_TYPE_LIST);

package.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/check-deadcode-ratchet.mjs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ const baselinePath = join(repoRoot, 'knip-baseline.json');
4848
// inside issues[].files[], NOT at the top level. `duplicates` is an
4949
// array-of-arrays (one inner array per duplicate group), so its `.length`
5050
// counts groups, which is the unit we ratchet on.
51+
//
52+
// This is the complete set of countable keys for the installed knip (6.20.0,
53+
// pinned exactly). There is no `nsExports`/`nsTypes`/`classMembers` in this
54+
// schema — namespace/enum members surface as `namespaceMembers`/`enumMembers`.
55+
// If knip is bumped, re-derive this list from its JSON (the countIssues guard
56+
// below will fail loud if the top-level shape changes).
5157
const COUNTED_KEYS = [
5258
'files',
5359
'dependencies',
@@ -87,8 +93,20 @@ function runKnip() {
8793
}
8894

8995
function countIssues(report) {
96+
// Fail loud, not open: if a future knip reshapes its JSON so `issues` is no
97+
// longer an array, `?? []` would yield count 0 — silently below any baseline,
98+
// reporting a passing build and disabling the gate. Require the array shape so
99+
// a schema change surfaces as an error instead. (knip is pinned exactly in
100+
// package.json to make such a change a deliberate, reviewed bump.)
101+
if (!Array.isArray(report.issues)) {
102+
console.error(
103+
'check-deadcode-ratchet: knip JSON has no `issues` array — its output schema may have changed. ' +
104+
'Re-validate COUNTED_KEYS against the installed knip before trusting the count.',
105+
);
106+
process.exit(2);
107+
}
90108
let total = 0;
91-
for (const issue of report.issues ?? []) {
109+
for (const issue of report.issues) {
92110
for (const key of COUNTED_KEYS) {
93111
if (Array.isArray(issue[key])) total += issue[key].length;
94112
}

yarn.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)