From f9c2e28727cb5e44248e7926ee9bd5db665bdd42 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 7 Apr 2026 08:52:23 -0600 Subject: [PATCH] feat(lib): add NonEmptyArray helper, tighten preferredRunner Adds `lib/helpers/arrays.ts` with a NonEmptyArray type alias and an isNonEmpty type-guard predicate. Tightens preferredRunner to require a NonEmptyArray input and return Runner (no longer | undefined). Updates the two call sites (init/skills.ts, init/format.ts) to use isNonEmpty for the non-empty check, and drops the now-provably-unreachable defensive `if (!preferred)` branch in skills.ts. The type system enforces the invariant that runtime code previously checked. --- .changeset/fiery-comics-shave.md | 2 + packages/cli-core/src/commands/init/format.ts | 4 +- packages/cli-core/src/commands/init/skills.ts | 14 +----- .../cli-core/src/lib/helpers/arrays.test.ts | 40 +++++++++++++++++ packages/cli-core/src/lib/helpers/arrays.ts | 24 +++++++++++ packages/cli-core/src/lib/runners.test.ts | 43 +++++++++++-------- packages/cli-core/src/lib/runners.ts | 10 +++-- 7 files changed, 102 insertions(+), 35 deletions(-) create mode 100644 .changeset/fiery-comics-shave.md create mode 100644 packages/cli-core/src/lib/helpers/arrays.test.ts create mode 100644 packages/cli-core/src/lib/helpers/arrays.ts diff --git a/.changeset/fiery-comics-shave.md b/.changeset/fiery-comics-shave.md new file mode 100644 index 00000000..a845151c --- /dev/null +++ b/.changeset/fiery-comics-shave.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/cli-core/src/commands/init/format.ts b/packages/cli-core/src/commands/init/format.ts index 74176575..272471a1 100644 --- a/packages/cli-core/src/commands/init/format.ts +++ b/packages/cli-core/src/commands/init/format.ts @@ -1,4 +1,5 @@ import { detectAvailableRunners, preferredRunner, runnerCommand } from "../../lib/runners.js"; +import { isNonEmpty } from "../../lib/helpers/arrays.js"; import type { ProjectContext } from "./frameworks/types.js"; type FormatterConfig = { @@ -32,9 +33,8 @@ export async function runFormatters(ctx: ProjectContext, files: string[]): Promi if (matchingFormatters.length === 0) return; const available = detectAvailableRunners(); - if (available.length === 0) return; + if (!isNonEmpty(available)) return; const runner = preferredRunner(ctx.packageManager, available); - if (!runner) return; for (const formatter of matchingFormatters) { const command = runnerCommand(runner, formatter.binArgs(files)); diff --git a/packages/cli-core/src/commands/init/skills.ts b/packages/cli-core/src/commands/init/skills.ts index 7a826d51..96ddef55 100644 --- a/packages/cli-core/src/commands/init/skills.ts +++ b/packages/cli-core/src/commands/init/skills.ts @@ -22,6 +22,7 @@ import { runnerCommand, runnerForPackageManager, } from "../../lib/runners.js"; +import { isNonEmpty } from "../../lib/helpers/arrays.js"; import type { ProjectContext } from "./frameworks/types.js"; /** Skills installed regardless of framework. */ @@ -87,7 +88,7 @@ export async function installSkills( // Detect runners after the user accepts — no point probing PATH if they decline. const available = detectAvailableRunners(); - if (available.length === 0) { + if (!isNonEmpty(available)) { const suggested = runnerForPackageManager(packageManager); log.blank(); log.warn( @@ -98,17 +99,6 @@ export async function installSkills( } const preferred = preferredRunner(packageManager, available); - if (!preferred) { - // Defensive: detectAvailableRunners returned a non-empty array above, so - // preferredRunner should always find something. This guards against any - // future change that decouples the two. - const suggested = runnerForPackageManager(packageManager); - log.blank(); - log.warn( - `Could not select a package runner. Run \`${suggested.display} skills add ${SKILLS_SOURCE}\` manually.`, - ); - return; - } // Only prompt when there's an actual choice and the user is interactive. let runner = preferred; diff --git a/packages/cli-core/src/lib/helpers/arrays.test.ts b/packages/cli-core/src/lib/helpers/arrays.test.ts new file mode 100644 index 00000000..706b6fd6 --- /dev/null +++ b/packages/cli-core/src/lib/helpers/arrays.test.ts @@ -0,0 +1,40 @@ +import { test, expect, describe } from "bun:test"; +import { isNonEmpty, type NonEmptyArray } from "./arrays.ts"; + +describe("isNonEmpty", () => { + test("returns false for an empty array", () => { + expect(isNonEmpty([])).toBe(false); + }); + + test("returns true for an array with one element", () => { + expect(isNonEmpty([1])).toBe(true); + }); + + test("returns true for an array with multiple elements", () => { + expect(isNonEmpty(["a", "b", "c"])).toBe(true); + }); + + test("works with readonly arrays", () => { + const ro: readonly number[] = [1, 2, 3]; + expect(isNonEmpty(ro)).toBe(true); + }); + + test("narrows the type so [0] is non-undefined", () => { + const arr: number[] = [42]; + if (isNonEmpty(arr)) { + // This line is the test — if isNonEmpty didn't narrow, the assignment + // would be `number | undefined` and TS would error under strict mode. + const first: number = arr[0]; + expect(first).toBe(42); + } else { + throw new Error("expected isNonEmpty to return true"); + } + }); + + test("NonEmptyArray accepts a tuple literal", () => { + // Compile-time check: this would error if NonEmptyArray rejected a + // single-element tuple. + const one: NonEmptyArray = ["hi"]; + expect(one).toHaveLength(1); + }); +}); diff --git a/packages/cli-core/src/lib/helpers/arrays.ts b/packages/cli-core/src/lib/helpers/arrays.ts new file mode 100644 index 00000000..6e6ce718 --- /dev/null +++ b/packages/cli-core/src/lib/helpers/arrays.ts @@ -0,0 +1,24 @@ +/** + * Generic array helpers and type-narrowing predicates. + */ + +/** A readonly array proven to contain at least one element. */ +export type NonEmptyArray = readonly [T, ...T[]]; + +/** + * Type guard: narrows a `readonly T[]` to `NonEmptyArray` when it has + * one or more elements. Use at boundaries where you want the type system + * (rather than a runtime null-check) to prove an array is non-empty. + * + * @example + * ```ts + * const items = await getItems(); + * if (!isNonEmpty(items)) { + * return; + * } + * // items is now NonEmptyArray — items[0] is Item, not Item | undefined + * ``` + */ +export function isNonEmpty(arr: readonly T[]): arr is NonEmptyArray { + return arr.length > 0; +} diff --git a/packages/cli-core/src/lib/runners.test.ts b/packages/cli-core/src/lib/runners.test.ts index 4c064944..194a90f3 100644 --- a/packages/cli-core/src/lib/runners.test.ts +++ b/packages/cli-core/src/lib/runners.test.ts @@ -1,12 +1,12 @@ import { test, expect, describe, afterEach } from "bun:test"; import { KNOWN_RUNNERS, - type Runner, detectAvailableRunners, preferredRunner, runnerCommand, runnerForPackageManager, } from "./runners.ts"; +import { isNonEmpty } from "./helpers/arrays.ts"; // Bun.which / Bun.spawnSync are native globals. We patch them directly the // same way commands/auth/login.test.ts patches Bun.spawn — wrapped in @@ -121,31 +121,35 @@ describe("preferredRunner", () => { const pnpm = KNOWN_RUNNERS.find((r) => r.id === "pnpm")!; const yarn = KNOWN_RUNNERS.find((r) => r.id === "yarn")!; - test("returns undefined when no runners are available", () => { - expect(preferredRunner("bun", [])).toBeUndefined(); - expect(preferredRunner(undefined, [])).toBeUndefined(); - }); + // preferredRunner now requires a NonEmptyArray, so the empty-array + // case is unrepresentable in the type system and doesn't need a runtime test. test("returns the runner matching the project's package manager", () => { - const all = [bunx, npx, pnpm, yarn]; - expect(preferredRunner("bun", all)?.id).toBe("bunx"); - expect(preferredRunner("npm", all)?.id).toBe("npx"); - expect(preferredRunner("pnpm", all)?.id).toBe("pnpm"); - expect(preferredRunner("yarn", all)?.id).toBe("yarn"); + const all = [bunx, npx, pnpm, yarn] as const; + expect(preferredRunner("bun", all).id).toBe("bunx"); + expect(preferredRunner("npm", all).id).toBe("npx"); + expect(preferredRunner("pnpm", all).id).toBe("pnpm"); + expect(preferredRunner("yarn", all).id).toBe("yarn"); }); test("falls back to first available when the preferred pm runner is missing", () => { - expect(preferredRunner("bun", [npx, pnpm])?.id).toBe("npx"); - expect(preferredRunner("yarn", [pnpm])?.id).toBe("pnpm"); + // Project is bun but only npx is on PATH → fall back to npx (first + // available, which RUNNERS orders as bunx > npx > pnpm > yarn). + expect(preferredRunner("bun", [npx, pnpm] as const).id).toBe("npx"); + // Project is yarn but only pnpm is on PATH → fall back to pnpm. + expect(preferredRunner("yarn", [pnpm] as const).id).toBe("pnpm"); }); test("returns first available when no package manager is given", () => { - expect(preferredRunner(undefined, [npx, pnpm, yarn])?.id).toBe("npx"); - expect(preferredRunner(undefined, [yarn])?.id).toBe("yarn"); + expect(preferredRunner(undefined, [npx, pnpm, yarn] as const).id).toBe("npx"); + expect(preferredRunner(undefined, [yarn] as const).id).toBe("yarn"); }); test("preserves KNOWN_RUNNERS preference order in fallback", () => { - expect(preferredRunner(undefined, KNOWN_RUNNERS)?.id).toBe("bunx"); + // Even with all four available, no pm hint → bunx wins (it's first in KNOWN_RUNNERS). + // KNOWN_RUNNERS isn't typed as NonEmptyArray, so we narrow via isNonEmpty. + if (!isNonEmpty(KNOWN_RUNNERS)) throw new Error("unreachable"); + expect(preferredRunner(undefined, KNOWN_RUNNERS).id).toBe("bunx"); }); }); @@ -186,6 +190,8 @@ describe("detectAvailableRunners", () => { }); test("preserves KNOWN_RUNNERS order in the output", () => { + // Even though we list yarn first in the set, the result should still + // be in KNOWN_RUNNERS preference order (bunx > npx > pnpm > yarn). mockWhich(new Set(["yarn", "bunx", "npx"])); mockSpawnSync(0); const result = detectAvailableRunners(); @@ -217,10 +223,13 @@ describe("detectAvailableRunners", () => { test("integrates cleanly with preferredRunner + runnerCommand", () => { mockWhich(new Set(["npx", "pnpm"])); const available = detectAvailableRunners(); + if (!isNonEmpty(available)) throw new Error("expected at least one runner"); + // After the isNonEmpty narrowing, preferredRunner returns Runner (no + // undefined) and `runner` doesn't need a cast. const runner = preferredRunner("pnpm", available); - expect(runner?.id).toBe("pnpm"); + expect(runner.id).toBe("pnpm"); - const command = runnerCommand(runner as Runner, ["prettier", "--write", "src/x.ts"]); + const command = runnerCommand(runner, ["prettier", "--write", "src/x.ts"]); expect(command).toEqual(["pnpm", "dlx", "prettier", "--write", "src/x.ts"]); }); }); diff --git a/packages/cli-core/src/lib/runners.ts b/packages/cli-core/src/lib/runners.ts index 5cc5fe69..09c2db80 100644 --- a/packages/cli-core/src/lib/runners.ts +++ b/packages/cli-core/src/lib/runners.ts @@ -15,6 +15,7 @@ */ import type { ProjectContext } from "../commands/init/frameworks/types.js"; +import type { NonEmptyArray } from "./helpers/arrays.js"; /** * One way to invoke an npm-published binary without installing it globally. @@ -106,13 +107,14 @@ export function runnerForPackageManager( * bunx). Otherwise falls back to the first available runner in {@link KNOWN_RUNNERS} * order. * - * Returns `undefined` only if `available` is empty. + * Requires a {@link NonEmptyArray} so the return type can be `Runner` (never + * undefined). Callers prove non-emptiness via `isNonEmpty()` from + * `lib/helpers/arrays.ts`. */ export function preferredRunner( packageManager: ProjectContext["packageManager"] | undefined, - available: readonly Runner[], -): Runner | undefined { - if (available.length === 0) return undefined; + available: NonEmptyArray, +): Runner { if (packageManager) { const preferredId = PM_TO_RUNNER[packageManager]; const match = available.find((r) => r.id === preferredId);