refactor: centralize npm lockfile selection

src/parsers/index.ts

@@ -1,7 +1,7 @@
 import fs from "node:fs";
 import path from "node:path";
 
-import { LOCKFILE_NAMES, LOCKFILE_PRIORITY_ROOT, type LockfileName } from "../constants.js";
+import { LOCKFILE_NAMES, LOCKFILE_PRIORITY_ROOT, NPM_LOCKFILE_NAMES, type LockfileName } from "../constants.js";
 import type { PackageRef, ScanInput, ScanSource } from "../types.js";
 import { findFiles, findNearestPackageJson, chooseBestLockfile, safeReadText, relativeOrName } from "../utils/file.js";
 import { loadFromBunLock } from "./bun-lock.js";
@@ -72,12 +72,12 @@ interface FoundLockfile {
   readonly declaredBy?: string;
 }
 
-/** The lockfile each package manager writes, for reading `packageManager`. */
-const PACKAGE_MANAGER_LOCKFILES: Readonly<Record<string, LockfileName>> = {
-  npm: "package-lock.json",
-  pnpm: "pnpm-lock.yaml",
-  yarn: "yarn.lock",
-  bun: "bun.lock",
+/** Each package manager's lockfiles, in the order it prefers them. */
+const PACKAGE_MANAGER_LOCKFILES: Readonly<Record<string, readonly LockfileName[]>> = {
+  npm: NPM_LOCKFILE_NAMES,
+  pnpm: ["pnpm-lock.yaml"],
+  yarn: ["yarn.lock"],
+  bun: ["bun.lock"],
 };
 
 /**
@@ -92,7 +92,7 @@ function readDeclaredPackageManager(projectRoot: string): string | null {
     const declared = (JSON.parse(manifest) as { packageManager?: unknown }).packageManager;
     if (typeof declared !== "string") return null;
     const name = declared.split("@")[0]?.trim().toLowerCase();
-    return name && name in PACKAGE_MANAGER_LOCKFILES ? name : null;
+    return name && Object.hasOwn(PACKAGE_MANAGER_LOCKFILES, name) ? name : null;
   } catch {
     // A manifest we cannot parse tells us nothing about the package manager.
     // Lockfile selection falls back to precedence rather than failing here.
@@ -116,15 +116,10 @@ function findRootLockfile(projectRoot: string): FoundLockfile | null {
   if (present.length === 0) return null;
 
   const declared = readDeclaredPackageManager(projectRoot);
-  // npm treats npm-shrinkwrap.json as authoritative over package-lock.json.
-  // Keep that rule even when package.json declares npm, while preserving the
-  // packageManager override for other ecosystems (for example, pnpm beside a
-  // stale package-lock.json).
-  const declaredLockfile = declared === "npm" && present.includes("npm-shrinkwrap.json")
-    ? "npm-shrinkwrap.json"
-    : declared ? PACKAGE_MANAGER_LOCKFILES[declared] : undefined;
-  const chosen =
-    declaredLockfile && present.includes(declaredLockfile) ? declaredLockfile : present[0]!;
+  const declaredLockfile = declared
+    ? PACKAGE_MANAGER_LOCKFILES[declared]?.find(name => present.includes(name))
+    : undefined;
+  const chosen = declaredLockfile ?? present[0]!;
 
   return {
     filePath: path.join(projectRoot, chosen),
@@ -138,6 +133,9 @@ function findRootLockfile(projectRoot: string): FoundLockfile | null {
 function describeLockfileChoice(found: FoundLockfile): string[] {
   if (found.skipped.length === 0) return [];
   const others = found.skipped.join(", ");
+  if (found.name === "npm-shrinkwrap.json" && found.skipped.length === 1 && found.skipped[0] === "package-lock.json") {
+    return [`Root also contains ${others}. Used npm-shrinkwrap.json because npm reads it ahead of package-lock.json.`];
+  }
   return found.declaredBy
     ? [`Root also contains ${others}. Used ${found.name} because package.json declares packageManager "${found.declaredBy}".`]
     : [`Root also contains ${others}. Used ${found.name} by precedence; no packageManager field declared.`];

tests/parsers/load-packages.test.ts

@@ -195,6 +195,9 @@ packages:
         expect.arrayContaining([expect.objectContaining({ name: "lodash" })]),
       );
       expect(result.packages.map(p => p.name)).not.toContain("express");
+      expect(result.notes).toContain(
+        "Root also contains package-lock.json. Used npm-shrinkwrap.json because npm reads it ahead of package-lock.json.",
+      );
     } finally {
       removeDir(projectDir);
     }
@@ -238,7 +241,9 @@ packages:
       expect(path.basename(result.filePath ?? "")).toBe("npm-shrinkwrap.json");
       expect(result.packages.map(p => p.name)).toContain("shrinkwrap-package");
       expect(result.packages.map(p => p.name)).not.toContain("stale-package");
-      expect(result.notes.find(note => note.includes("packageManager \"npm\""))).toBeDefined();
+      expect(result.notes).toContain(
+        "Root also contains package-lock.json. Used npm-shrinkwrap.json because npm reads it ahead of package-lock.json.",
+      );
     } finally {
       removeDir(projectDir);
     }
@@ -337,6 +342,40 @@ snapshots:
     fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify(manifest), "utf8");
   }
 
+  it.each([
+    {
+      label: "filename precedence with pnpm also present",
+      manifest: { name: "app" },
+      includeBun: false,
+      expectedNote: "Root also contains package-lock.json, pnpm-lock.yaml. Used npm-shrinkwrap.json by precedence; no packageManager field declared.",
+    },
+    {
+      label: "the npm declaration with Bun and pnpm also present",
+      manifest: { name: "app", packageManager: "npm@11.0.0" },
+      includeBun: true,
+      expectedNote: 'Root also contains bun.lock, package-lock.json, pnpm-lock.yaml. Used npm-shrinkwrap.json because package.json declares packageManager "npm".',
+    },
+  ])("explains $label when shrinkwrap wins", ({ manifest, includeBun, expectedNote }) => {
+    const projectDir = createTempProjectDir();
+    try {
+      writeBothLockfiles(projectDir, manifest);
+      fs.writeFileSync(path.join(projectDir, "npm-shrinkwrap.json"), JSON.stringify({
+        lockfileVersion: 3,
+        packages: { "node_modules/shrinkwrap-package": { version: "1.0.0" } },
+      }), "utf8");
+      if (includeBun) {
+        fs.writeFileSync(path.join(projectDir, "bun.lock"), "{}", "utf8");
+      }
+
+      const result = loadPackages(projectDir, false, 4);
+      expect(result.source).toBe("npm-shrinkwrap");
+      expect(result.packages.map(p => p.name)).toEqual(["shrinkwrap-package"]);
+      expect(result.notes).toContain(expectedNote);
+    } finally {
+      removeDir(projectDir);
+    }
+  });
+
   it("uses the lockfile the project declares in packageManager, not filename precedence", () => {
     const projectDir = createTempProjectDir();
     // Exactly the caveman shape: a pnpm workspace with a stale package-lock.json
@@ -372,6 +411,23 @@ snapshots:
     }
   });
 
+  it("uses package-lock.json when npm is declared and no shrinkwrap is present", () => {
+    const projectDir = createTempProjectDir();
+    writeBothLockfiles(projectDir, { name: "app", packageManager: "npm@11.0.0" });
+
+    try {
+      const result = loadPackages(projectDir, false, 4);
+      expect(result.source).toBe("package-lock");
+      expect(result.packages.map(p => p.name)).toContain("leftpad");
+      expect(result.packages.map(p => p.name)).not.toContain("chalk");
+      expect(result.notes).toContain(
+        'Root also contains pnpm-lock.yaml. Used package-lock.json because package.json declares packageManager "npm".',
+      );
+    } finally {
+      removeDir(projectDir);
+    }
+  });
+
   it("falls back to precedence when no packageManager is declared, and says so", () => {
     const projectDir = createTempProjectDir();
     writeBothLockfiles(projectDir, { name: "app" });
@@ -386,10 +442,10 @@ snapshots:
     }
   });
 
-  it("ignores a packageManager naming a lockfile that is not present", () => {
+  it.each(["yarn@4.0.0", "constructor", "__proto__"])("falls back to precedence when packageManager %s has no matching lockfile", (packageManager) => {
     const projectDir = createTempProjectDir();
-    // Declares yarn, but only the pnpm and npm lockfiles exist.
-    writeBothLockfiles(projectDir, { name: "app", packageManager: "yarn@4.0.0" });
+    // Only pnpm and npm lockfiles exist; unsupported names must also fall back safely.
+    writeBothLockfiles(projectDir, { name: "app", packageManager });
 
     try {
       const result = loadPackages(projectDir, false, 4);