Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/core.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
49 changes: 39 additions & 10 deletions src/runners/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -20,20 +20,28 @@ export function runLint(cwd: string, stack: StackInfo, workspace?: WorkspaceInfo
if (stack.linter === "none" && projectRuns.length > 0) {
const projects: Array<Record<string, unknown>> = [];
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;
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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");
}
Expand Down
72 changes: 60 additions & 12 deletions src/runners/performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, { kb: number; alt: string }> = {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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;
Expand All @@ -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,
};
}

Expand Down
Loading