Reorganize exports/types and nsExports/nsTypes issue types (resolves #…

…475)

packages/knip/fixtures/imports-namespace-with-nsexports/index.ts

@@ -0,0 +1,26 @@
+export * as ReExported from './re-exported-module';
+import * as NS from './namespace';
+import * as NS2 from './namespace2';
+import * as NS3 from './namespace3';
+import * as NS4 from './namespace4';
+import * as NS5 from './namespace5';
+import * as NS6 from './namespace6';
+import fn from 'external';
+
+NS.identifier15;
+NS['identifier16'];
+NS.identifier17();
+
+const { identifier18, identifier19, identifier20 } = NS2;
+
+NS2.identifier21.method();
+
+function usage() {
+  const hello = { NS3 };
+}
+
+fn(NS4);
+
+const spread = { ...NS5 };
+
+const assign = NS6;

packages/knip/fixtures/imports-namespace-with-nsexports/namespace.ts

@@ -0,0 +1,3 @@
+export const identifier15 = 1;
+export const identifier16 = 1;
+export const identifier17 = () => 1;

packages/knip/fixtures/imports-namespace-with-nsexports/namespace2.ts

@@ -0,0 +1,4 @@
+export const identifier18 = 1;
+export const identifier19 = 1;
+export const identifier20 = () => 1;
+export const identifier21 = { method: () => 1 };

packages/knip/fixtures/imports-namespace-with-nsexports/namespace3.ts

@@ -0,0 +1,2 @@
+export const identifier31 = 31;
+export const identifier32 = 32;

packages/knip/fixtures/imports-namespace-with-nsexports/namespace4.ts

@@ -0,0 +1,2 @@
+export const identifier33 = 33;
+export const identifier34 = 34;

packages/knip/fixtures/imports-namespace-with-nsexports/namespace5.ts

@@ -0,0 +1,2 @@
+export const identifier35 = 35;
+export const identifier36 = 36;

packages/knip/fixtures/imports-namespace-with-nsexports/namespace6.ts

@@ -0,0 +1,2 @@
+export const identifier37 = 37;
+export const identifier38 = 38;

packages/knip/fixtures/imports-namespace-with-nsexports/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@fixtures/imports-namespace-with-nsexports",
+  "knip": {
+    "include": [
+      "nsExports"
+    ]
+  }
+}

packages/knip/fixtures/imports-namespace-with-nsexports/re-exported-module.ts

@@ -0,0 +1,3 @@
+const myFunction = () => void 0;
+
+export default myFunction;

packages/knip/fixtures/imports-namespace/index.ts

@@ -1,6 +1,11 @@
 export * as ReExported from './re-exported-module';
 import * as NS from './namespace';
 import * as NS2 from './namespace2';
+import * as NS3 from './namespace3';
+import * as NS4 from './namespace4';
+import * as NS5 from './namespace5';
+import * as NS6 from './namespace6';
+import fn from 'external';
 
 NS.identifier15;
 NS['identifier16'];
@@ -9,3 +14,13 @@ NS.identifier17();
 const { identifier18, identifier19, identifier20 } = NS2;
 
 NS2.identifier21.method();
+
+function usage() {
+  const hello = { NS3 };
+}
+
+fn(NS4);
+
+const spread = { ...NS5 };
+
+const assign = NS6;

packages/knip/fixtures/imports-namespace/namespace3.ts

@@ -0,0 +1,2 @@
+export const identifier31 = 31;
+export const identifier32 = 32;

packages/knip/fixtures/imports-namespace/namespace4.ts

@@ -0,0 +1,2 @@
+export const identifier33 = 33;
+export const identifier34 = 34;

packages/knip/fixtures/imports-namespace/namespace5.ts

@@ -0,0 +1,2 @@
+export const identifier35 = 35;
+export const identifier36 = 36;

packages/knip/fixtures/imports-namespace/namespace6.ts

@@ -0,0 +1,2 @@
+export const identifier37 = 37;
+export const identifier38 = 38;

packages/knip/fixtures/imports-namespace/package.json

@@ -1,11 +1,3 @@
 {
-  "name": "@fixtures/namespace",
-  "knip": {
-    "entry": [
-      "index.ts"
-    ],
-    "project": [
-      "*.ts"
-    ]
-  }
+  "name": "@fixtures/imports-namespace"
 }

packages/knip/src/constants.ts

