Refactor hasStrictlyNsReferences (resolve #1427)

webpro · webpro · commit 0768c8a50567 · 2026-01-04T14:23:01.000+01:00
diff --git a/packages/knip/fixtures/ns-spread-reexport/aliased-import.ts b/packages/knip/fixtures/ns-spread-reexport/aliased-import.ts
@@ -0,0 +1,6 @@
+// Tests importsForExport.importedAs: `import { foo as bar }`
+import { resolvers as myResolvers } from './resolvers.js';
+
+export function useAliasedResolvers() {
+  return myResolvers;
+}
diff --git a/packages/knip/fixtures/ns-spread-reexport/animals.ts b/packages/knip/fixtures/ns-spread-reexport/animals.ts
@@ -0,0 +1,2 @@
+export const cat = 'cat';
+export const dog = 'dog';
diff --git a/packages/knip/fixtures/ns-spread-reexport/colors.ts b/packages/knip/fixtures/ns-spread-reexport/colors.ts
@@ -0,0 +1,2 @@
+export const red = 'red';
+export const blue = 'blue';
diff --git a/packages/knip/fixtures/ns-spread-reexport/consumer.ts b/packages/knip/fixtures/ns-spread-reexport/consumer.ts
@@ -0,0 +1,6 @@
+// Consumes the spread namespace via named import
+import { utilsAlias } from './ns-alias-reexport.js';
+
+export function useUtils() {
+  return utilsAlias;
+}
diff --git a/packages/knip/fixtures/ns-spread-reexport/destructured.ts b/packages/knip/fixtures/ns-spread-reexport/destructured.ts
@@ -0,0 +1,8 @@
+// NOT strictly NS: destructured from namespace
+import * as Animals from './animals.js';
+
+const { cat } = Animals;
+
+export function useAnimal() {
+  return cat;
+}
diff --git a/packages/knip/fixtures/ns-spread-reexport/fruits.ts b/packages/knip/fixtures/ns-spread-reexport/fruits.ts
@@ -0,0 +1,2 @@
+export const apple = 'apple';
+export const banana = 'banana';
diff --git a/packages/knip/fixtures/ns-spread-reexport/hello.resolver.ts b/packages/knip/fixtures/ns-spread-reexport/hello.resolver.ts
@@ -0,0 +1 @@
+export const Hello = 'hi';
diff --git a/packages/knip/fixtures/ns-spread-reexport/index.ts b/packages/knip/fixtures/ns-spread-reexport/index.ts
@@ -0,0 +1,10 @@
+import { resolvers } from './resolvers.js';
+import { useAliasedResolvers } from './aliased-import.js';
+import { useUtils } from './consumer.js';
+import { useFruit } from './member-access.js';
+import { useAnimal } from './destructured.js';
+import { allColors, justRed } from './mixed-usage.js';
+
+export function useResolvers() {
+  return [resolvers, useAliasedResolvers, useUtils, useFruit, useAnimal, allColors, justRed];
+}
diff --git a/packages/knip/fixtures/ns-spread-reexport/member-access.ts b/packages/knip/fixtures/ns-spread-reexport/member-access.ts
@@ -0,0 +1,6 @@
+// NOT strictly NS: direct member access (NS.member)
+import * as Fruits from './fruits.js';
+
+export function useFruit() {
+  return Fruits.apple;
+}
diff --git a/packages/knip/fixtures/ns-spread-reexport/mixed-usage.ts b/packages/knip/fixtures/ns-spread-reexport/mixed-usage.ts
@@ -0,0 +1,5 @@
+// NOT strictly NS: mixed usage (spread + member access)
+import * as Colors from './colors.js';
+
+export const allColors = { ...Colors };
+export const justRed = Colors.red;
diff --git a/packages/knip/fixtures/ns-spread-reexport/ns-alias-reexport.ts b/packages/knip/fixtures/ns-spread-reexport/ns-alias-reexport.ts
@@ -0,0 +1,4 @@
+// Tests nsAliases traversal: namespace spread re-exported as alias
+import * as Utils from './utils.js';
+
+export const utilsAlias = { ...Utils };
diff --git a/packages/knip/fixtures/ns-spread-reexport/package.json b/packages/knip/fixtures/ns-spread-reexport/package.json
@@ -0,0 +1,3 @@
+{
+  "name": "@fixtures/ns-spread-reexport"
+}
diff --git a/packages/knip/fixtures/ns-spread-reexport/resolvers.barrel.ts b/packages/knip/fixtures/ns-spread-reexport/resolvers.barrel.ts
@@ -0,0 +1 @@
+export * from './hello.resolver.js';
diff --git a/packages/knip/fixtures/ns-spread-reexport/resolvers.ts b/packages/knip/fixtures/ns-spread-reexport/resolvers.ts
@@ -0,0 +1,5 @@
+import * as resolverBarrel from './resolvers.barrel.js';
+
+export const resolvers = {
+  ...resolverBarrel,
+};
diff --git a/packages/knip/fixtures/ns-spread-reexport/utils.ts b/packages/knip/fixtures/ns-spread-reexport/utils.ts
@@ -0,0 +1 @@
+export const helper = 'helper';
diff --git a/packages/knip/fixtures/re-exports-default-renamed-deep/index.ts b/packages/knip/fixtures/re-exports-default-renamed-deep/index.ts
@@ -1,7 +1,7 @@
 import * as NS from './intermediate';
 
 Object.getOwnPropertyNames(NS);
-// Uncommenting any of the following with result in all the others being unused (or w/ --include nsExports)
+// Uncommenting any of the following will result in all the others being unused (or w/ --include nsExports)
 // NS.notReexported.banana;
 // NS.namedReexported.coconut;
 // NS.defaultReexported.pineapple;
diff --git a/packages/knip/src/graph-explorer/operations/has-strictly-ns-references.ts b/packages/knip/src/graph-explorer/operations/has-strictly-ns-references.ts
@@ -1,10 +1,5 @@
 import type { ImportMaps, ModuleGraph } from '../../types/module-graph.js';
-import {
-  forEachAliasReExport,
-  forEachNamespaceReExport,
-  forEachPassThroughReExport,
-  getStarReExportSources,
-} from '../visitors.js';
+import { getAliasReExportMap, getPassThroughReExportSources, getStarReExportSources } from '../visitors.js';
 
 export const hasStrictlyNsReferences = (
   graph: ModuleGraph,
@@ -18,49 +13,15 @@ export const hasStrictlyNsReferences = (
   if (seenFiles.has(filePath)) return [false];
   seenFiles.add(filePath);
 
-  let foundNamespace: string | undefined;
+  let namespace: string | undefined;
 
-  const aliasByIdentifier = new Map<string, Array<{ alias: string; sources: Set<string> }>>();
-  const namespaceReExports = new Map<string, Array<Set<string>>>();
-  const namespaceEdges: Array<{ namespace: string; sources: Set<string> }> = [];
-  const directById = new Map<string, Set<string>>();
-
-  forEachPassThroughReExport(importsForExport, (id, sources) => {
-    directById.set(id, sources);
-  });
-
-  forEachAliasReExport(importsForExport, (id, alias, sources) => {
-    let arr = aliasByIdentifier.get(id);
-    if (!arr) {
-      arr = [];
-      aliasByIdentifier.set(id, arr);
-    }
-    arr.push({ alias, sources });
-  });
-
-  forEachNamespaceReExport(importsForExport, (namespace, sources) => {
-    namespaceEdges.push({ namespace, sources });
-    let arr = namespaceReExports.get(namespace);
-    if (!arr) {
-      arr = [];
-      namespaceReExports.set(namespace, arr);
-    }
-    arr.push(sources);
-  });
-
-  const starSources = getStarReExportSources(importsForExport);
-
-  const followReExports = (
-    sources: Set<string>,
-    nextId: string,
-    propagateNamespace = true
-  ): [boolean, string?] | undefined => {
+  const followReExports = (sources: Set<string>, nextId: string): [boolean, string?] | undefined => {
     for (const filePath of sources) {
       const file = graph.get(filePath);
       if (!file?.imported) continue;
       const result = hasStrictlyNsReferences(graph, filePath, file.imported, nextId, seenFiles);
-      if (result[0] === false) return result;
-      if (propagateNamespace && result[1]) foundNamespace = result[1];
+      if (result[0] === false && result[1]) return result;
+      if (result[1] && !namespace) namespace = result[1];
     }
     return undefined;
   };
@@ -73,50 +34,57 @@ export const hasStrictlyNsReferences = (
       if (ref.startsWith(`${ns}.`)) return [false, ns];
     }
 
-    const nsReExports = namespaceReExports.get(ns);
-    if (nsReExports) {
-      for (const sources of nsReExports) {
-        const result = followReExports(sources, identifier, false);
-        if (result) return result;
-      }
-    }
+    namespace = ns;
 
-    const nsAliases = aliasByIdentifier.get(ns);
+    const nsAliases = getAliasReExportMap(importsForExport, ns);
     if (nsAliases) {
-      for (const { sources } of nsAliases) {
-        const result = followReExports(sources, identifier, false);
+      for (const [alias, sources] of nsAliases) {
+        const result = followReExports(sources, alias);
         if (result) return result;
       }
     }
-
-    foundNamespace = ns;
   }
 
-  const directSources = directById.get(identifier);
+  const directSources = getPassThroughReExportSources(importsForExport, identifier);
   if (directSources) {
-    const result = followReExports(directSources, identifier, true);
+    const result = followReExports(directSources, identifier);
     if (result) return result;
   }
 
+  const starSources = getStarReExportSources(importsForExport);
   if (starSources) {
-    const result = followReExports(starSources, identifier, true);
+    const result = followReExports(starSources, identifier);
     if (result) return result;
   }
 
   const [id, ...rest] = identifier.split('.');
-  const aliasEntries = aliasByIdentifier.get(id);
+  const aliasEntries = getAliasReExportMap(importsForExport, id);
   if (aliasEntries) {
-    for (const { alias, sources } of aliasEntries) {
-      const result = followReExports(sources, [alias, ...rest].join('.'), true);
+    for (const [alias, sources] of aliasEntries) {
+      const result = followReExports(sources, [alias, ...rest].join('.'));
       if (result) return result;
     }
   }
 
-  for (const { namespace: ns, sources } of namespaceEdges) {
-    const result = followReExports(sources, `${ns}.${identifier}`, true);
+  for (const [ns, sources] of importsForExport.reExportedNs) {
+    const result = followReExports(sources, `${ns}.${identifier}`);
+    if (result) return result;
+  }
+
+  const importedSources = importsForExport.imported.get(identifier);
+  if (importedSources) {
+    const result = followReExports(importedSources, identifier);
     if (result) return result;
   }
 
-  if (foundNamespace) return [true, foundNamespace];
+  const importedAsMap = importsForExport.importedAs.get(identifier);
+  if (importedAsMap) {
+    for (const [alias, sources] of importedAsMap) {
+      const result = followReExports(sources, alias);
+      if (result) return result;
+    }
+  }
+
+  if (namespace) return [true, namespace];
   return [false];
 };
diff --git a/packages/knip/test/ns-spread-reexport.test.ts b/packages/knip/test/ns-spread-reexport.test.ts
@@ -0,0 +1,47 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+import { main } from '../src/index.js';
+import baseCounters from './helpers/baseCounters.js';
+import { createOptions } from './helpers/create-options.js';
+import { resolve } from './helpers/resolve.js';
+
+const cwd = resolve('fixtures/ns-spread-reexport');
+
+test('Should report members of re-exported spread namespace', async () => {
+  const options = await createOptions({ cwd });
+  const { issues, counters } = await main(options);
+
+  assert(issues.exports['fruits.ts']['Fruits.banana']);
+  assert(issues.exports['animals.ts']['Animals.dog']);
+  assert(issues.exports['colors.ts']['Colors.blue']);
+
+  assert(!issues.exports['hello.resolver.ts']);
+  assert(!issues.exports['utils.ts']);
+
+  assert.deepStrictEqual(counters, {
+    ...baseCounters,
+    exports: 3,
+    processed: 14,
+    total: 14,
+  });
+});
+
+test('Should report members of re-exported spread namespace (with nsExports)', async () => {
+  const options = await createOptions({ cwd, includedIssueTypes: ['nsExports'] });
+  const { issues, counters } = await main(options);
+
+  assert(issues.exports['fruits.ts']['Fruits.banana']);
+  assert(issues.exports['animals.ts']['Animals.dog']);
+  assert(issues.exports['colors.ts']['Colors.blue']);
+
+  assert(issues.nsExports['hello.resolver.ts']['resolverBarrel.Hello']);
+  assert(issues.nsExports['utils.ts']['Utils.helper']);
+
+  assert.deepStrictEqual(counters, {
+    ...baseCounters,
+    exports: 3,
+    nsExports: 2,
+    processed: 14,
+    total: 14,
+  });
+});

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+export const cat = 'cat';`
	`2`	`+export const dog = 'dog';`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+export const red = 'red';`
	`2`	`+export const blue = 'blue';`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+export const apple = 'apple';`
	`2`	`+export const banana = 'banana';`