Skip to content
Open
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
11 changes: 11 additions & 0 deletions src/overrides/context-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { loadFromPnpmLock } from "../parsers/pnpm-lock.js";
import { loadFromYarnLock } from "../parsers/yarn-lock.js";
import { loadFromBunLock } from "../parsers/bun-lock.js";
import { scanAllImports } from "../usage/scanner.js";
import { readWorkspaceMemberManifests } from "../utils/package-json.js";
import { getDeclaredPackages } from "./detectors/phantom-utils.js";

type LockfileReadResult =
| { kind: "ok"; names: Set<string> }
Expand Down Expand Up @@ -84,6 +86,14 @@ export function buildOverrideContext(

const importedPackageNames = scanAllImports(projectPath);

// Imports are collected from the whole tree, so on a monorepo root a member's
// own source is included. The PD detectors need each member's manifest to
// decide whether an import found under that member is actually declared.
const workspaceMembers = readWorkspaceMemberManifests(projectPath).map((member) => ({
dir: member.dir,
declared: getDeclaredPackages(member.manifest),
}));

return {
projectPath,
packageJson: parsed,
Expand All @@ -97,6 +107,7 @@ export function buildOverrideContext(
registryDistTags,
skippedDetectors: skipped,
importedPackageNames,
workspaceMembers,
auditLog: opts.auditLog,
logger: opts.logger,
};
Expand Down
13 changes: 13 additions & 0 deletions src/overrides/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ export interface RegistryDistTags {
[tag: string]: string | undefined;
}

/** A workspace member's location and declared dependencies - for PD001/PD002. */
export interface WorkspaceMember {
/** Directory relative to the project root, POSIX-separated (e.g. "apps/web"). */
dir: string;
/** Names declared in any dependency section of the member's package.json. */
declared: Set<string>;
}

/** Reason a detector was pre-emptively skipped (e.g., missing node_modules). */
export interface SkippedDetector {
ruleId: string;
Expand Down Expand Up @@ -88,6 +96,11 @@ export interface OverrideContext {
/** All bare module names imported in source files, mapped to relative file paths.
* Empty map if no source files were found. Populated by context-builder. */
importedPackageNames: Map<string, string[]>;
/** Workspace members (npm/yarn/bun `workspaces`, pnpm-workspace.yaml) and the
* packages each one declares. A source file under a member's directory
* resolves its imports against that member's manifest, not only the root's.
* Absent or empty for a single-package project. */
workspaceMembers?: WorkspaceMember[];
auditLog: AuditLogHandle;
logger: Logger;
}
7 changes: 4 additions & 3 deletions src/overrides/detectors/pd001-override-only-phantom.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { OverrideContext } from "../context.js";
import type { OverrideFinding } from "../types.js";
import { getDeclaredPackages, installCmd } from "./phantom-utils.js";
import { getDeclaredPackages, installCmd, undeclaredImportFiles } from "./phantom-utils.js";

const RULE_ID = "PD001" as const;

Expand All @@ -13,9 +13,10 @@ export function detect(ctx: OverrideContext): OverrideFinding[] {
const cmd = installCmd(ctx.packageManager);

const findings: OverrideFinding[] = [];
for (const [pkgName, files] of ctx.importedPackageNames) {
if (declared.has(pkgName)) continue;
for (const [pkgName, importedIn] of ctx.importedPackageNames) {
if (!overrideNames.has(pkgName)) continue;
const files = undeclaredImportFiles(ctx, pkgName, importedIn, declared);
if (files.length === 0) continue;

const shown = files.slice(0, 3);
const extra = files.length > 3 ? ` (+${files.length - 3} more)` : "";
Expand Down
7 changes: 4 additions & 3 deletions src/overrides/detectors/pd002-transitive-only-phantom.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { OverrideContext } from "../context.js";
import type { OverrideFinding } from "../types.js";
import { getDeclaredPackages, installCmd } from "./phantom-utils.js";
import { getDeclaredPackages, installCmd, undeclaredImportFiles } from "./phantom-utils.js";

const RULE_ID = "PD002" as const;

Expand All @@ -13,10 +13,11 @@ export function detect(ctx: OverrideContext): OverrideFinding[] {
const cmd = installCmd(ctx.packageManager);

const findings: OverrideFinding[] = [];
for (const [pkgName, files] of ctx.importedPackageNames) {
if (declared.has(pkgName)) continue;
for (const [pkgName, importedIn] of ctx.importedPackageNames) {
if (overrideNames.has(pkgName)) continue; // PD001 owns this
if (!ctx.lockfilePackageNames.has(pkgName)) continue; // not in graph
const files = undeclaredImportFiles(ctx, pkgName, importedIn, declared);
if (files.length === 0) continue;

const shown = files.slice(0, 3);
const extra = files.length > 3 ? ` (+${files.length - 3} more)` : "";
Expand Down
35 changes: 34 additions & 1 deletion src/overrides/detectors/phantom-utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { OverrideContext } from "../context.js";
import type { OverrideContext, WorkspaceMember } from "../context.js";

export function getDeclaredPackages(packageJson: Record<string, unknown>): Set<string> {
const declared = new Set<string>();
Expand All @@ -13,6 +13,39 @@ export function getDeclaredPackages(packageJson: Record<string, unknown>): Set<s
return declared;
}

// The importing files for which `pkgName` is genuinely undeclared. A root
// declaration satisfies every file (root node_modules is on every member's
// resolution path). Otherwise a file is resolved against the nearest enclosing
// workspace member: a dependency declared in apps/web/package.json is a real
// direct dependency for apps/web/src/*, not a phantom. Files outside any member
// fall back to the root manifest.
export function undeclaredImportFiles(
ctx: OverrideContext,
pkgName: string,
files: string[],
rootDeclared: Set<string>,
): string[] {
if (rootDeclared.has(pkgName)) return [];
const members = ctx.workspaceMembers ?? [];
if (members.length === 0) return files;

return files.filter((file) => {
const owner = owningWorkspaceMember(members, file);
return !owner || !owner.declared.has(pkgName);
});
}

function owningWorkspaceMember(members: WorkspaceMember[], file: string): WorkspaceMember | null {
const normalized = file.replace(/\\/g, "/");
let best: WorkspaceMember | null = null;
for (const member of members) {
if (!normalized.startsWith(`${member.dir}/`)) continue;
// Nested members (packages/a and packages/a/plugins/b): the deepest wins.
if (!best || member.dir.length > best.dir.length) best = member;
}
return best;
}

export function installCmd(pm: OverrideContext["packageManager"]): string {
if (pm === "pnpm") return "pnpm add";
if (pm === "yarn") return "yarn add";
Expand Down
109 changes: 96 additions & 13 deletions src/usage/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,97 @@ const ALL_IMPORTS_EXCLUDED_DIRS = new Set([
// await import('pkg')
const IMPORT_REQUIRE_REGEX = /(?:(?:import|export)\s+[\w\s{},*]+\s+from\s+['"]([^'"]+)['"]|import\s+['"]([^'"]+)['"]|require\s*\(\s*['"]([^'"]+)['"]\s*\)|import\s*\(\s*['"]([^'"]+)['"]\s*\))/g;

// Blank out `//` and `/* */` comments so the import regex never matches text
// that is not code. The motivating case is a JSDoc annotation such as
// `/** @type {import('postcss-load-config').Config} */`: nothing loads that
// package at runtime, but a raw-text regex sees `import('...')` and counts it
// as a dynamic import. String literals are tracked so a `//` inside
// 'https://...' is not mistaken for a line comment. Comment bodies are
// replaced with spaces (newlines kept) so nothing else shifts position.
export function stripComments(source: string): string {
let out = "";
let i = 0;
const n = source.length;

while (i < n) {
const ch = source[i];
const next = source[i + 1];

if (ch === "/" && next === "/") {
while (i < n && source[i] !== "\n") i++;
out += " ";
continue;
}

if (ch === "/" && next === "*") {
i += 2;
while (i < n && !(source[i] === "*" && source[i + 1] === "/")) {
out += source[i] === "\n" ? "\n" : " ";
i++;
}
i += 2;
out += " ";
continue;
}

if (ch === "'" || ch === '"' || ch === "`") {
const quote = ch;
out += ch;
i++;
while (i < n) {
const c = source[i];
out += c;
i++;
if (c === "\\" && i < n) {
out += source[i];
i++;
continue;
}
if (c === quote) break;
// An unterminated ' or " string ends at the line break; template
// literals may span lines.
if (c === "\n" && quote !== "`") break;
}
continue;
}

out += ch;
i++;
}

return out;
}

// `import type { X } from 'pkg'`, `export type { X } from 'pkg'`, and
// `import { type A, type B } from 'pkg'` are erased by the TypeScript compiler
// and never load `pkg` at runtime. A default import that happens to be named
// `type` (`import type from 'pkg'`) or a mixed list (`import { type A, b }`)
// still loads the module and is kept.
export function isTypeOnlyImportStatement(statement: string): boolean {
const clause = /^(?:import|export)\s+([\s\S]*?)\s+from\s/.exec(statement)?.[1]?.trim();
if (!clause) return false;

if (/^type\s+[\w{*]/.test(clause)) return true;

const braced = /^\{([\s\S]*)\}$/.exec(clause);
if (!braced) return false;
const specifiers = braced[1].split(",").map(s => s.trim()).filter(Boolean);
return specifiers.length > 0 && specifiers.every(s => /^type\s+\S/.test(s));
}

// Every module specifier that `content` would actually load at runtime:
// static imports and re-exports, side-effect imports, `require()` and dynamic
// `import()`, minus anything inside a comment or in a type-only import.
function collectRuntimeImportPaths(content: string): string[] {
const paths: string[] = [];
for (const match of stripComments(content).matchAll(IMPORT_REQUIRE_REGEX)) {
if (match[1] && isTypeOnlyImportStatement(match[0])) continue;
const importPath = match[1] || match[2] || match[3] || match[4];
if (importPath) paths.push(importPath);
}
return paths;
}

function getBareModuleName(importPath: string): string {
if (importPath.startsWith(".") || importPath.startsWith("/")) {
return "";
Expand Down Expand Up @@ -99,16 +190,11 @@ export function scanProjectForPackageUsage(
}
if (!hasPotentialMatch) return;

const matches = content.matchAll(IMPORT_REQUIRE_REGEX);
const foundPackages = new Set<string>();

for (const match of matches) {
const importPath = match[1] || match[2] || match[3] || match[4];
if (importPath) {
const bare = getBareModuleName(importPath);
if (bare && packagesToLookFor.has(bare)) {
foundPackages.add(bare);
}
for (const importPath of collectRuntimeImportPaths(content)) {
const bare = getBareModuleName(importPath);
if (bare && packagesToLookFor.has(bare)) {
foundPackages.add(bare);
}
}

Expand Down Expand Up @@ -162,12 +248,9 @@ export function scanAllImports(projectPath: string): Map<string, string[]> {
}
if (!content.includes("import") && !content.includes("require") && !content.includes("export")) return;

const matches = content.matchAll(IMPORT_REQUIRE_REGEX);
const relPath = path.relative(projectPath, filePath);

for (const match of matches) {
const importPath = match[1] || match[2] || match[3] || match[4];
if (!importPath) continue;
for (const importPath of collectRuntimeImportPaths(content)) {
const bare = getBareModuleName(importPath);
if (!bare) continue;
const existing = results.get(bare) ?? [];
Expand Down
24 changes: 24 additions & 0 deletions src/utils/package-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,30 @@ export function readDirectDependencyNames(projectPath: string, prodOnly: boolean
}
}

export type WorkspaceMemberManifest = {
/** Member directory relative to the project root, POSIX-separated (e.g. "apps/web"). */
dir: string;
manifest: Record<string, unknown>;
};

// Every workspace member declared by the root manifest's `workspaces` field or
// by pnpm-workspace.yaml, with its parsed package.json. Empty for a
// single-package project. Best-effort: unreadable members are skipped.
export function readWorkspaceMemberManifests(projectPath: string): WorkspaceMemberManifest[] {
const rootManifest = readPackageJsonObject(path.join(projectPath, "package.json"));
if (!rootManifest) return [];

const members: WorkspaceMemberManifest[] = [];
for (const packageJsonPath of resolveWorkspacePackageJsonPaths(projectPath, readWorkspacePatterns(rootManifest, projectPath))) {
const manifest = readPackageJsonObject(packageJsonPath);
if (!manifest) continue;
const dir = path.relative(projectPath, path.dirname(packageJsonPath)).split(path.sep).join("/");
if (!dir || dir === ".") continue;
members.push({ dir, manifest });
}
return members;
}

function readPackageJsonObject(filePath: string): Record<string, unknown> | null {
try {
const raw = JSON.parse(fs.readFileSync(filePath, "utf8"));
Expand Down
72 changes: 72 additions & 0 deletions tests/overrides/context-builder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,78 @@ describe("buildOverrideContext", () => {
expect(ctx.importedPackageNames.get("semver")).toEqual([expect.stringMatching(/src.index\.ts/)]);
});

it("collects workspace members and their declared packages for a pnpm workspace root (#966)", () => {
writeFileSync(join(dir, "package.json"), JSON.stringify({
name: "pd002-repro", private: true, version: "1.0.0",
}));
writeFileSync(join(dir, "pnpm-workspace.yaml"), "packages:\n - 'apps/*'\n");
mkdirSync(join(dir, "apps", "web", "src"), { recursive: true });
writeFileSync(join(dir, "apps", "web", "package.json"), JSON.stringify({
name: "@repro/web", dependencies: { "js-yaml": "^4.1.0" }, devDependencies: { vitest: "^2.0.0" },
}));
writeFileSync(join(dir, "apps", "web", "src", "index.ts"), "import yaml from 'js-yaml';");

const ctx = buildOverrideContext(dir, {
auditLog: NULL_AUDIT_LOG,
logger: makeNoopLogger() as any,
checkNetwork: false,
});

expect(ctx.workspaceMembers).toHaveLength(1);
expect(ctx.workspaceMembers![0]!.dir).toBe("apps/web");
expect([...ctx.workspaceMembers![0]!.declared].sort()).toEqual(["js-yaml", "vitest"]);
expect(ctx.importedPackageNames.get("js-yaml")).toEqual([expect.stringMatching(/apps.web.src.index\.ts/)]);
});

it("collects workspace members from the root manifest's workspaces field", () => {
writeFileSync(join(dir, "package.json"), JSON.stringify({
name: "root", workspaces: ["packages/*"],
}));
mkdirSync(join(dir, "packages", "a"), { recursive: true });
mkdirSync(join(dir, "packages", "b"), { recursive: true });
writeFileSync(join(dir, "packages", "a", "package.json"), JSON.stringify({ name: "a", dependencies: { lodash: "^4" } }));
writeFileSync(join(dir, "packages", "b", "package.json"), JSON.stringify({ name: "b", peerDependencies: { react: "^18" } }));

const ctx = buildOverrideContext(dir, {
auditLog: NULL_AUDIT_LOG,
logger: makeNoopLogger() as any,
checkNetwork: false,
});

expect(ctx.workspaceMembers!.map((m) => [m.dir, [...m.declared]])).toEqual(
expect.arrayContaining([["packages/a", ["lodash"]], ["packages/b", ["react"]]]),
);
});

it("leaves workspaceMembers empty for a single-package project", () => {
writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "x" }));

const ctx = buildOverrideContext(dir, {
auditLog: NULL_AUDIT_LOG,
logger: makeNoopLogger() as any,
checkNetwork: false,
});

expect(ctx.workspaceMembers).toEqual([]);
});

it("does not count a JSDoc import() type annotation as an import (#966)", () => {
writeFileSync(join(dir, "package.json"), JSON.stringify({ name: "x" }));
writeFileSync(join(dir, "postcss.config.mjs"), [
"/** @type {import('postcss-load-config').Config} */",
"const config = { plugins: { tailwindcss: {}, autoprefixer: {} } };",
"export default config;",
].join("\n"));

const ctx = buildOverrideContext(dir, {
auditLog: NULL_AUDIT_LOG,
logger: makeNoopLogger() as any,
checkNetwork: false,
});

expect(ctx.importedPackageNames.has("postcss-load-config")).toBe(false);
});

it("flags OA001/OA004/OA006/OA008 as skipped when node_modules is absent", () => {
writeFileSync(join(dir, "package.json"), JSON.stringify({
name: "x",
Expand Down
Loading