@@ -113,9 +113,9 @@ export const ISSUE_TYPE_TITLE: Record<IssueType, string> = {
   binaries: 'Unlisted binaries',
   unresolved: 'Unresolved imports',
   exports: 'Unused exports',
-  nsExports: 'Unused exports in namespaces',
+  nsExports: 'Exports in used namespace',
   types: 'Unused exported types',
-  nsTypes: 'Unused exported types in namespaces',
+  nsTypes: 'Exported types in used namespace',
   enumMembers: 'Unused exported enum members',
   classMembers: 'Unused exported class members',
   duplicates: 'Duplicate exports',

packages/knip/src/index.ts

@@ -23,6 +23,7 @@ import { fromBinary, isBinary } from './util/protocols.js';
 import { _resolveSpecifier } from './util/require.js';
 import { shouldIgnore } from './util/tag.js';
 import { loadTSConfig } from './util/tsconfig-loader.js';
+import { getType, getHasStrictlyNsReferences } from './util/type.js';
 import { WorkspaceWorker } from './WorkspaceWorker.js';
 import type { Workspace } from './ConfigurationChief.js';
 import type { CommandLineOptions } from './types/cli.js';
@@ -577,19 +578,20 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
             }
           }
 
-          const hasNsImport = Boolean(importsForExport?.hasStar);
+          const [hasStrictlyNsReferences, namespace] = getHasStrictlyNsReferences(importsForExport);
 
           const isType = ['enum', 'type', 'interface'].includes(exportedItem.type);
 
