From 0e1e33eecb83d2a8e417a76890172a395c87351c Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 14:04:28 +1100 Subject: [PATCH 1/3] fix(toolchain): report types/lint/knip unavailable when dependencies are not installed (#100) On a checkout with no installed dependencies, `npx tsc`, `npx eslint` and `npx knip` either fetched an unrelated package or ran without the project's compiler, config plugins and packages; the masked output then scored as a real result. - toolchain: probeDependencies(dir) reads the nearest package.json and asks whether any declared dependency resolves via node_modules in that directory or an ancestor (workspace hoisting and pnpm per-package links both count; Yarn PnP counts as installed). Cached per directory against an install fingerprint so a later install is seen. dependencyGap(dir, tool) also requires the invoked tool package to resolve, so npx never fetches it. - types: TS targets whose deps are missing are not run; all missing -> unavailable, some missing -> listed under details.unavailableProjects. - lint: root ESLint and per-project ESLint report unavailable the same way the Dart SDK path does. Zero-config Biome is unchanged. - performance: knip roots without deps are skipped and recorded as details.deadCodeUnavailable; with no knip result the dead-code part is absent from the score (the existing no-knip path) and the dead-code check reports unavailable with the install reason. The testing check is not changed here. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/runners/lint.ts | 49 ++++-- src/runners/performance.ts | 55 +++++-- src/runners/toolchain.test.ts | 272 ++++++++++++++++++++++++++++++++++ src/runners/toolchain.ts | 137 +++++++++++++++++ src/runners/types-check.ts | 36 ++++- 5 files changed, 524 insertions(+), 25 deletions(-) create mode 100644 src/runners/toolchain.test.ts diff --git a/src/runners/lint.ts b/src/runners/lint.ts index ed30fbe..7d44dee 100644 --- a/src/runners/lint.ts +++ b/src/runners/lint.ts @@ -10,7 +10,7 @@ import { hasFileWithExt, isIgnoredPath, normalizeToolPath } from "../fs-utils.js import type { CheckResult, Issue, ProjectContext, StackInfo, WorkspaceInfo } from "../types.js"; import { gradeFromScore } from "../types.js"; import { run } from "./exec.js"; -import { DART_SDK_MISSING_REASON, hasDartSdk, unavailableResult } from "./toolchain.js"; +import { DART_SDK_MISSING_REASON, dependencyGap, hasDartSdk, unavailableResult } from "./toolchain.js"; export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo): CheckResult { const start = Date.now(); @@ -20,20 +20,28 @@ export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo if (stack.linter === "none" && projectRuns.length > 0) { const projects: Array> = []; let linted = 0; - let dartUnavailable = 0; + const unavailableReasons: string[] = []; for (const project of projectRuns) { const projectCwd = project.path === "." ? cwd : join(cwd, project.path); - // A Dart project we cannot analyze contributes no issues — which would - // otherwise read as "this package lints clean" (#92). Record it as - // unavailable and keep it out of the count of projects we really linted. - if (project.stack.linter === "dart_analyze" && !hasDartSdk(cwd)) { - dartUnavailable++; + // A project we cannot lint contributes no issues — which would otherwise + // read as "this package lints clean". Record it as unavailable and keep + // it out of the count of projects we really linted: a Dart project + // without the SDK (#92), or an ESLint project whose config's plugins are + // not installed (#100). + const unavailableReason = + project.stack.linter === "dart_analyze" && !hasDartSdk(cwd) + ? DART_SDK_MISSING_REASON + : project.stack.linter === "eslint" + ? eslintDependencyGap(projectCwd) + : null; + if (unavailableReason) { + unavailableReasons.push(unavailableReason); projects.push({ id: project.id, path: project.path, linter: project.stack.linter, unavailable: true, - reason: DART_SDK_MISSING_REASON, + reason: unavailableReason, }); continue; } @@ -48,8 +56,14 @@ export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo }); } // Nothing was actually linted — do not report a score for it. - if (linted === 0 && dartUnavailable > 0) { - return unavailableResult("lint", DART_SDK_MISSING_REASON, { linter: "dart_analyze", projects }, start); + if (linted === 0 && unavailableReasons.length > 0) { + const linters = [...new Set(projectRuns.map((p) => p.stack.linter))]; + return unavailableResult( + "lint", + unavailableReasons[0]!, + { linter: linters.length === 1 ? linters[0] : "project-scoped", projects }, + start, + ); } const { score, errors, warnings } = scoreLint(issues); return { @@ -86,6 +100,13 @@ export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo for (let i = 0; i < parseInt(warnings, 10); i++) issues.push({ severity: "warning", message: "lint warning" }); } } else if (stack.linter === "eslint") { + // ESLint runs the project's own config, and its plugins and parsers come + // from the project's dependencies. Uninstalled, the config fails to load, + // the masked crash prints nothing, and nothing parses as zero issues (#100). + const depsReason = eslintDependencyGap(cwd); + if (depsReason) { + return unavailableResult("lint", depsReason, { linter: "eslint", lintTarget }, start); + } const { stdout } = run(`npx eslint ${lintTarget} --format json 2>/dev/null || true`, cwd); issues.push(...parseEslintJson(stdout, { repoCwd: cwd, toolCwd: cwd })); } else if (stack.linter === "dart_analyze") { @@ -255,6 +276,14 @@ function dartAnalyzeIssues(cwd: string, workspace?: WorkspaceInfo): Issue[] { return issues; } +/** Why ESLint cannot run against the project at `dir`, or null when it can: + * the project's dependencies (its config's plugins) and ESLint itself must + * resolve from there, or `npx eslint` fetches a bare ESLint that cannot load + * the config. */ +function eslintDependencyGap(dir: string): string | null { + return dependencyGap(dir, "eslint"); +} + function lintableProjects(workspace?: WorkspaceInfo): ProjectContext[] { return (workspace?.projects ?? []).filter((project) => project.path !== "." && project.stack.linter !== "none"); } diff --git a/src/runners/performance.ts b/src/runners/performance.ts index c760fb7..32416a2 100644 --- a/src/runners/performance.ts +++ b/src/runners/performance.ts @@ -15,6 +15,7 @@ import { getProductionFiles, isIgnoredPath, normalizeToolPath, readDeps } from " import type { CheckResult, Issue, WorkspaceInfo } from "../types.js"; import { gradeFromScore } from "../types.js"; import { run } from "./exec.js"; +import { dependenciesMissingReason, probeDependencies, unavailableResult } from "./toolchain.js"; // Packages known to be heavy (bundled KB, approx) const HEAVY_DEPS: Record = { @@ -165,7 +166,7 @@ export function runPerformance(cwd: string, workspace?: WorkspaceInfo, inventory let deadExports = 0; let unusedFiles = 0; let unusedDeps = 0; - const knipResult = tryKnip(cwd, workspace); + const { findings: knipResult, unavailable: knipUnavailable } = tryKnip(cwd, workspace); if (knipResult) { deadExports = knipResult.exports; unusedFiles = knipResult.files; @@ -286,6 +287,11 @@ export function runPerformance(cwd: string, workspace?: WorkspaceInfo, inventory }, } : {}), + // Knip roots that were never run because their dependencies are not + // installed (#100). With no knip result at all, the dead-code part is + // simply absent from this score — the same as when knip produces no + // parseable report — and the dead-code check reports `unavailable`. + ...(knipUnavailable.length > 0 ? { deadCodeUnavailable: knipUnavailable } : {}), ...(bundleSizeKB > 0 ? { bundleSizeKB } : {}), }, issues, @@ -354,6 +360,16 @@ export function deadCodeCheckFromPerformance(performance: CheckResult): CheckRes const total = unusedFiles + unusedDeps + deadExports; const issues = performance.issues.filter((i) => ["dead-files", "dead-exports", "unused-deps"].includes(i.rule ?? "")); + const knipUnavailable = details.deadCodeUnavailable as Array<{ path: string; reason: string }> | undefined; + if (details.deadCodeTool !== "knip" && knipUnavailable && knipUnavailable.length > 0) { + return unavailableResult( + "dead-code", + knipUnavailable[0]!.reason, + { synthetic: true, sourceCheck: "performance", deadCodeTool: "knip", roots: knipUnavailable }, + Date.now(), + ); + } + if (details.deadCodeTool !== "knip") { return { name: "dead-code", @@ -476,16 +492,28 @@ export function knipRoots(cwd: string, workspace?: WorkspaceInfo): { dir: string return [{ dir: cwd, rel: "", configured: false }]; } -function tryKnip( - cwd: string, - workspace?: WorkspaceInfo, -): (KnipFindings & { files: number; exports: number; deps: number; configured: boolean; excluded: number }) | null { +interface KnipOutcome { + findings: (KnipFindings & { files: number; exports: number; deps: number; configured: boolean; excluded: number }) | null; + /** Roots knip was not run in because their dependencies are not installed. */ + unavailable: Array<{ path: string; reason: string }>; +} + +function tryKnip(cwd: string, workspace?: WorkspaceInfo): KnipOutcome { const roots = knipRoots(cwd, workspace); const merged: KnipFindings = { unusedFiles: [], unusedExports: [], unusedTypes: [], unusedDeps: [] }; + const unavailable: KnipOutcome["unavailable"] = []; let any = false; let configured = true; let excluded = 0; for (const root of roots) { + // Knip resolves the project's imports and enables plugins from its + // installed packages; without them its "unused" lists describe the + // missing install, not the code (#100). Do not run it at all. + const deps = probeDependencies(root.dir); + if (!deps.installed) { + unavailable.push({ path: root.rel || ".", reason: dependenciesMissingReason(deps.packageManager) }); + continue; + } const { stdout } = run("npx knip --reporter json 2>/dev/null || true", root.dir, 60_000); const parsed = parseKnipJson(stdout); if (!parsed) continue; @@ -502,14 +530,17 @@ function tryKnip( } } } - if (!any) return null; + if (!any) return { findings: null, unavailable }; return { - ...merged, - configured, - files: merged.unusedFiles.length, - exports: merged.unusedExports.length + merged.unusedTypes.length, - deps: merged.unusedDeps.length, - excluded, + findings: { + ...merged, + configured, + files: merged.unusedFiles.length, + exports: merged.unusedExports.length + merged.unusedTypes.length, + deps: merged.unusedDeps.length, + excluded, + }, + unavailable, }; } diff --git a/src/runners/toolchain.test.ts b/src/runners/toolchain.test.ts new file mode 100644 index 0000000..2d631b0 --- /dev/null +++ b/src/runners/toolchain.test.ts @@ -0,0 +1,272 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { detectStack, detectWorkspace } from "../detect.js"; + +/** + * Issue #100: `types`, `lint` (project ESLint config) and knip all delegate to + * tools resolved from the project's own dependencies. On a checkout nobody has + * installed they must report `unavailable` — and must not shell out to `npx` at + * all — rather than score whatever a half-blind or wrong-package run printed. + * + * The exec layer is replaced with a recorder that answers with canned tool + * output, so these assert on exactly which commands were attempted, with no + * network and no real tools. + */ +const runCalls: Array<{ cmd: string; cwd: string }> = []; + +vi.mock("./exec.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + run: vi.fn((cmd: string, cwd: string) => { + runCalls.push({ cmd, cwd }); + if (cmd.startsWith("npx tsc")) { + return { stdout: "src/index.ts(1,7): error TS2322: Type 'string' is not assignable to type 'number'.\n", ok: true }; + } + if (cmd.startsWith("npx eslint")) { + const filePath = join(cwd, "src/index.ts"); + return { + stdout: JSON.stringify([{ filePath, messages: [{ severity: 2, message: "no-unused-vars", line: 1, ruleId: "no-unused-vars" }] }]), + ok: true, + }; + } + if (cmd.startsWith("npx knip")) { + return { stdout: JSON.stringify({ issues: [{ file: "src/dead.ts", files: [{ name: "src/dead.ts" }] }] }), ok: true }; + } + if (cmd.startsWith("npx @biomejs/biome")) { + return { + stdout: JSON.stringify({ + diagnostics: [ + { severity: "warning", description: "use const", category: "lint/style/useConst", location: { path: "src/index.js" } }, + ], + }), + ok: true, + }; + } + return { stdout: "", ok: false }; + }), + }; +}); + +const { runTypeCheck } = await import("./types-check.js"); +const { runLint } = await import("./lint.js"); +const { runPerformance, deadCodeCheckFromPerformance } = await import("./performance.js"); +const { probeDependencies, resetToolchainProbes } = await import("./toolchain.js"); + +const dirs: string[] = []; + +function project(files: Record): string { + const dir = mkdtempSync(join(tmpdir(), "vcqa-deps-")); + dirs.push(dir); + for (const [name, content] of Object.entries(files)) { + const full = join(dir, name); + mkdirSync(dirname(full), { recursive: true }); + writeFileSync(full, content); + } + return dir; +} + +/** Stand-in for ` install`: one resolvable package per name. */ +function install(dir: string, names: string[]): void { + for (const name of names) { + mkdirSync(join(dir, "node_modules", name), { recursive: true }); + writeFileSync(join(dir, "node_modules", name, "package.json"), JSON.stringify({ name, version: "1.0.0" })); + } +} + +/** A TypeScript + ESLint project, as checked out: no node_modules. */ +function tsEslintFixture(): string { + return project({ + "package.json": JSON.stringify({ + name: "ts-eslint-fixture", + main: "src/index.ts", + devDependencies: { typescript: "^5", eslint: "^9", "typescript-eslint": "^8" }, + }), + "pnpm-lock.yaml": "lockfileVersion: '9.0'\n", + "tsconfig.json": JSON.stringify({ compilerOptions: { strict: true, noEmit: true }, include: ["src"] }), + "eslint.config.js": "import tseslint from 'typescript-eslint';\nexport default tseslint.config(...tseslint.configs.recommended);\n", + "src/index.ts": "const x: number = 'nope';\nexport const y = x;\n", + }); +} + +const npxCalls = () => runCalls.filter((c) => c.cmd.startsWith("npx ")); + +beforeEach(() => { + runCalls.length = 0; + resetToolchainProbes(); +}); + +afterEach(() => { + for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); + resetToolchainProbes(); +}); + +describe("probeDependencies", () => { + it("reports a declared-but-uninstalled project and infers the package manager from the lockfile", () => { + const dir = tsEslintFixture(); + expect(probeDependencies(dir)).toMatchObject({ installed: false, packageManager: "pnpm", declared: 3 }); + }); + + it("counts a manifest that declares nothing as installed", () => { + const dir = project({ "package.json": "{}" }); + expect(probeDependencies(dir).installed).toBe(true); + }); + + it("resolves dependencies hoisted to a workspace root, like Node does", () => { + const dir = project({ + "package.json": JSON.stringify({ private: true, workspaces: ["packages/*"] }), + "yarn.lock": "", + "packages/web/package.json": JSON.stringify({ name: "web", dependencies: { react: "^19" } }), + }); + expect(probeDependencies(join(dir, "packages/web"))).toMatchObject({ installed: false, packageManager: "yarn" }); + install(dir, ["react"]); + // Installing changes the fingerprint the cache is keyed on — no reset needed. + expect(probeDependencies(join(dir, "packages/web")).installed).toBe(true); + }); +}); + +describe("TypeScript + ESLint fixture without node_modules (#100)", () => { + const reason = "dependencies not installed — run `pnpm install`"; + + it("reports types unavailable and never invokes npx tsc", () => { + const dir = tsEslintFixture(); + const result = runTypeCheck(dir, false, detectWorkspace(dir)); + expect(result.details).toMatchObject({ skipped: true, unavailable: true, reason }); + expect(result.issues).toEqual([]); + expect(npxCalls()).toEqual([]); + }); + + it("reports lint unavailable and never invokes npx eslint", () => { + const dir = tsEslintFixture(); + const workspace = detectWorkspace(dir); + const stack = detectStack(dir, workspace); + expect(stack.linter).toBe("eslint"); + const result = runLint(dir, stack, workspace); + expect(result.details).toMatchObject({ skipped: true, unavailable: true, reason, linter: "eslint" }); + expect(npxCalls()).toEqual([]); + }); + + it("reports the knip part unavailable, never invokes npx knip, and still scores the rest of performance", () => { + const dir = tsEslintFixture(); + const perf = runPerformance(dir, detectWorkspace(dir)); + const details = perf.details as Record; + expect(details.deadCodeTool).toBeUndefined(); + expect(details.deadCodeUnavailable).toEqual([{ path: ".", reason }]); + expect(perf.score).toBe(100); + expect(details.unavailable).toBeUndefined(); + + const deadCode = deadCodeCheckFromPerformance(perf); + expect(deadCode.name).toBe("dead-code"); + expect(deadCode.details).toMatchObject({ skipped: true, unavailable: true, reason, synthetic: true }); + expect(npxCalls()).toEqual([]); + }); +}); + +describe("the same fixture with dependencies installed (#100)", () => { + it("scores types, lint and knip as before", () => { + const dir = tsEslintFixture(); + install(dir, ["typescript", "eslint", "typescript-eslint"]); + const workspace = detectWorkspace(dir); + + const types = runTypeCheck(dir, false, workspace); + expect(types.details.unavailable).toBeUndefined(); + expect(types.score).toBe(95); + expect(types.issues).toEqual([expect.objectContaining({ rule: "TS2322", file: "src/index.ts" })]); + + const lint = runLint(dir, detectStack(dir, workspace), workspace); + expect(lint.details.unavailable).toBeUndefined(); + expect(lint.details).toMatchObject({ linter: "eslint", errors: 1 }); + + const perf = runPerformance(dir, workspace); + expect(perf.details).toMatchObject({ deadCodeTool: "knip", unusedFiles: 1 }); + expect((perf.details as Record).deadCodeUnavailable).toBeUndefined(); + expect(deadCodeCheckFromPerformance(perf).details.unavailable).toBeUndefined(); + + expect(npxCalls().map((c) => c.cmd.split(" ").slice(0, 2).join(" "))).toEqual(["npx tsc", "npx eslint", "npx knip"]); + }); +}); + +describe("zero-config Biome (#100 leaves it unchanged)", () => { + it("still scores a project with no linter config and no node_modules", () => { + const dir = project({ + "package.json": JSON.stringify({ name: "plain", dependencies: { lodash: "^4" } }), + "src/index.js": "var x = 1;\nexport { x };\n", + }); + const workspace = detectWorkspace(dir); + const stack = detectStack(dir, workspace); + expect(stack.linter).toBe("none"); + expect(probeDependencies(dir).installed).toBe(false); + + const result = runLint(dir, stack, workspace); + expect(result.details).toMatchObject({ linter: "biome", zeroConfig: true, warnings: 1 }); + expect(result.details.unavailable).toBeUndefined(); + expect(npxCalls().map((c) => c.cmd)).toEqual([expect.stringMatching(/^npx @biomejs\/biome lint /)]); + }); +}); + +describe("monorepos (#100) — probed per project directory", () => { + function monorepo(pkgDeps: Record): string { + const pkg = (name: string) => JSON.stringify({ name, devDependencies: pkgDeps }); + return project({ + "package.json": JSON.stringify({ private: true }), + "pnpm-workspace.yaml": "packages:\n - packages/*\n", + "pnpm-lock.yaml": "", + "packages/a/package.json": pkg("a"), + "packages/a/tsconfig.json": JSON.stringify({ include: ["src"] }), + "packages/a/eslint.config.js": "export default [];\n", + "packages/a/src/index.ts": "export const a = 1;\n", + "packages/b/package.json": pkg("b"), + "packages/b/tsconfig.json": JSON.stringify({ include: ["src"] }), + "packages/b/eslint.config.js": "export default [];\n", + "packages/b/src/index.ts": "export const b = 1;\n", + }); + } + + it("type-checks and lints the installed package and marks the other unavailable", () => { + // ESLint is configured per package (config files) and not declared, so the + // root has no linter and lint runs project by project. + const dir = monorepo({ typescript: "^5" }); + // pnpm links each package's own dependencies into its own node_modules. + install(join(dir, "packages/a"), ["typescript", "eslint"]); + const workspace = detectWorkspace(dir); + const stack = detectStack(dir, workspace); + expect(stack.linter).toBe("none"); + + const types = runTypeCheck(dir, false, workspace); + expect(types.details.unavailable).toBeUndefined(); + expect(types.details).toMatchObject({ + projects: [expect.objectContaining({ path: "packages/a" })], + unavailableProjects: [expect.objectContaining({ path: "packages/b", reason: "dependencies not installed — run `pnpm install`" })], + }); + + const lint = runLint(dir, stack, workspace); + expect(lint.details.unavailable).toBeUndefined(); + expect(lint.details).toMatchObject({ + linter: "project-scoped", + projects: expect.arrayContaining([ + expect.objectContaining({ path: "packages/a", linter: "eslint", issues: 1 }), + expect.objectContaining({ path: "packages/b", unavailable: true }), + ]), + }); + + const attempted = npxCalls().map((c) => c.cwd); + expect(attempted).toEqual([join(dir, "packages/a"), join(dir, "packages/a")]); + }); + + it("does not let root ESLint fetch a bare ESLint when only the packages declare it", () => { + // Packages declare eslint, so the root stack detects eslint and lints from + // the root — where, uninstalled, `npx eslint` would download ESLint and + // crash on the packages' configs, scoring zero issues. + const dir = monorepo({ typescript: "^5", eslint: "^9" }); + const workspace = detectWorkspace(dir); + const stack = detectStack(dir, workspace); + expect(stack.linter).toBe("eslint"); + + const lint = runLint(dir, stack, workspace); + expect(lint.details).toMatchObject({ unavailable: true, linter: "eslint" }); + expect(String(lint.details.reason)).toMatch(/^dependencies not installed — run `pnpm install`/); + expect(npxCalls()).toEqual([]); + }); +}); diff --git a/src/runners/toolchain.ts b/src/runners/toolchain.ts index 8ecdab9..16152af 100644 --- a/src/runners/toolchain.ts +++ b/src/runners/toolchain.ts @@ -10,6 +10,8 @@ * Unmasking the real exit codes is the general fix and belongs to #26. Until * then a runner that is about to delegate to the Dart SDK asks here first, and * returns an *unavailable* result rather than a score if the SDK is absent. + * Runners that delegate to a tool resolved from the project's own dependencies + * (tsc, a project ESLint config, knip) ask `probeDependencies` the same way (#100). * * `unavailable` is deliberately not `not-applicable`: the stack was detected, * the tool was not. Both are excluded from the composite score (#52 — a missing @@ -17,6 +19,8 @@ * by installing something. */ +import { existsSync, readFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; import type { CheckResult } from "../types.js"; import { run } from "./exec.js"; @@ -40,9 +44,142 @@ export function hasDartSdk(cwd: string): boolean { return available; } +// ── Project dependencies (#100) ── +// +// `npx tsc`, `npx eslint` and `npx knip` all run *against the project's own +// dependencies*: its compiler and `@types`, its ESLint config's plugins, the +// packages knip's plugins key off. On a checkout nobody has installed, npx +// either fetches some unrelated package or the tool runs half-blind — and the +// output, masked by `|| true`, scores as if the check had happened. + +/** What a dependency probe found for one project directory. */ +export interface DependencyProbe { + /** True when the project's declared dependencies resolve from this directory. */ + installed: boolean; + /** The package manager inferred from the nearest lockfile (npm when none). */ + packageManager: "npm" | "pnpm" | "yarn" | "bun"; + /** The package.json the probe read, or null when there is none. */ + manifest: string | null; + /** How many dependencies the manifest declares (dependencies + devDependencies). */ + declared: number; + /** True when a Yarn Plug'n'Play map is on the path (no node_modules to inspect). */ + pnp: boolean; +} + +const depProbes = new Map(); + +/** Every ancestor of `dir` (inclusive), nearest first. */ +function ancestors(dir: string): string[] { + const out: string[] = []; + let current = resolve(dir); + for (;;) { + out.push(current); + const parent = dirname(current); + if (parent === current) return out; + current = parent; + } +} + +/** Which ancestors hold an install (node_modules or a Yarn PnP map). This is + * what an `install` changes, so it keys the cache: a long-lived process that + * sees the user install re-probes instead of answering from a stale result. */ +function installFingerprint(dirs: string[]): string { + return dirs + .map( + (d) => + (existsSync(join(d, "node_modules")) ? "n" : "") + (existsSync(join(d, ".pnp.cjs")) || existsSync(join(d, ".pnp.js")) ? "p" : ""), + ) + .join("|"); +} + +function inferPackageManager(dirs: string[]): DependencyProbe["packageManager"] { + // Same precedence as detect.ts, nearest directory first. + for (const d of dirs) { + if (existsSync(join(d, "pnpm-lock.yaml"))) return "pnpm"; + if (existsSync(join(d, "bun.lockb")) || existsSync(join(d, "bun.lock"))) return "bun"; + if (existsSync(join(d, "yarn.lock"))) return "yarn"; + if (existsSync(join(d, "package-lock.json"))) return "npm"; + } + return "npm"; +} + +/** Probe whether the dependencies of the project at `dir` are installed. + * + * Reads the nearest package.json at or above `dir` — the package that declares + * the dependencies — and asks whether any of its `dependencies` / + * `devDependencies` resolves the way Node would resolve it: a + * `node_modules/` in that directory or any ancestor, so workspace-root + * hoisting (npm, yarn, bun) and pnpm's per-package symlinks both count. A Yarn + * Plug'n'Play map (`.pnp.cjs`) on the path counts as installed too. + * + * "Any", not "all": optional and platform-specific packages are legitimately + * absent from a good install, while a checkout nobody installed resolves none. + * A manifest that declares nothing has nothing to install, so it passes. + * + * Cached per directory against the install fingerprint of its ancestors. */ +export function probeDependencies(dir: string): DependencyProbe { + const dirs = ancestors(dir); + const key = dirs[0]!; + const fingerprint = installFingerprint(dirs); + const cached = depProbes.get(key); + if (cached && cached.fingerprint === fingerprint) return cached.probe; + + const packageManager = inferPackageManager(dirs); + const manifestDir = dirs.find((d) => existsSync(join(d, "package.json"))); + let declaredNames: string[] = []; + if (manifestDir) { + try { + const pkg = JSON.parse(readFileSync(join(manifestDir, "package.json"), "utf-8")); + declaredNames = [...Object.keys(pkg?.dependencies ?? {}), ...Object.keys(pkg?.devDependencies ?? {})]; + } catch { + /* unreadable manifest — treat as declaring nothing */ + } + } + const pnp = fingerprint.includes("p"); + const installed = declaredNames.length === 0 || pnp || declaredNames.some((name) => resolvesFrom(dirs, name)); + + const probe: DependencyProbe = { + installed, + packageManager, + manifest: manifestDir ? join(manifestDir, "package.json") : null, + declared: declaredNames.length, + pnp, + }; + depProbes.set(key, { fingerprint, probe }); + return probe; +} + +/** Does `node_modules/` exist in any of `dirs` — Node's lookup path? */ +function resolvesFrom(dirs: string[], name: string): boolean { + return dirs.some((d) => existsSync(join(d, "node_modules", name))); +} + +/** What the user is told when a project's dependencies are not installed. */ +export function dependenciesMissingReason(packageManager: DependencyProbe["packageManager"]): string { + return `dependencies not installed — run \`${packageManager} install\``; +} + +/** Why a tool cannot run against the project at `dir`, or null when it can. + * + * Fails when the project's dependencies are not installed at all, and — when + * `toolPackage` is given — when that package does not resolve from `dir`. The + * second case matters for `npx `: with nothing local to resolve, npx + * fetches a package from the registry instead (for `tsc`, an unrelated package + * of that name), so the run measures nothing about the project. That happens + * at a monorepo root whose packages, not the root, declare the tool. */ +export function dependencyGap(dir: string, toolPackage?: string): string | null { + const deps = probeDependencies(dir); + if (!deps.installed) return dependenciesMissingReason(deps.packageManager); + if (toolPackage && !deps.pnp && !resolvesFrom(ancestors(dir), toolPackage)) { + return `${dependenciesMissingReason(deps.packageManager)} (${toolPackage} does not resolve from this project)`; + } + return null; +} + /** Forget every probed result. Tests use this; nothing in a scan should need it. */ export function resetToolchainProbes(): void { dartProbe = null; + depProbes.clear(); } /** A check result meaning "this check applies here, but its tool is missing". diff --git a/src/runners/types-check.ts b/src/runners/types-check.ts index d83f8c3..6661264 100644 --- a/src/runners/types-check.ts +++ b/src/runners/types-check.ts @@ -10,7 +10,7 @@ import { isIgnoredPath, normalizeToolPath } from "../fs-utils.js"; import type { CheckResult, Issue, ProjectContext, WorkspaceInfo } from "../types.js"; import { gradeFromScore } from "../types.js"; import { run } from "./exec.js"; -import { DART_SDK_MISSING_REASON, hasDartSdk, unavailableResult } from "./toolchain.js"; +import { DART_SDK_MISSING_REASON, dependencyGap, hasDartSdk, unavailableResult } from "./toolchain.js"; interface TypeCheckTarget { cwd: string; @@ -77,6 +77,35 @@ export function runTypeCheck(cwd: string, isDart = false, workspace?: WorkspaceI ); } + // A TypeScript target type-checks against the project's own compiler and + // `@types`. Without its dependencies, `npx tsc` either resolves some other + // package or reports a wall of module-not-found errors — neither is a + // measurement of the code (#100). Such targets are not run at all. + const unavailableTargets: Array<{ id: string; path: string; command: string; reason: string }> = []; + const runnableTargets = isDart + ? targets + : targets.filter((target) => { + // `npx tsc` must find the project's own TypeScript; a package script + // (`pnpm typecheck`) needs the install but resolves its own binaries. + const gap = dependencyGap(target.cwd, target.mode === "script" ? undefined : "typescript"); + if (!gap) return true; + unavailableTargets.push({ + id: target.projectId ?? "root", + path: target.projectPath ?? ".", + command: target.command, + reason: gap, + }); + return false; + }); + if (runnableTargets.length === 0) { + return unavailableResult( + "types", + unavailableTargets[0]!.reason, + { tool: "tsc", projects: unavailableTargets, excluded: plan.excluded, strategy: plan.strategy }, + start, + ); + } + if (isDart) { for (const root of targets) { parseDartAnalyzeErrors( @@ -90,7 +119,7 @@ export function runTypeCheck(cwd: string, isDart = false, workspace?: WorkspaceI ); } } else { - for (const target of targets) { + for (const target of runnableTargets) { parseTscOutput( run(`${target.command} 2>&1 || true`, target.cwd, target.mode === "build" || target.mode === "script" ? 60_000 : 30_000, { projectId: target.projectId, @@ -113,7 +142,7 @@ export function runTypeCheck(cwd: string, isDart = false, workspace?: WorkspaceI details: { errors: errorCount, ok: errorCount === 0, - projects: targets.map((t) => ({ + projects: runnableTargets.map((t) => ({ id: t.projectId ?? "root", path: t.projectPath ?? ".", tool: t.tool, @@ -122,6 +151,7 @@ export function runTypeCheck(cwd: string, isDart = false, workspace?: WorkspaceI authoritative: t.authoritative, reason: t.reason, })), + ...(unavailableTargets.length > 0 ? { unavailableProjects: unavailableTargets } : {}), excluded: plan.excluded, excludedRootFallback: plan.excluded.some((target) => target.projectPath === "."), strategy: plan.strategy, From fe362868262d1dc39d2e9f1e7ec082af9d868414 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 19:32:22 +1100 Subject: [PATCH 2/3] fix(toolchain): bound the dependency probe to the project being scanned (#100) The probe walked node_modules lookups up to the filesystem root, so a nested project with no install of its own passed on whatever an ancestor directory had installed. `npx tsc` then also resolved the ancestor's compiler and ran half-blind: a nested project under an installed parent scored types 95 with "Cannot find module 'node:fs'" errors. probeDependencies / dependencyGap now take the scan root and only search directories inside its install boundary. That boundary is the scan root itself, widened to an enclosing workspace root only when the scan root matches one of that workspace's package globs (pnpm-workspace.yaml or package.json `workspaces`, negations honoured), since npm/yarn/bun hoist a member's dependencies there. The lockfile used for the install hint is bounded the same way. types, lint and knip all pass the scan root. Three existing fixtures live inside the cli checkout and passed only by borrowing its node_modules from above. They now get their own install: the types-check fixtures link the repo's TypeScript into the fixture (they run the real tsc), and the core monorepo fixture gets a hoisted typescript stub (it runs a fake npx). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/core.test.ts | 4 ++ src/runners/lint.ts | 8 +-- src/runners/performance.ts | 2 +- src/runners/toolchain.test.ts | 94 ++++++++++++++++++++++++++-- src/runners/toolchain.ts | 107 ++++++++++++++++++++++++++++---- src/runners/types-check.test.ts | 19 +++++- src/runners/types-check.ts | 2 +- 7 files changed, 210 insertions(+), 26 deletions(-) diff --git a/src/core.test.ts b/src/core.test.ts index 80bb7ee..bc1ff12 100644 --- a/src/core.test.ts +++ b/src/core.test.ts @@ -307,6 +307,10 @@ describe("auditability", () => { writeFileSync(join(TMP, "packages/web/package.json"), JSON.stringify({ name: "web", devDependencies: { typescript: "^5" } })); writeFileSync(join(TMP, "packages/web/tsconfig.json"), JSON.stringify({ include: ["src/**/*.ts"] })); writeFileSync(join(TMP, "packages/web/src/index.ts"), "export const web = 1;\n"); + // The packages' TypeScript, hoisted to the workspace root. Without it the + // targets are unavailable (#100) — the cli checkout's own install, above + // this fixture, does not count for it. + mkdirSync(join(TMP, "node_modules/typescript"), { recursive: true }); const fakeNpx = join(TMP, "bin", "npx"); writeFileSync(fakeNpx, '#!/bin/sh\necho "fake-npx:$PWD:$*"\nexit 0\n'); chmodSync(fakeNpx, 0o755); diff --git a/src/runners/lint.ts b/src/runners/lint.ts index 7d44dee..5c1e50d 100644 --- a/src/runners/lint.ts +++ b/src/runners/lint.ts @@ -32,7 +32,7 @@ export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo project.stack.linter === "dart_analyze" && !hasDartSdk(cwd) ? DART_SDK_MISSING_REASON : project.stack.linter === "eslint" - ? eslintDependencyGap(projectCwd) + ? eslintDependencyGap(projectCwd, cwd) : null; if (unavailableReason) { unavailableReasons.push(unavailableReason); @@ -103,7 +103,7 @@ export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo // ESLint runs the project's own config, and its plugins and parsers come // from the project's dependencies. Uninstalled, the config fails to load, // the masked crash prints nothing, and nothing parses as zero issues (#100). - const depsReason = eslintDependencyGap(cwd); + const depsReason = eslintDependencyGap(cwd, cwd); if (depsReason) { return unavailableResult("lint", depsReason, { linter: "eslint", lintTarget }, start); } @@ -280,8 +280,8 @@ function dartAnalyzeIssues(cwd: string, workspace?: WorkspaceInfo): Issue[] { * the project's dependencies (its config's plugins) and ESLint itself must * resolve from there, or `npx eslint` fetches a bare ESLint that cannot load * the config. */ -function eslintDependencyGap(dir: string): string | null { - return dependencyGap(dir, "eslint"); +function eslintDependencyGap(dir: string, scanRoot: string): string | null { + return dependencyGap(dir, scanRoot, "eslint"); } function lintableProjects(workspace?: WorkspaceInfo): ProjectContext[] { diff --git a/src/runners/performance.ts b/src/runners/performance.ts index 32416a2..c31496f 100644 --- a/src/runners/performance.ts +++ b/src/runners/performance.ts @@ -509,7 +509,7 @@ function tryKnip(cwd: string, workspace?: WorkspaceInfo): KnipOutcome { // Knip resolves the project's imports and enables plugins from its // installed packages; without them its "unused" lists describe the // missing install, not the code (#100). Do not run it at all. - const deps = probeDependencies(root.dir); + const deps = probeDependencies(root.dir, cwd); if (!deps.installed) { unavailable.push({ path: root.rel || ".", reason: dependenciesMissingReason(deps.packageManager) }); continue; diff --git a/src/runners/toolchain.test.ts b/src/runners/toolchain.test.ts index 2d631b0..7db960d 100644 --- a/src/runners/toolchain.test.ts +++ b/src/runners/toolchain.test.ts @@ -53,7 +53,7 @@ vi.mock("./exec.js", async (importOriginal) => { const { runTypeCheck } = await import("./types-check.js"); const { runLint } = await import("./lint.js"); const { runPerformance, deadCodeCheckFromPerformance } = await import("./performance.js"); -const { probeDependencies, resetToolchainProbes } = await import("./toolchain.js"); +const { dependencyGap, installBoundary, probeDependencies, resetToolchainProbes } = await import("./toolchain.js"); const dirs: string[] = []; @@ -106,12 +106,12 @@ afterEach(() => { describe("probeDependencies", () => { it("reports a declared-but-uninstalled project and infers the package manager from the lockfile", () => { const dir = tsEslintFixture(); - expect(probeDependencies(dir)).toMatchObject({ installed: false, packageManager: "pnpm", declared: 3 }); + expect(probeDependencies(dir, dir)).toMatchObject({ installed: false, packageManager: "pnpm", declared: 3 }); }); it("counts a manifest that declares nothing as installed", () => { const dir = project({ "package.json": "{}" }); - expect(probeDependencies(dir).installed).toBe(true); + expect(probeDependencies(dir, dir).installed).toBe(true); }); it("resolves dependencies hoisted to a workspace root, like Node does", () => { @@ -120,10 +120,10 @@ describe("probeDependencies", () => { "yarn.lock": "", "packages/web/package.json": JSON.stringify({ name: "web", dependencies: { react: "^19" } }), }); - expect(probeDependencies(join(dir, "packages/web"))).toMatchObject({ installed: false, packageManager: "yarn" }); + expect(probeDependencies(join(dir, "packages/web"), dir)).toMatchObject({ installed: false, packageManager: "yarn" }); install(dir, ["react"]); // Installing changes the fingerprint the cache is keyed on — no reset needed. - expect(probeDependencies(join(dir, "packages/web")).installed).toBe(true); + expect(probeDependencies(join(dir, "packages/web"), dir).installed).toBe(true); }); }); @@ -197,7 +197,7 @@ describe("zero-config Biome (#100 leaves it unchanged)", () => { const workspace = detectWorkspace(dir); const stack = detectStack(dir, workspace); expect(stack.linter).toBe("none"); - expect(probeDependencies(dir).installed).toBe(false); + expect(probeDependencies(dir, dir).installed).toBe(false); const result = runLint(dir, stack, workspace); expect(result.details).toMatchObject({ linter: "biome", zeroConfig: true, warnings: 1 }); @@ -270,3 +270,85 @@ describe("monorepos (#100) — probed per project directory", () => { expect(npxCalls()).toEqual([]); }); }); + +describe("the probe stops at the project being scanned (#100, review round 1)", () => { + /** `parent/` has an install; `parent/child/` is its own project, never installed. */ + function nestedUnderInstalledParent(parentManifest: Record = { name: "parent" }): { parent: string; child: string } { + const parent = project({ + "package.json": JSON.stringify({ ...parentManifest, devDependencies: { typescript: "^5" } }), + "child/package.json": JSON.stringify({ name: "child", devDependencies: { typescript: "^5", "@types/node": "^22" } }), + "child/tsconfig.json": JSON.stringify({ compilerOptions: { strict: true, noEmit: true }, include: ["src"] }), + "child/src/index.ts": "import { readFileSync } from 'node:fs';\nexport const f = readFileSync;\n", + }); + install(parent, ["typescript", "@types/node"]); + return { parent, child: join(parent, "child") }; + } + + it("does not count a parent directory's install for a nested project (reviewer repro)", () => { + const { child } = nestedUnderInstalledParent(); + expect(installBoundary(child)).toBe(child); + expect(probeDependencies(child, child).installed).toBe(false); + expect(dependencyGap(child, child, "typescript")).toBe("dependencies not installed — run `npm install`"); + + const types = runTypeCheck(child, false, detectWorkspace(child)); + expect(types.details).toMatchObject({ skipped: true, unavailable: true, reason: "dependencies not installed — run `npm install`" }); + expect(types.issues).toEqual([]); + expect(npxCalls()).toEqual([]); + }); + + it("still sees the parent's install when the parent is what is being scanned", () => { + const { parent, child } = nestedUnderInstalledParent(); + expect(probeDependencies(child, parent).installed).toBe(true); + expect(dependencyGap(child, parent, "typescript")).toBeNull(); + }); + + it("does not widen to an enclosing workspace the project is not a member of", () => { + const { child } = nestedUnderInstalledParent({ name: "parent", private: true, workspaces: ["packages/*"] }); + expect(installBoundary(child)).toBe(child); + expect(probeDependencies(child, child).installed).toBe(false); + }); + + it("honours a negated workspace glob", () => { + const { child } = nestedUnderInstalledParent({ name: "parent", private: true, workspaces: ["*", "!child"] }); + expect(installBoundary(child)).toBe(child); + }); + + it("widens to the workspace root when the scanned project is a hoisted workspace member", () => { + const root = project({ + "package.json": JSON.stringify({ private: true, workspaces: ["packages/*"] }), + "package-lock.json": "{}", + "packages/web/package.json": JSON.stringify({ name: "web", devDependencies: { typescript: "^5" } }), + "packages/web/tsconfig.json": JSON.stringify({ include: ["src"] }), + "packages/web/src/index.ts": "export const web = 1;\n", + }); + install(root, ["typescript"]); + const web = join(root, "packages/web"); + expect(installBoundary(web)).toBe(root); + expect(probeDependencies(web, web).installed).toBe(true); + + const types = runTypeCheck(web, false, detectWorkspace(web)); + expect(types.details.unavailable).toBeUndefined(); + expect(npxCalls().map((c) => c.cmd.split(" ").slice(0, 2).join(" "))).toEqual(["npx tsc"]); + }); + + it("widens for pnpm-workspace.yaml members too, including ** globs", () => { + const root = project({ + "pnpm-workspace.yaml": "packages:\n - apps/**\n", + "package.json": JSON.stringify({ private: true }), + "apps/site/web/package.json": JSON.stringify({ name: "web", dependencies: { react: "^19" } }), + }); + install(root, ["react"]); + const web = join(root, "apps/site/web"); + expect(installBoundary(web)).toBe(root); + expect(probeDependencies(web, web).installed).toBe(true); + }); + + it("bounds knip the same way", () => { + const { child } = nestedUnderInstalledParent(); + const perf = runPerformance(child, detectWorkspace(child)); + expect((perf.details as Record).deadCodeUnavailable).toEqual([ + { path: ".", reason: "dependencies not installed — run `npm install`" }, + ]); + expect(npxCalls()).toEqual([]); + }); +}); diff --git a/src/runners/toolchain.ts b/src/runners/toolchain.ts index 16152af..b0b33b4 100644 --- a/src/runners/toolchain.ts +++ b/src/runners/toolchain.ts @@ -20,7 +20,8 @@ */ import { existsSync, readFileSync } from "node:fs"; -import { dirname, join, resolve } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { parseYamlList } from "../detect.js"; import type { CheckResult } from "../types.js"; import { run } from "./exec.js"; @@ -68,7 +69,7 @@ export interface DependencyProbe { const depProbes = new Map(); -/** Every ancestor of `dir` (inclusive), nearest first. */ +/** Every ancestor of `dir` (inclusive), nearest first, up to the filesystem root. */ function ancestors(dir: string): string[] { const out: string[] = []; let current = resolve(dir); @@ -80,6 +81,81 @@ function ancestors(dir: string): string[] { } } +/** Is `dir` the same directory as `root`, or inside it? */ +function isWithin(dir: string, root: string): boolean { + const rel = relative(root, dir); + return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel)); +} + +/** Match a workspace glob (`packages/*`, `apps/**`, `tools/cli`) against a + * POSIX relative path. Only the shapes package managers document. */ +function workspaceGlobMatches(glob: string, rel: string): boolean { + const pattern = glob.replace(/^\.\//, "").replace(/\/+$/, "").split("/").filter(Boolean); + if (pattern.length === 0) return false; + const segment = (p: string) => new RegExp(`^${p.replace(/[.+^${}()|[\]\\]/g, "\\$&").replace(/\*/g, "[^/]*")}$`); + const match = (p: string[], r: string[]): boolean => { + if (p.length === 0) return r.length === 0; + if (p[0] === "**") return match(p.slice(1), r) || (r.length > 0 && match(p, r.slice(1))); + return r.length > 0 && segment(p[0]!).test(r[0]!) && match(p.slice(1), r.slice(1)); + }; + return match(pattern, rel.split("/").filter(Boolean)); +} + +/** The workspace globs `dir` declares (pnpm-workspace.yaml or package.json + * `workspaces`), or null when `dir` is not a workspace root. */ +function workspaceGlobs(dir: string): string[] | null { + const pnpmManifest = join(dir, "pnpm-workspace.yaml"); + if (existsSync(pnpmManifest)) { + try { + return parseYamlList(readFileSync(pnpmManifest, "utf-8"), "packages"); + } catch { + return []; + } + } + const manifest = join(dir, "package.json"); + if (!existsSync(manifest)) return null; + try { + const ws = JSON.parse(readFileSync(manifest, "utf-8"))?.workspaces; + if (Array.isArray(ws)) return ws; + if (ws && Array.isArray(ws.packages)) return ws.packages; + } catch { + /* unreadable manifest — not a workspace root */ + } + return null; +} + +/** The directory whose installs count for a scan of `scanRoot`. + * + * That is the scan root itself — an install somewhere above the project being + * scanned is not *its* install (#100: a nested project under an installed parent + * resolved the parent's `typescript`, and `tsc` ran half-blind). The one + * exception is a scan root that is a member of an enclosing workspace: npm, yarn + * and bun hoist its dependencies to the workspace root, so the boundary widens + * to that root. Membership is read from the nearest workspace manifest above the + * scan root and must match one of its package globs (negations honoured). */ +export function installBoundary(scanRoot: string): string { + const root = resolve(scanRoot); + for (const dir of ancestors(root).slice(1)) { + const globs = workspaceGlobs(dir); + if (globs === null) continue; + const rel = relative(dir, root).split(sep).join("/"); + const included = globs.some((g) => !g.startsWith("!") && workspaceGlobMatches(g, rel)); + const excluded = globs.some((g) => g.startsWith("!") && workspaceGlobMatches(g.slice(1), rel)); + return included && !excluded ? dir : root; + } + return root; +} + +/** The directories Node would search for `dir`'s dependencies, nearest first, + * stopping at the install boundary of `scanRoot`. A `dir` outside that + * boundary is searched on its own. */ +function searchPath(dir: string, scanRoot: string): string[] { + const boundary = installBoundary(scanRoot); + const all = ancestors(dir); + if (!isWithin(all[0]!, boundary)) return [all[0]!]; + return all.filter((d) => isWithin(d, boundary)); +} + /** Which ancestors hold an install (node_modules or a Yarn PnP map). This is * what an `install` changes, so it keys the cache: a long-lived process that * sees the user install re-probes instead of answering from a stale result. */ @@ -104,22 +180,27 @@ function inferPackageManager(dirs: string[]): DependencyProbe["packageManager"] } /** Probe whether the dependencies of the project at `dir` are installed. + * + * `scanRoot` is the directory being scanned. The lookup never leaves its + * install boundary (see `installBoundary`): what is installed above the + * project being scanned says nothing about the project's own install. * * Reads the nearest package.json at or above `dir` — the package that declares * the dependencies — and asks whether any of its `dependencies` / * `devDependencies` resolves the way Node would resolve it: a - * `node_modules/` in that directory or any ancestor, so workspace-root - * hoisting (npm, yarn, bun) and pnpm's per-package symlinks both count. A Yarn - * Plug'n'Play map (`.pnp.cjs`) on the path counts as installed too. + * `node_modules/` in that directory or an ancestor within the boundary, so + * workspace-root hoisting (npm, yarn, bun) and pnpm's per-package symlinks both + * count. A Yarn Plug'n'Play map (`.pnp.cjs`) on the path counts as installed too. * * "Any", not "all": optional and platform-specific packages are legitimately * absent from a good install, while a checkout nobody installed resolves none. * A manifest that declares nothing has nothing to install, so it passes. * - * Cached per directory against the install fingerprint of its ancestors. */ -export function probeDependencies(dir: string): DependencyProbe { - const dirs = ancestors(dir); - const key = dirs[0]!; + * Cached per directory and boundary against the install fingerprint of the + * searched directories. */ +export function probeDependencies(dir: string, scanRoot: string): DependencyProbe { + const dirs = searchPath(dir, scanRoot); + const key = dirs.join("\u0000"); const fingerprint = installFingerprint(dirs); const cached = depProbes.get(key); if (cached && cached.fingerprint === fingerprint) return cached.probe; @@ -160,6 +241,8 @@ export function dependenciesMissingReason(packageManager: DependencyProbe["packa } /** Why a tool cannot run against the project at `dir`, or null when it can. + * + * `scanRoot` bounds the lookup exactly as in `probeDependencies`. * * Fails when the project's dependencies are not installed at all, and — when * `toolPackage` is given — when that package does not resolve from `dir`. The @@ -167,10 +250,10 @@ export function dependenciesMissingReason(packageManager: DependencyProbe["packa * fetches a package from the registry instead (for `tsc`, an unrelated package * of that name), so the run measures nothing about the project. That happens * at a monorepo root whose packages, not the root, declare the tool. */ -export function dependencyGap(dir: string, toolPackage?: string): string | null { - const deps = probeDependencies(dir); +export function dependencyGap(dir: string, scanRoot: string, toolPackage?: string): string | null { + const deps = probeDependencies(dir, scanRoot); if (!deps.installed) return dependenciesMissingReason(deps.packageManager); - if (toolPackage && !deps.pnp && !resolvesFrom(ancestors(dir), toolPackage)) { + if (toolPackage && !deps.pnp && !resolvesFrom(searchPath(dir, scanRoot), toolPackage)) { return `${dependenciesMissingReason(deps.packageManager)} (${toolPackage} does not resolve from this project)`; } return null; diff --git a/src/runners/types-check.test.ts b/src/runners/types-check.test.ts index 247cdc2..d3b8fd3 100644 --- a/src/runners/types-check.test.ts +++ b/src/runners/types-check.test.ts @@ -1,5 +1,6 @@ -import { mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { join } from "node:path"; +import { mkdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { dirname, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { detectWorkspace } from "../detect.js"; import type { Issue } from "../types.js"; @@ -17,6 +18,18 @@ function setup(files: Record) { } } +/** Install this repo's own TypeScript into a fixture, as ` install` would. + * + * These fixtures live inside the cli checkout and run the real `tsc`. They used + * to pass by resolving the checkout's node_modules from above — exactly the + * borrowed install #100 no longer counts — so each one now gets its own link. */ +function installTypeScript(dir: string) { + const tsDir = dirname(createRequire(import.meta.url).resolve("typescript/package.json")); + mkdirSync(join(dir, "node_modules/.bin"), { recursive: true }); + symlinkSync(tsDir, join(dir, "node_modules/typescript"), "dir"); + symlinkSync(join(tsDir, "bin/tsc"), join(dir, "node_modules/.bin/tsc")); +} + afterEach(() => { rmSync(TMP, { recursive: true, force: true }); }); @@ -128,6 +141,7 @@ describe("typeCheckTargets", () => { }), "packages/web/src/App.tsx": "declare namespace JSX { interface IntrinsicElements { div: any } }\nexport const App =
;\n", }); + installTypeScript(TMP); const workspace = detectWorkspace(TMP); const result = runTypeCheck(TMP, false, workspace); @@ -148,6 +162,7 @@ describe("typeCheckTargets", () => { "tsconfig.json": JSON.stringify({ compilerOptions: { strict: true }, include: ["src/**/*.tsx"] }), "src/App.tsx": "declare namespace JSX { interface IntrinsicElements { div: any } }\nexport const App =
;\n", }); + installTypeScript(TMP); const workspace = detectWorkspace(TMP); const result = runTypeCheck(TMP, false, workspace); diff --git a/src/runners/types-check.ts b/src/runners/types-check.ts index 6661264..3b46bd2 100644 --- a/src/runners/types-check.ts +++ b/src/runners/types-check.ts @@ -87,7 +87,7 @@ export function runTypeCheck(cwd: string, isDart = false, workspace?: WorkspaceI : targets.filter((target) => { // `npx tsc` must find the project's own TypeScript; a package script // (`pnpm typecheck`) needs the install but resolves its own binaries. - const gap = dependencyGap(target.cwd, target.mode === "script" ? undefined : "typescript"); + const gap = dependencyGap(target.cwd, cwd, target.mode === "script" ? undefined : "typescript"); if (!gap) return true; unavailableTargets.push({ id: target.projectId ?? "root", From 61de53051b8d5a30d2eeb5bae3e377708ad60f45 Mon Sep 17 00:00:00 2001 From: Sergey Ivochkin Date: Mon, 5 Oct 2026 19:34:45 +1100 Subject: [PATCH 3/3] fix(performance): say on the check when knip was skipped for missing dependencies (#100) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With knip skipped, `performance` carries no dead-code penalty, so a fresh checkout could score higher than the same code installed, and the only trace was details.deadCodeUnavailable. The check now gets an info issue (rule dead-code-unavailable) naming the folders knip did not run in and the install command, e.g. "Dead code not measured in . (root) — dependencies not installed (run `pnpm install`); this score does not cover dead code there". The issue does not change the score: performance is computed from its penalties, not from its issues, and the derived dead-code check only picks up the dead-files / dead-exports / unused-deps rules. Each deadCodeUnavailable entry now also carries its `install` command. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SmtSK7qDTyXhimJ9grap88 --- src/runners/performance.ts | 21 +++++++++++++++++++-- src/runners/toolchain.test.ts | 15 ++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/runners/performance.ts b/src/runners/performance.ts index c31496f..c188622 100644 --- a/src/runners/performance.ts +++ b/src/runners/performance.ts @@ -167,6 +167,19 @@ export function runPerformance(cwd: string, workspace?: WorkspaceInfo, inventory let unusedFiles = 0; let unusedDeps = 0; const { findings: knipResult, unavailable: knipUnavailable } = tryKnip(cwd, workspace); + if (knipUnavailable.length > 0) { + // Without this, a fresh checkout scores `performance` with no dead-code + // penalty and nothing on the check says why — it reads as "no dead code" + // and can outscore the same code installed (#100). Info only: it does not + // change the score, which is computed from the penalties below. + const folders = knipUnavailable.map((u) => (u.path === "." ? ". (root)" : u.path)).join(", "); + const commands = [...new Set(knipUnavailable.map((u) => u.install))].map((c) => `\`${c}\``).join(" or "); + issues.push({ + severity: "info", + message: `Dead code not measured in ${folders} — dependencies not installed${commands ? ` (run ${commands})` : ""}; this score does not cover dead code there`, + rule: "dead-code-unavailable", + }); + } if (knipResult) { deadExports = knipResult.exports; unusedFiles = knipResult.files; @@ -495,7 +508,7 @@ export function knipRoots(cwd: string, workspace?: WorkspaceInfo): { dir: string interface KnipOutcome { findings: (KnipFindings & { files: number; exports: number; deps: number; configured: boolean; excluded: number }) | null; /** Roots knip was not run in because their dependencies are not installed. */ - unavailable: Array<{ path: string; reason: string }>; + unavailable: Array<{ path: string; reason: string; install: string }>; } function tryKnip(cwd: string, workspace?: WorkspaceInfo): KnipOutcome { @@ -511,7 +524,11 @@ function tryKnip(cwd: string, workspace?: WorkspaceInfo): KnipOutcome { // missing install, not the code (#100). Do not run it at all. const deps = probeDependencies(root.dir, cwd); if (!deps.installed) { - unavailable.push({ path: root.rel || ".", reason: dependenciesMissingReason(deps.packageManager) }); + unavailable.push({ + path: root.rel || ".", + reason: dependenciesMissingReason(deps.packageManager), + install: `${deps.packageManager} install`, + }); continue; } const { stdout } = run("npx knip --reporter json 2>/dev/null || true", root.dir, 60_000); diff --git a/src/runners/toolchain.test.ts b/src/runners/toolchain.test.ts index 7db960d..9df0a1f 100644 --- a/src/runners/toolchain.test.ts +++ b/src/runners/toolchain.test.ts @@ -153,9 +153,16 @@ describe("TypeScript + ESLint fixture without node_modules (#100)", () => { const perf = runPerformance(dir, detectWorkspace(dir)); const details = perf.details as Record; expect(details.deadCodeTool).toBeUndefined(); - expect(details.deadCodeUnavailable).toEqual([{ path: ".", reason }]); + expect(details.deadCodeUnavailable).toEqual([{ path: ".", reason, install: "pnpm install" }]); expect(perf.score).toBe(100); expect(details.unavailable).toBeUndefined(); + // The skipped part is visible on the check itself, not only in details. + expect(perf.issues).toContainEqual({ + severity: "info", + rule: "dead-code-unavailable", + message: + "Dead code not measured in . (root) — dependencies not installed (run `pnpm install`); this score does not cover dead code there", + }); const deadCode = deadCodeCheckFromPerformance(perf); expect(deadCode.name).toBe("dead-code"); @@ -182,6 +189,7 @@ describe("the same fixture with dependencies installed (#100)", () => { const perf = runPerformance(dir, workspace); expect(perf.details).toMatchObject({ deadCodeTool: "knip", unusedFiles: 1 }); expect((perf.details as Record).deadCodeUnavailable).toBeUndefined(); + expect(perf.issues.some((i) => i.rule === "dead-code-unavailable")).toBe(false); expect(deadCodeCheckFromPerformance(perf).details.unavailable).toBeUndefined(); expect(npxCalls().map((c) => c.cmd.split(" ").slice(0, 2).join(" "))).toEqual(["npx tsc", "npx eslint", "npx knip"]); @@ -343,12 +351,13 @@ describe("the probe stops at the project being scanned (#100, review round 1)", expect(probeDependencies(web, web).installed).toBe(true); }); - it("bounds knip the same way", () => { + it("bounds knip the same way and says so on the performance check", () => { const { child } = nestedUnderInstalledParent(); const perf = runPerformance(child, detectWorkspace(child)); expect((perf.details as Record).deadCodeUnavailable).toEqual([ - { path: ".", reason: "dependencies not installed — run `npm install`" }, + { path: ".", reason: "dependencies not installed — run `npm install`", install: "npm install" }, ]); + expect(perf.issues.filter((i) => i.rule === "dead-code-unavailable")).toHaveLength(1); expect(npxCalls()).toEqual([]); }); });