From b8d7aa50eb2897a096749ea68f2087d69ac6ad34 Mon Sep 17 00:00:00 2001 From: Christopher Monsanto Date: Mon, 17 Aug 2026 19:03:56 -0400 Subject: [PATCH] Replace non-null assertions with explicit checks Parsers use charAt or check-and-throw; the executor gets a paired() helper for its parallel arrays; matchSubsets returns {entry, matched} pairs so the deploy loop stops indexing two arrays; spawn stdin and pid get real guards. Co-Authored-By: Claude Fable 5 --- data/lib/index.ts | 15 +++++++------- rules/publish.ts | 6 +++--- tools/build/artifact.ts | 11 ++++++++-- tools/build/exec.ts | 5 ++++- tools/build/executor.ts | 30 ++++++++++++++++++++------- tools/build/helpers.ts | 2 +- tools/build/subst.ts | 6 +++--- tools/deploy/api.ts | 5 +++-- tools/deploy/config.ts | 6 +++--- tools/deploy/index.ts | 35 ++++++++++++++++++-------------- tools/deploy/test/config.test.ts | 5 +++-- 11 files changed, 80 insertions(+), 46 deletions(-) diff --git a/data/lib/index.ts b/data/lib/index.ts index 93f95618..c6130b9e 100644 --- a/data/lib/index.ts +++ b/data/lib/index.ts @@ -70,7 +70,7 @@ export function parseFilename(s: string): SpriteFilename { if (s.length < 2) throw new Error(`Filename ${s} needs to be at least 2 characters'`); - let prefix = s[0]!; + let prefix = s.charAt(0); if (!prefix.match(/[a-z]/)) throw new Error(`Filename ${s} must start with alpha character`); @@ -79,15 +79,16 @@ export function parseFilename(s: string): SpriteFilename { for (let part of parts.slice(1)) { if (part.length === 0) throw new Error(`Can't parse ${s}`); - extra.set(part[0]!, part.slice(1)); + extra.set(part.charAt(0), part.slice(1)); } - + + let first = parts[0]; + if (first === undefined) + throw new Error(`Can't parse ${s}`); if (prefix === 'x') { - let name = parts[0]!.slice(1); - return {extension: true, name, extra}; + return {extension: true, name: first.slice(1), extra}; } else { - let id = parts[0]!; - return {extension: false, id, extra}; + return {extension: false, id: first, extra}; } } diff --git a/rules/publish.ts b/rules/publish.ts index ff75b087..74242ffb 100644 --- a/rules/publish.ts +++ b/rules/publish.ts @@ -28,10 +28,10 @@ export class Manifest { write(dst: string): void { let sorted: Record = {}; - for (let k of [...this.#entries.keys()].sort()) { - sorted[k] = this.#entries.get(k)!; + for (let [k, v] of [...this.#entries].sort((a, b) => a[0] < b[0] ? -1 : 1)) { + sorted[k] = v; } - this.ctx.write(dst, JSON.stringify(sorted, null, 4) + "\n"); + this.ctx.write(dst, JSON.stringify(sorted, null, 4) + '\n'); } } diff --git a/tools/build/artifact.ts b/tools/build/artifact.ts index 0165c1f3..05086532 100644 --- a/tools/build/artifact.ts +++ b/tools/build/artifact.ts @@ -148,7 +148,7 @@ function makeDecl(inputs: Input[], deps: Input[], spec: CmdSpec, outputs: string throw new Error(`Rule with no commands (outputs: ${outputs.join(' ')})`); } if (outputs.length === 0) { - throw new Error(`Rule with no outputs (cmds: ${cmds[0]!})`); + throw new Error(`Rule with no outputs (cmds: ${cmds[0] ?? ''})`); } // An identical declaration returns the already-registered rule. Artifact @@ -225,7 +225,14 @@ export function rule(input: Input | Input[], spec: CmdSpec | subst.Cmd[], output: string | readonly string[]): Artifact | Artifact[] { let s = normalizeSpec(spec); let decl = makeDecl(resolveInputs(input), resolveInputs(s.deps), s, astable(output)); - return typeof output === 'string' ? decl.outputs[0]! : decl.outputs; + if (typeof output !== 'string') { + return decl.outputs; + } + let first = decl.outputs[0]; + if (first === undefined) { + throw new Error('Rule with no outputs'); + } + return first; } export function forEachRule(input: Input | Input[], spec: CmdSpec | subst.Cmd[], diff --git a/tools/build/exec.ts b/tools/build/exec.ts index 1e991148..82779146 100644 --- a/tools/build/exec.ts +++ b/tools/build/exec.ts @@ -39,8 +39,11 @@ export function runShell(command: string, opts: {cwd: string, signal: AbortSigna let killTimer: NodeJS.Timeout | undefined; let kill = (sig: NodeJS.Signals) => { + if (child.pid === undefined) { + return; + } try { - process.kill(-child.pid!, sig); + process.kill(-child.pid, sig); } catch {} }; let onAbort = () => { diff --git a/tools/build/executor.ts b/tools/build/executor.ts index d4039960..8caca175 100644 --- a/tools/build/executor.ts +++ b/tools/build/executor.ts @@ -40,7 +40,17 @@ export type ExecutorOpts = { }; export function label(decl: artifact.RuleDecl): string { - return decl.display ?? decl.cmds[0]!; + return decl.display ?? decl.cmds[0] ?? '(no commands)'; +} + +// Parallel-array access: the arrays are constructed the same length, so a +// miss is a bug worth crashing on. +function paired(xs: readonly T[], n: number): T { + let x = xs[n]; + if (x === undefined) { + throw new Error('parallel array length mismatch'); + } + return x; } function indent(text: string): string { @@ -153,7 +163,7 @@ export class Executor { try { let inputs = [...decl.inputs, ...decl.deps]; let resolved = await Promise.all(inputs.map(i => this.#digestOf(i))); - digests = new Map(inputs.map((i, n) => [i, resolved[n]!])); + digests = new Map(inputs.map((i, n) => [i, paired(resolved, n)])); } catch (err) { if (err instanceof RuleFailed) { this.#outcomes.set(decl, {status: 'blocked'}); @@ -164,7 +174,13 @@ export class Executor { throw err; } - let key = artifact.computeKey(decl, i => digests.get(i)!); + let key = artifact.computeKey(decl, i => { + let d = digests.get(i); + if (d === undefined) { + throw new Error(`No digest for input ${typeof i === 'string' ? i : i.filename}`); + } + return d; + }); this.#keys.set(decl, key); // Byte-identical duplicate declarations share one execution. @@ -172,7 +188,7 @@ export class Executor { if (existing !== undefined) { try { let shared = await existing; - decl.outputs.forEach((o, n) => o.resolve(shared[n]!)); + decl.outputs.forEach((o, n) => o.resolve(paired(shared, n))); this.#outcomes.set(decl, {status: 'clean'}); return shared; } catch (err) { @@ -187,7 +203,7 @@ export class Executor { let work = this.#perform(decl, key); this.#inflightByKey.set(key, work); let result = await work; - decl.outputs.forEach((o, n) => o.resolve(result[n]!)); + decl.outputs.forEach((o, n) => o.resolve(paired(result, n))); return result; } @@ -196,7 +212,7 @@ export class Executor { let stored = store.lookupRule(key); if (stored !== null && stored.length === decl.outputs.length - && stored.every((o, n) => o.ext === decl.outputs[n]!.ext) + && stored.every((o, n) => o.ext === paired(decl.outputs, n).ext) && stored.every(o => cas.casStat(casDir, o.digest, o.ext) === o.size)) { this.#outcomes.set(decl, {status: 'clean'}); return stored.map(o => o.digest); @@ -238,7 +254,7 @@ export class Executor { let missing = tempOutputs.filter(p => !fs.existsSync(p)); if (missing.length === 0) { let outputs = decl.outputs.map((o, n) => { - let object = cas.casInsert(casDir, tempOutputs[n]!, o.ext); + let object = cas.casInsert(casDir, paired(tempOutputs, n), o.ext); return {digest: object.digest, ext: o.ext, size: object.size}; }); store.recordRule(key, decl.cmds, outputs); diff --git a/tools/build/helpers.ts b/tools/build/helpers.ts index 1f981cce..186c154f 100644 --- a/tools/build/helpers.ts +++ b/tools/build/helpers.ts @@ -79,7 +79,7 @@ export function spritedata(basename: string): SpriteData { if (part.length === 1) { data[part] = true; } else { - data[part[0]!] = part.slice(1); + data[part.charAt(0)] = part.slice(1); } } return {id: parts[0] ?? '', data}; diff --git a/tools/build/subst.ts b/tools/build/subst.ts index c6b83965..2e294d8d 100644 --- a/tools/build/subst.ts +++ b/tools/build/subst.ts @@ -35,11 +35,11 @@ export function substituteNames(s: string, inputs: string[]): string { export function substitute(s: string, inputs: string[], outputs: string[]): string { return s.replace(/%o(\d+)|%([a-zA-Z])/g, (match, n: string | undefined, c: string | undefined) => { if (n !== undefined) { - let i = Number(n); - if (i < 1 || i > outputs.length) { + let out = outputs[Number(n) - 1]; + if (out === undefined) { throw new Error(`Output index out of range (${outputs.length} outputs): ${match} in: ${s}`); } - return outputs[i - 1]!; + return out; } switch (c) { case 'f': return inputs.join(' '); diff --git a/tools/deploy/api.ts b/tools/deploy/api.ts index a8057b5c..43ad1c8e 100644 --- a/tools/deploy/api.ts +++ b/tools/deploy/api.ts @@ -91,8 +91,9 @@ export function makeCtx(casDir: string, queue: ActionQueue): DeployCtx { return result; }, hash(...srcs: CopySource[]): string { - if (srcs.length === 1) { - return shortHash(digestOf(srcs[0]!)); + let [only] = srcs; + if (only !== undefined && srcs.length === 1) { + return shortHash(digestOf(only)); } let digests = srcs.map(digestOf).sort(Buffer.compare); let h = crypto.createHash('sha256'); diff --git a/tools/deploy/config.ts b/tools/deploy/config.ts index 4b76fda5..fd99400e 100644 --- a/tools/deploy/config.ts +++ b/tools/deploy/config.ts @@ -70,8 +70,8 @@ export function loadDeployConfig(path: string): DeployConfig { // Route finish outputs to deploy entries: per entry, the set of dsts its // subset globs match. Every glob must match something and every dst must be // covered by some entry; to unship an output, don't emit it in finish. -export function matchSubsets(dsts: readonly string[], - entries: readonly DeployEntry[]): Set[] { +export function matchSubsets(dsts: readonly string[], entries: readonly DeployEntry[]) + : {entry: DeployEntry, matched: Set}[] { let covered = new Set(); let perEntry = entries.map(entry => { let matched = new Set(); @@ -85,7 +85,7 @@ export function matchSubsets(dsts: readonly string[], covered.add(hit); } } - return matched; + return {entry, matched}; }); let uncovered = dsts.filter(d => !covered.has(d)); if (uncovered.length > 0) { diff --git a/tools/deploy/index.ts b/tools/deploy/index.ts index 26e6dcaf..d559fd0c 100644 --- a/tools/deploy/index.ts +++ b/tools/deploy/index.ts @@ -232,25 +232,24 @@ async function cmdDeploy(names: string[], opts: VerbOpts): Promise { } return; } - for (let name of names) { - if (!config.has(name)) { + let targets = names.map(name => { + let target = config.get(name); + if (target === undefined) { throw new BuildError(`deploy.json5: no deploy named ${name}`); } - } + return {name, target}; + }); setConfig(loadConfig(opts.config)); - let files = [...new Set(names.map(n => config.get(n)!.buildFile))]; + let files = [...new Set(targets.map(t => t.target.buildFile))]; let specs = await importDeploys(files); process.exitCode = await buildThen(artifact.getDecls(), opts, false, async () => { - for (let name of names) { - let target = config.get(name)!; + for (let {name, target} of targets) { let aq = await runFinish(finishOf(specs, target.buildFile), Boolean(opts.verbose)); if (aq === null) { return 1; } let dsts = aq.log.filter(e => e.type === 'Op').map(e => e.dst); - let subsets = matchSubsets(dsts, target.deploy); - for (let [i, entry] of target.deploy.entries()) { - let matched = subsets[i]!; + for (let [i, {entry, matched}] of matchSubsets(dsts, target.deploy).entries()) { console.log(`${name}: ${matched.size} files | ${entry.cmd}`); // Debug mode: materialize what each entry would ship (tar // entries included) instead of running its command. @@ -276,13 +275,17 @@ async function cmdDeploy(names: string[], opts: VerbOpts): Promise { continue; } let upload = spawn(entry.cmd, {shell: true, stdio: ['pipe', 'inherit', 'inherit']}); + let stdin = upload.stdin; + if (stdin === null) { + throw new BuildError(`no stdin pipe for: ${entry.cmd}`); + } // If the command dies early we report its exit code; don't // also crash on the resulting EPIPE, which reaches both // stdin and (via streamx's destroy propagation) the pack. - upload.stdin!.on('error', () => {}); + stdin.on('error', () => {}); let pack = aq.pack(dst => matched.has(dst)); pack.on('error', () => {}); - pack.pipe(upload.stdin!); + pack.pipe(stdin); if (await waitExit(upload) !== 0) { return 1; } @@ -307,7 +310,7 @@ async function cmdRun(file: string, opts: VerbOpts): Promise { } function slugOf(decl: artifact.RuleDecl): string { - let template = decl.displayTemplate ?? decl.cmds[0]!; + let template = decl.displayTemplate ?? decl.cmds[0] ?? ''; let slug = template.replace(/%[a-zA-Z0-9]+/g, ' ') .toLowerCase().replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, ''); return slug === '' ? 'rule' : slug; @@ -413,11 +416,13 @@ async function main(argv: string[]): Promise { return cmdBuild(positionals, opts); case 'deploy': return cmdDeploy(positionals, opts); - case 'run': - if (positionals.length !== 1) { + case 'run': { + let [file, ...extra] = positionals; + if (file === undefined || extra.length > 0) { throw new BuildError('run takes exactly one deploy file'); } - return cmdRun(positionals[0]!, opts); + return cmdRun(file, opts); + } case 'inspect': if (positionals.length === 0) { throw new BuildError('inspect takes at least one source path'); diff --git a/tools/deploy/test/config.test.ts b/tools/deploy/test/config.test.ts index 1ea1bab5..86a6a0e8 100644 --- a/tools/deploy/test/config.test.ts +++ b/tools/deploy/test/config.test.ts @@ -59,8 +59,9 @@ test('matchSubsets routes dsts to entries, overlap allowed', () => { {subset: ['xy/**', 'xyicons/**'], cmd: 'one'}, {subset: ['**/manifest.json'], cmd: 'two'}, ]); - assert.deepEqual([...xy!].sort(), dsts); - assert.deepEqual([...manifests!], ['xy/manifest.json']); + assert.deepEqual([...xy!.matched].sort(), dsts); + assert.equal(xy!.entry.cmd, 'one'); + assert.deepEqual([...manifests!.matched], ['xy/manifest.json']); }); test('matchSubsets errors on a glob matching nothing', () => {