Skip to content

Commit 384efa8

Browse files
author
bgagent
committed
fix(cdk): restore exhaustiveness guard + correct knip ratchet keys (#282)
Addresses PR review (ayushtr-aws): 1. validation.ts: the `void (true as _AssertAttachmentExhaustive)` form silently defeated the AttachmentType exhaustiveness guard — a type assertion never errors, whereas the original ASSIGNMENT to a `never` type is the actual compile-time check. Restored the assignment and consume the binding with `void` so it survives noUnusedLocals and no-unused-vars without weakening the guard. Verified: tsc errors when a member is missing from ATTACHMENT_TYPE_LIST, compiles clean when exhaustive. 2. check-deadcode-ratchet.mjs: COUNTED_KEYS used non-existent knip 6.x keys (classMembers, nsExports, nsTypes) and counted unused files from a non-existent top-level report.files. Switched to the real keys (namespaceMembers, catalog) and count files from issues[].files[] so unused files actually move the ratchet. Count unchanged at 78.
1 parent ea6d75a commit 384efa8

2 files changed

Lines changed: 21 additions & 9 deletions

File tree

cdk/src/handlers/shared/validation.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,14 @@ export const MAX_TOTAL_ATTACHMENT_SIZE_BYTES = MAX_TOTAL_ATTACHMENT_SIZE_MB * 10
286286
/** Compile-time exhaustiveness check for AttachmentType. */
287287
const ATTACHMENT_TYPE_LIST = ['image', 'file', 'url'] as const satisfies readonly AttachmentType[];
288288
type _AssertAttachmentExhaustive = Exclude<AttachmentType, (typeof ATTACHMENT_TYPE_LIST)[number]> extends never ? true : never;
289-
// `void` keeps the compile-time exhaustiveness assertion without an unused binding
290-
// (tsconfig noUnusedLocals does not honor eslint's ^_ ignore pattern).
291-
void (true as _AssertAttachmentExhaustive);
289+
// The ASSIGNMENT is the guard: if AttachmentType gains a member missing from
290+
// ATTACHMENT_TYPE_LIST, _AssertAttachmentExhaustive resolves to `never` and
291+
// assigning `true` is a hard compile error. (A `true as never` assertion would
292+
// 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.
295+
const _attachmentExhaustiveCheck: _AssertAttachmentExhaustive = true;
296+
void _attachmentExhaustiveCheck;
292297
const VALID_ATTACHMENT_TYPES = new Set<string>(ATTACHMENT_TYPE_LIST);
293298

294299
/** Allowed image MIME types (PNG and JPEG only — passed directly to Bedrock). */

scripts/check-deadcode-ratchet.mjs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,9 @@
2828
* (someone cleaned up), it prints the new lower number so the baseline can be
2929
* tightened in the same PR.
3030
*
31-
* Counted categories mirror what knip surfaces for this repo today (unused
32-
* files, dependencies, unlisted, binaries, exports, types, enum/class members).
31+
* Counted categories are knip 6.x's array-valued issue keys (unused files,
32+
* dependencies, unlisted/unresolved, binaries, exports, types, enum and
33+
* namespace members, duplicates, catalog) — see COUNTED_KEYS below.
3334
* Per-category false positives are suppressed in `knip.json`, not here.
3435
*/
3536

@@ -41,7 +42,14 @@ import { dirname, join } from 'node:path';
4142
const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..');
4243
const baselinePath = join(repoRoot, 'knip-baseline.json');
4344

45+
// knip 6.x emits one issues[] entry per file; each entry carries these
46+
// array-valued issue keys. `file` and `owners` are labels, not findings, so
47+
// they are excluded. `files` (unused files) is counted here — knip lists them
48+
// inside issues[].files[], NOT at the top level. `duplicates` is an
49+
// array-of-arrays (one inner array per duplicate group), so its `.length`
50+
// counts groups, which is the unit we ratchet on.
4451
const COUNTED_KEYS = [
52+
'files',
4553
'dependencies',
4654
'devDependencies',
4755
'optionalPeerDependencies',
@@ -50,11 +58,10 @@ const COUNTED_KEYS = [
5058
'unresolved',
5159
'exports',
5260
'types',
53-
'nsExports',
54-
'nsTypes',
5561
'duplicates',
5662
'enumMembers',
57-
'classMembers',
63+
'namespaceMembers',
64+
'catalog',
5865
];
5966

6067
function runKnip() {
@@ -80,7 +87,7 @@ function runKnip() {
8087
}
8188

8289
function countIssues(report) {
83-
let total = (report.files ?? []).length;
90+
let total = 0;
8491
for (const issue of report.issues ?? []) {
8592
for (const key of COUNTED_KEYS) {
8693
if (Array.isArray(issue[key])) total += issue[key].length;

0 commit comments

Comments
 (0)