-          if (hasNsImport && ((!report.nsTypes && isType) || (!report.nsExports && !isType))) continue;
+          if (hasStrictlyNsReferences && ((!report.nsTypes && isType) || (!report.nsExports && !isType))) continue;
 
           if (!isExportedItemReferenced(exportedItem)) {
-            const type = isType ? (hasNsImport ? 'nsTypes' : 'types') : hasNsImport ? 'nsExports' : 'exports';
+            const type = getType(hasStrictlyNsReferences, isType);
             collector.addIssue({
               type,
               filePath,
               symbol: identifier,
               symbolType: exportedItem.type,
+              parentSymbol: namespace,
               pos: exportedItem.pos,
               line: exportedItem.line,
               col: exportedItem.col,

packages/knip/src/reporters/json.ts

@@ -2,7 +2,7 @@ import { OwnershipEngine } from '@snyk/github-codeowners/dist/lib/ownership/inde
 import { isFile } from '../util/fs.js';
 import { relative, resolve } from '../util/path.js';
 import { convert } from './util.js';
-import type { Report, ReporterOptions, IssueRecords, SymbolIssueType, Issue } from '../types/issues.js';
+import type { Report, ReporterOptions, IssueRecords, Issue } from '../types/issues.js';
 import type { Entries } from 'type-fest';
 
 type ExtraReporterOptions = {
@@ -22,14 +22,13 @@ type Row = {
   unresolved?: Array<{ name: string }>;
   exports?: Array<Item>;
   types?: Array<Item>;
+  nsExports?: Array<Item>;
+  nsTypes?: Array<Item>;
   duplicates?: Array<Item[]>;
   enumMembers?: Record<string, Array<Item>>;
   classMembers?: Record<string, Array<Item>>;
 };
 
-const mergeTypes = (type: SymbolIssueType) =>
-  type === 'exports' || type === 'nsExports' ? 'exports' : type === 'types' || type === 'nsTypes' ? 'types' : type;
-
 export default async ({ report, issues, options }: ReporterOptions) => {
   let opts: ExtraReporterOptions = {};
   try {
@@ -55,22 +54,23 @@ export default async ({ report, issues, options }: ReporterOptions) => {
       ...(report.unlisted && { unlisted: [] }),
       ...(report.binaries && { binaries: [] }),
       ...(report.unresolved && { unresolved: [] }),
-      ...((report.exports || report.nsExports) && { exports: [] }),
-      ...((report.types || report.nsTypes) && { types: [] }),
+      ...(report.exports && { exports: [] }),
+      ...(report.nsExports && { nsExports: [] }),
+      ...(report.types && { types: [] }),
+      ...(report.nsTypes && { nsTypes: [] }),
       ...(report.enumMembers && { enumMembers: {} }),
       ...(report.classMembers && { classMembers: {} }),
       ...(report.duplicates && { duplicates: [] }),
     };
     return row;
   };
 
-  for (const [reportType, isReportType] of Object.entries(report) as Entries<Report>) {
+  for (const [type, isReportType] of Object.entries(report) as Entries<Report>) {
     if (isReportType) {
-      if (reportType === 'files') {
+      if (type === 'files') {
         // Ignore
       } else {
-        const type = mergeTypes(reportType);
-        flatten(issues[reportType] as IssueRecords).forEach(issue => {
+        flatten(issues[type] as IssueRecords).forEach(issue => {
           const { filePath, symbol, symbols, parentSymbol } = issue;
           json[filePath] = json[filePath] ?? initRow(filePath);
           if (type === 'duplicates') {
@@ -82,7 +82,7 @@ export default async ({ report, issues, options }: ReporterOptions) => {
               item[parentSymbol].push(convert(issue));
             }
           } else {
-            if (type === 'exports' || type === 'types' || type === 'unresolved') {
+            if (['exports', 'nsExports', 'types', 'nsTypes', 'unresolved'].includes(type)) {
               json[filePath][type]?.push(convert(issue));
             } else {
               json[filePath][type]?.push({ name: symbol });

packages/knip/src/typescript/getImportsAndExports.ts

@@ -285,19 +285,56 @@ const getImportsAndExports = (
       result && (Array.isArray(result) ? result.forEach(addScript) : addScript(result));
     }
 
-    if (ts.isIdentifier(node) && isAccessExpression(node.parent)) {
-      const symbol = sourceFile.locals?.get(String(node.escapedText));
-      if (symbol) {
-        if (importedInternalSymbols.has(symbol)) {
-          let members: string[] = [];
-          let current: ts.Node = node.parent;
-          while (current) {
-            const ms = getMemberStringLiterals(typeChecker, current);
-            if (!ms) break;
-            members = members.concat(ms.flatMap(id => (members.length === 0 ? id : members.map(ns => `${ns}.${id}`))));
-            current = current.parent;
+    if (ts.isIdentifier(node)) {
+      if (isAccessExpression(node.parent)) {
+        const symbol = sourceFile.locals?.get(String(node.escapedText));
+        if (symbol) {
+          if (importedInternalSymbols.has(symbol)) {
+            let members: string[] = [];
+            let current: ts.Node = node.parent;
+            while (current) {
+              const ms = getMemberStringLiterals(typeChecker, current);
+              if (!ms) break;
+              members = members.concat(
+                ms.flatMap(id => (members.length === 0 ? id : members.map(ns => `${ns}.${id}`)))
+              );
+              current = current.parent;
+            }
+            maybeAddAccessExpressionAsNsImport(String(node.escapedText), members);
+          }
+        }
+      } else if (
+        // TODO Ideally we store NamespaceImport symbols and check directly against those, but can't get symbols to match
+        ts.isShorthandPropertyAssignment(node.parent) ||
+        (ts.isCallExpression(node.parent) && node.parent.arguments.includes(node)) ||
+        ts.isSpreadAssignment(node.parent) ||
+        ts.isExportAssignment(node.parent)
+      ) {
+        const symbol = sourceFile.locals?.get(String(node.escapedText));
+        if (symbol) {
+          const importedSymbolFilePath = importedInternalSymbols.get(symbol);
+          if (importedSymbolFilePath) {
+            internalImports[importedSymbolFilePath].identifiers.add(String(node.escapedText));
+          }
+        }
+      } else if (ts.isVariableDeclaration(node.parent)) {
+        if (ts.isVariableDeclarationList(node.parent.parent) && ts.isObjectBindingPattern(node.parent.name)) {
+          const symbol = sourceFile.locals?.get(String(node.escapedText));
+          if (symbol) {
+            const importedSymbolFilePath = importedInternalSymbols.get(symbol);
+            if (importedSymbolFilePath) {
+              const members = node.parent.name.elements.flatMap(decl => decl.name.getText());
+              maybeAddAccessExpressionAsNsImport(String(node.escapedText), members);
+            }
+          }
+        } else if (node.parent.initializer === node) {
+          const symbol = sourceFile.locals?.get(String(node.escapedText));
+          if (symbol) {
+            const importedSymbolFilePath = importedInternalSymbols.get(symbol);
+            if (importedSymbolFilePath) {
+              internalImports[importedSymbolFilePath].identifiers.add(String(node.escapedText));
+            }
           }
-          maybeAddAccessExpressionAsNsImport(String(node.escapedText), members);
         }
       }
     }

packages/knip/src/util/get-included-issue-types.ts

@@ -17,7 +17,8 @@ type Options = {
   exports?: boolean;
 };
 
-const defaultExcludedIssueTypes = ['classMembers'];
+/** @internal */
+export const defaultExcludedIssueTypes = ['classMembers', 'nsExports', 'nsTypes'];
 const defaultIssueTypes = ISSUE_TYPES.filter(type => !defaultExcludedIssueTypes.includes(type));
 
 const normalize = (values: string[]) => values.map(value => value.split(',')).flat();

packages/knip/src/util/type.ts

@@ -0,0 +1,16 @@
+import type { SerializableImports } from '../types/imports.js';
+
+export const getHasStrictlyNsReferences = (importsForExport: SerializableImports): [boolean, string?] => {
+  if (!importsForExport || !importsForExport.hasStar || importsForExport.importedNs.size === 0) return [false];
+  let namespace;
+  for (const ns of importsForExport.importedNs) {
+    const hasNs = importsForExport.identifiers.has(ns);
+    if (!hasNs) return [false, ns];
+    for (const id of importsForExport.identifiers) if (id.startsWith(ns + '.')) return [false, ns];
+    namespace = ns;
+  }
+  return [true, namespace];
+};
+
+export const getType = (hasOnlyNsReference: boolean, isType: boolean) =>
+  hasOnlyNsReference ? (isType ? 'nsTypes' : 'nsExports') : isType ? 'types' : 'exports';

packages/knip/test/cli-reporter-json.test.ts

@@ -74,31 +74,31 @@ test('knip --reporter json (exports & types)', () => {
         ],
       },
       {
-        file: 'named-exports.ts',
+        file: 'my-namespace.ts',
         dependencies: [],
         devDependencies: [],
         optionalPeerDependencies: [],
         unlisted: [],
         binaries: [],
         unresolved: [],
-        exports: [
-          { name: 'renamedExport', line: 6, col: 30, pos: 179 },
-          { name: 'namedExport', line: 7, col: 15, pos: 215 },
-        ],
-        types: [],
+        exports: [{ name: 'nsUnusedKey', line: 3, col: 14, pos: 84 }],
+        types: [{ name: 'MyNamespace', line: 5, col: 18, pos: 119 }],
         enumMembers: {},
         duplicates: [],
       },
       {
-        file: 'my-namespace.ts',
+        file: 'named-exports.ts',
         dependencies: [],
         devDependencies: [],
         optionalPeerDependencies: [],
         unlisted: [],
         binaries: [],
         unresolved: [],
-        exports: [{ name: 'nsUnusedKey', line: 3, col: 14, pos: 84 }],
-        types: [{ name: 'MyNamespace', line: 5, col: 18, pos: 119 }],
+        exports: [
+          { name: 'renamedExport', line: 6, col: 30, pos: 179 },
+          { name: 'namedExport', line: 7, col: 15, pos: 215 },
+        ],
+        types: [],
         enumMembers: {},
         duplicates: [],
       },

packages/knip/test/entry-js.test.ts

@@ -16,18 +16,14 @@ test('Find unused files and exports with JS entry file', async () => {
   assert.equal(issues.files.size, 1);
   assert(issues.files.has(join(cwd, 'dangling.js')));
 
-  assert.equal(Object.values(issues.exports).length, 1);
+  assert.equal(Object.values(issues.exports).length, 2);
   assert.equal(issues.exports['my-module.ts']['unused'].symbol, 'unused');
   assert.equal(issues.exports['my-module.ts']['default'].symbol, 'default');
+  assert.equal(issues.exports['my-namespace.ts']['key'].symbol, 'key');
 
-  assert.equal(Object.values(issues.types).length, 1);
+  assert.equal(Object.values(issues.types).length, 2);
   assert.equal(issues.types['my-module.ts']['AnyType'].symbolType, 'type');
-
-  assert.equal(Object.values(issues.nsExports).length, 1);
-  assert.equal(issues.nsExports['my-namespace.ts']['key'].symbol, 'key');
-
-  assert.equal(Object.values(issues.nsTypes).length, 1);
-  assert.equal(issues.nsTypes['my-namespace.ts']['MyNamespace'].symbol, 'MyNamespace');
+  assert.equal(issues.types['my-namespace.ts']['MyNamespace'].symbol, 'MyNamespace');
 
   assert.equal(Object.values(issues.duplicates).length, 1);
   assert.equal(issues.duplicates['my-module.ts']['myExport|default'].symbols?.length, 2);
@@ -36,10 +32,8 @@ test('Find unused files and exports with JS entry file', async () => {
     ...baseCounters,
     files: 1,
     unlisted: 0,
-    exports: 2,
-    nsExports: 1,
-    nsTypes: 1,
-    types: 1,
+    exports: 3,
+    types: 2,
     duplicates: 1,
     processed: 4,
     total: 4,