Simplify deslop stale package detection

packages/deslop-js/src/report/packages.ts

@@ -1,3 +1,3 @@
 import { resolve, join } from "node:path";
 import { readFileSync, existsSync } from "node:fs";
 import fg from "fast-glob";
@@ -18,10 +18,11 @@
 interface PackageJsonDependencies {
   dependencies?: Record<string, string>;
   devDependencies?: Record<string, string>;
+  peerDependencies?: Record<string, string>;
 }
 
-const discoverAllPackageJsonPaths = (rootDir: string): string[] => {
-  const paths = [join(rootDir, "package.json")];
+const discoverAllPackageJsonPathSet = (rootDir: string): Set<string> => {
+  const paths = new Set([join(rootDir, "package.json")]);
   const workspacePackageJsons = fg.sync("**/package.json", {
     cwd: rootDir,
     absolute: true,
@@ -30,9 +31,7 @@
     deep: 5,
   });
   for (const workspacePath of workspacePackageJsons) {
-    if (workspacePath !== paths[0] && !paths.includes(workspacePath)) {
-      paths.push(workspacePath);
-    }
+    paths.add(workspacePath);
   }
   return paths;
 };
@@ -66,21 +65,22 @@
   const usedPackageNames = collectUsedPackages(graph);
 
   const monorepoRoot = findMonorepoRoot(config.rootDir);
-  const nodeModulesSearchRoots =
+  const searchRoots =
     monorepoRoot && monorepoRoot !== config.rootDir
       ? [config.rootDir, monorepoRoot]
       : [config.rootDir];
 
-  const allPackageJsonPaths = discoverAllPackageJsonPaths(config.rootDir);
+  const allPackageJsonPathSet = discoverAllPackageJsonPathSet(config.rootDir);
   if (monorepoRoot) {
     const monorepoPackageJson = join(monorepoRoot, "package.json");
-    if (!allPackageJsonPaths.includes(monorepoPackageJson) && existsSync(monorepoPackageJson)) {
-      allPackageJsonPaths.push(monorepoPackageJson);
+    if (existsSync(monorepoPackageJson)) {
+      allPackageJsonPathSet.add(monorepoPackageJson);
     }
   }
+  const allPackageJsonPaths = [...allPackageJsonPathSet];
 
   const { binToPackage, packagesProvidingBinary } = buildBinaryPackageIndex(
-    nodeModulesSearchRoots,
+    searchRoots,
     declaredNames,
   );
 
@@ -110,11 +110,7 @@
   );
   for (const packageName of nxProjectReferenced) usedPackageNames.add(packageName);
 
-  const configSearchRoots =
-    monorepoRoot && monorepoRoot !== config.rootDir
-      ? [config.rootDir, monorepoRoot]
-      : [config.rootDir];
-  for (const configSearchRoot of configSearchRoots) {
+  for (const configSearchRoot of searchRoots) {
     const configReferenced = collectConfigReferencedPackages(
       configSearchRoot,
       graph,
@@ -154,39 +150,29 @@
   }
 
   if (declaredNames.has("react") && declaredNames.has("react-dom")) {
-    const packageJsonPath = resolve(config.rootDir, "package.json");
-    try {
-      const content = readFileSync(packageJsonPath, "utf-8");
-      const packageJson = JSON.parse(content);
-      const peerDeps = packageJson.peerDependencies ?? {};
-      if ("react" in peerDeps && declaredDependencies.get("react") === true) {
-        usedPackageNames.add("react");
-      }
-      if ("react-dom" in peerDeps && declaredDependencies.get("react-dom") === true) {
-        usedPackageNames.add("react-dom");
-      }
-    } catch {
-      // fall through
+    const peerDependencies = packageJson.peerDependencies ?? {};
+    if ("react" in peerDependencies && declaredDependencies.get("react") === true) {
+      usedPackageNames.add("react");
+    }
+    if ("react-dom" in peerDependencies && declaredDependencies.get("react-dom") === true) {
+      usedPackageNames.add("react-dom");
     }
   }
 
   const peerSatisfied = collectPeerSatisfiedPackages(
-    nodeModulesSearchRoots,
+    searchRoots,
     declaredNames,
     usedPackageNames,
   );
   for (const packageName of peerSatisfied) usedPackageNames.add(packageName);
 
   const overrideMappings = collectOverrideMappings(
-    configSearchRoots,
+    searchRoots,
     allPackageJsonPaths,
     monorepoRoot,
   );
-  for (const { fromPackage, toPackage } of overrideMappings) {
+  for (const { toPackage } of overrideMappings) {
     if (declaredNames.has(toPackage)) usedPackageNames.add(toPackage);
-    if (usedPackageNames.has(fromPackage) && declaredNames.has(toPackage)) {
-      usedPackageNames.add(toPackage);
-    }
   }
 
   const candidateUnused = new Set<string>();
@@ -311,7 +297,9 @@
       const binPackageJson = JSON.parse(binContent);
       const binField = binPackageJson.bin;
       if (typeof binField === "string" && binField.length > 0) {
-        binToPackage.set(packageName.split("/").pop()!, packageName);
+        const defaultBinaryName = packageName.split("/").at(-1);
+        if (!defaultBinaryName) continue;
+        binToPackage.set(defaultBinaryName, packageName);
         packagesProvidingBinary.add(packageName);
       } else if (typeof binField === "object" && binField !== null) {
         const binaryNames = Object.keys(binField);
@@ -756,6 +744,7 @@
 ): Set<string> => {
   const found = new Set<string>();
   if (candidatePackages.size === 0) return found;
+  const remainingCandidates = new Set(candidatePackages);
 
   const sourceFiles = fg.sync(SOURCE_FILE_GLOBS, {
     cwd: rootDir,
@@ -766,13 +755,13 @@
   });
 
   for (const filePath of sourceFiles) {
-    if (candidatePackages.size === 0) break;
+    if (remainingCandidates.size === 0) break;
     try {
       const content = readFileSync(filePath, "utf-8");
-      for (const packageName of candidatePackages) {
+      for (const packageName of remainingCandidates) {
         if (matchesPackageImportReference(content, packageName)) {
           found.add(packageName);
-          candidatePackages.delete(packageName);
+          remainingCandidates.delete(packageName);
         }
       }
     } catch {

packages/deslop-js/tests/analyze.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, test } from "node:test";
+import { before, describe, it, test } from "node:test";
 import assert from "node:assert/strict";
 import { resolve, relative } from "node:path";
 import { analyze, defineConfig } from "../src/index.js";
@@ -40,49 +40,48 @@ const staleDependencyNames = (result: ScanResult): string[] =>
   result.unusedDependencies.map((dep) => dep.name).sort();
 
 describe("simple-app", () => {
-  it("should detect orphan file", async () => {
-    const result = await scanFixture("simple-app");
-    const fixtureDir = resolve(FIXTURES_DIR, "simple-app");
+  let result: ScanResult;
+  const fixtureDir = resolve(FIXTURES_DIR, "simple-app");
+
+  before(async () => {
+    result = await scanFixture("simple-app");
+  });
+
+  it("should detect orphan file", () => {
     const unusedFilePaths = orphanPaths(result, fixtureDir);
     assert.ok(
       unusedFilePaths.includes("src/orphan.ts"),
       `orphan.ts should be unused, got: ${unusedFilePaths}`,
     );
   });
 
-  it("should detect unused exports in utils", async () => {
-    const result = await scanFixture("simple-app");
-    const fixtureDir = resolve(FIXTURES_DIR, "simple-app");
+  it("should detect unused exports in utils", () => {
     const exportsByFile = deadExportsByFile(result, fixtureDir);
     assert.ok(
       exportsByFile["src/utils.ts"]?.includes("unusedFunction"),
       `unusedFunction should be flagged, got: ${JSON.stringify(exportsByFile["src/utils.ts"])}`,
     );
   });
 
-  it("should detect unused dependency", async () => {
-    const result = await scanFixture("simple-app");
+  it("should detect unused dependency", () => {
     const deps = staleDependencyNames(result);
     assert.ok(deps.includes("unused-dep"), `unused-dep should be flagged, got: ${deps}`);
   });
 
-  it("should explain each unused dependency with a reason that names the package", async () => {
-    const result = await scanFixture("simple-app");
+  it("should explain each unused dependency with a reason that names the package", () => {
     const unusedDep = result.unusedDependencies.find((dep) => dep.name === "unused-dep");
     assert.ok(unusedDep, `unused-dep finding should exist, got: ${staleDependencyNames(result)}`);
     assert.equal(unusedDep.isDevDependency, false);
     assert.match(unusedDep.reason, /"unused-dep"/);
     assert.match(unusedDep.reason, /declared in dependencies\b/);
   });
 
-  it("should not flag usedFunction as unused", async () => {
-    const result = await scanFixture("simple-app");
+  it("should not flag usedFunction as unused", () => {
     const allUnusedNames = deadExportNames(result);
     assert.ok(!allUnusedNames.includes("usedFunction"), "usedFunction should not be unused");
   });
 
-  it("should flag react as unused (declared but never imported)", async () => {
-    const result = await scanFixture("simple-app");
+  it("should flag react as unused (declared but never imported)", () => {
     const deps = staleDependencyNames(result);
     assert.ok(deps.includes("react"), `react should be unused since never imported, got: ${deps}`);
   });
@@ -974,7 +973,9 @@ describe("import-dynamic", () => {
 describe("type-deps", () => {
   it("should detect type-only imports", async () => {
     const result = await scanFixture("type-deps");
-    assert.ok(result.totalFiles > 0, "should find files");
+    const deps = staleDependencyNames(result);
+    assert.ok(!deps.includes("express"), `express is imported as a value, got: ${deps}`);
+    assert.ok(!deps.includes("zod"), `zod is imported as a type, got: ${deps}`);
   });
 });
 
@@ -983,9 +984,9 @@ describe("orphan-barrel-subtree", () => {
     const result = await scanFixture("orphan-barrel-subtree");
     const fixtureDir = resolve(FIXTURES_DIR, "orphan-barrel-subtree");
     const unusedFilePaths = orphanPaths(result, fixtureDir);
-    assert.ok(
-      unusedFilePaths.includes("src/subtree/setup.ts"),
-      `setup.ts should be unused, got: ${unusedFilePaths}`,
+    assert.deepEqual(
+      unusedFilePaths.filter((filePath) => filePath.startsWith("src/subtree/")),
+      ["src/subtree/setup.ts", "src/subtree/tabs/helpers.ts", "src/subtree/tabs/index.ts"],
     );
   });
 });
@@ -4815,7 +4816,10 @@ describe("code-clones", () => {
     const result = await scanFixture("duplicate-blocks-basic", {
       duplicateBlocks: { enabled: true, mode: "semantic", minTokens: 30, minLines: 3 },
     });
-    if (result.duplicateBlocks.length === 0) return;
+    assert.ok(
+      result.duplicateBlocks.length > 0,
+      `expected duplicate blocks before checking clusters, got: ${JSON.stringify(result.duplicateBlocks, null, 2)}`,
+    );
     assert.ok(
       result.duplicateBlockClusters.length > 0,
       "expected at least one duplicate-block cluster when clones are present",