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 <noreply@anthropic.com>
This commit is contained in:
Christopher Monsanto
2026-08-17 19:03:56 -04:00
parent dc2bff9738
commit b8d7aa50eb
11 changed files with 80 additions and 46 deletions

View File

@@ -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};
}
}

View File

@@ -28,10 +28,10 @@ export class Manifest {
write(dst: string): void {
let sorted: Record<string, string> = {};
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');
}
}

View File

@@ -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[],

View File

@@ -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 = () => {

View File

@@ -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<T>(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);

View File

@@ -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};

View File

@@ -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(' ');

View File

@@ -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');

View File

@@ -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<string>[] {
export function matchSubsets(dsts: readonly string[], entries: readonly DeployEntry[])
: {entry: DeployEntry, matched: Set<string>}[] {
let covered = new Set<string>();
let perEntry = entries.map(entry => {
let matched = new Set<string>();
@@ -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) {

View File

@@ -232,25 +232,24 @@ async function cmdDeploy(names: string[], opts: VerbOpts): Promise<void> {
}
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<void> {
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<void> {
}
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<void> {
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');

View File

@@ -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', () => {