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 ed30fbe..5c1e50d 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, cwd) + : 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, 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, scanRoot: string): string | null { + return dependencyGap(dir, scanRoot, "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..c188622 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,20 @@ 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 (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; @@ -286,6 +300,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 +373,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 +505,32 @@ 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; install: 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, cwd); + if (!deps.installed) { + 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); const parsed = parseKnipJson(stdout); if (!parsed) continue; @@ -502,14 +547,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..9df0a1f --- /dev/null +++ b/src/runners/toolchain.test.ts @@ -0,0 +1,363 @@ +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 { dependencyGap, installBoundary, 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, 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, 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"), 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"), dir).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, 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"); + 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(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"]); + }); +}); + +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, 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([]); + }); +}); + +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 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`", install: "npm install" }, + ]); + expect(perf.issues.filter((i) => i.rule === "dead-code-unavailable")).toHaveLength(1); + expect(npxCalls()).toEqual([]); + }); +}); diff --git a/src/runners/toolchain.ts b/src/runners/toolchain.ts index 8ecdab9..b0b33b4 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,9 @@ * by installing something. */ +import { existsSync, readFileSync } from "node:fs"; +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"; @@ -40,9 +45,224 @@ 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, up to the filesystem root. */ +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; + } +} + +/** 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. */ +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. + * + * `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 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 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; + + 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. + * + * `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 + * 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, scanRoot: string, toolPackage?: string): string | null { + const deps = probeDependencies(dir, scanRoot); + if (!deps.installed) return dependenciesMissingReason(deps.packageManager); + if (toolPackage && !deps.pnp && !resolvesFrom(searchPath(dir, scanRoot), 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.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 d83f8c3..3b46bd2 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, 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,