Don't remove internally referenced bindingElement nodes (#808)

webpro-nl · Oct 10, 2024 · 307ef8d · 307ef8d
1 parent 1c04163
commit 307ef8d
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 22 deletions.
diff --git a/packages/knip/fixtures/exports-value-refs-default/refs.ts b/packages/knip/fixtures/exports-value-refs-default/refs.ts
@@ -21,3 +21,16 @@ export function logger(s: string) {
 function setLogger(log: typeof logger): void {
   log;
 }
+
+const obj = {
+  destrRefObj: 1,
+};
+
+const arr = [1, 2, 3];
+
+export const { destrRefObj } = obj;
+
+export const [destrRefArr] = arr;
+
+destrRefObj;
+destrRefArr;
diff --git a/packages/knip/src/typescript/ast-helpers.ts b/packages/knip/src/typescript/ast-helpers.ts
@@ -216,7 +216,7 @@ const getAncestorTypeDeclaration = (node: ts.Node) => {
   }
 };
 
-export const isReferencedInExported = (node: ts.Node) => {
+export const isReferencedInExport = (node: ts.Node) => {
   if (ts.isTypeQueryNode(node.parent) && isExported(node.parent.parent)) return true;
   if (ts.isTypeReferenceNode(node.parent) && isExported(node.parent.parent)) return true;
   const typeNode = getAncestorTypeDeclaration(node);

diff --git a/packages/knip/src/typescript/find-internal-references.ts b/packages/knip/src/typescript/find-internal-references.ts
@@ -11,7 +11,8 @@ export const findInternalReferences = (
   item: Export | ExportMember,
   sourceFile: ts.SourceFile,
   typeChecker: ts.TypeChecker,
-  referencedSymbolsInExportedTypes: Set<ts.Symbol>
+  referencedSymbolsInExport: Set<ts.Symbol>,
+  isBindingElement?: boolean
 ): [number, boolean] => {
   if (!item.symbol) return [0, false];
   if (item.identifier === '') return [1, false]; // not pretty, ideally we'd find ref(s) to empty-string enum key
@@ -23,7 +24,7 @@ export const findInternalReferences = (
   const symbols = new Set<ts.Symbol>();
 
   let refCount = 0;
-  let isSymbolInExportedType = false;
+  let isSymbolInExport = false;
   let index = 0;
 
   // biome-ignore lint/suspicious/noAssignInExpressions: deal with it
@@ -34,26 +35,27 @@ export const findInternalReferences = (
         // @ts-expect-error ts.getTokenAtPosition is internal fn
         const symbol = typeChecker.getSymbolAtLocation(ts.getTokenAtPosition(sourceFile, index));
         if (symbol) {
-          const isInExportedType = referencedSymbolsInExportedTypes.has(symbol);
+          const isInExport = referencedSymbolsInExport.has(symbol);
 
-          if (isInExportedType) isSymbolInExportedType = true;
+          if (isInExport) isSymbolInExport = true;
 
           if (item.symbol === symbol) {
             refCount++;
-            if (isInExportedType || isType(item)) return [refCount, isSymbolInExportedType];
+            if (isInExport || isType(item)) return [refCount, isSymbolInExport];
+            if (isBindingElement) return [refCount, true];
           }
 
           // @ts-expect-error Keep it cheap
           const declaration = symbol.declarations?.[0];
           if (declaration) {
             // Pattern: export { identifier }
             if (item.symbol === declaration.name?.flowNode?.node?.symbol) {
-              return [++refCount, isSymbolInExportedType];
+              return [++refCount, isSymbolInExport];
             }
 
             if (ts.isImportSpecifier(declaration) && symbols.has(symbol)) {
               // Consider re-exports referenced
-              return [++refCount, isSymbolInExportedType];
+              return [++refCount, isSymbolInExport];
             }
           }
 
@@ -64,5 +66,5 @@ export const findInternalReferences = (
     index += id.length;
   }
 
-  return [refCount, isSymbolInExportedType];
+  return [refCount, isSymbolInExport];
 };
diff --git a/packages/knip/src/typescript/get-imports-and-exports.ts b/packages/knip/src/typescript/get-imports-and-exports.ts
@@ -24,7 +24,7 @@ import {
   isDestructuring,
   isImportSpecifier,
   isObjectEnumerationCallExpressionArgument,
-  isReferencedInExported,
+  isReferencedInExport,
 } from './ast-helpers.js';
 import { findInternalReferences, isType } from './find-internal-references.js';
 import getDynamicImportVisitors from './visitors/dynamic-imports/index.js';
@@ -92,7 +92,7 @@ const getImportsAndExports = (
 
   const importedInternalSymbols = new Map<ts.Symbol, string>();
 
-  const referencedSymbolsInExportedTypes = new Set<ts.Symbol>();
+  const referencedSymbolsInExport = new Set<ts.Symbol>();
 
   const visitors = getVisitors(sourceFile);
 
@@ -373,8 +373,8 @@ const getImportsAndExports = (
 
         // Store exports referenced in exported types, including `typeof` values
         // Simplifies and speeds up (*) below while we're still in the realm of bound AST
-        if (!isTopLevel && symbol.exportSymbol && isReferencedInExported(node)) {
-          referencedSymbolsInExportedTypes.add(symbol.exportSymbol);
+        if (!isTopLevel && symbol.exportSymbol && isReferencedInExport(node)) {
+          referencedSymbolsInExport.add(symbol.exportSymbol);
         }
       }
     }
@@ -408,19 +408,24 @@ const getImportsAndExports = (
   // For each export, see if it's referenced in same file,
   // and whether it's referenced in an exported type and should be exported with it (*)
   for (const item of exports.values()) {
-    const isType_ = isType(item);
-    if (item.symbol && referencedSymbolsInExportedTypes.has(item.symbol)) {
+    if (item.symbol && referencedSymbolsInExport.has(item.symbol)) {
       item.refs = [1, true];
-    } else if (
-      ignoreExportsUsedInFile === true ||
-      (typeof ignoreExportsUsedInFile === 'object' && item.type !== 'unknown' && ignoreExportsUsedInFile[item.type]) ||
-      isType_
-    ) {
-      item.refs = findInternalReferences(item, sourceFile, typeChecker, referencedSymbolsInExportedTypes);
+    } else {
+      const isBindingElement = item.symbol?.valueDeclaration && ts.isBindingElement(item.symbol.valueDeclaration);
+      if (
+        ignoreExportsUsedInFile === true ||
+        (typeof ignoreExportsUsedInFile === 'object' &&
+          item.type !== 'unknown' &&
+          ignoreExportsUsedInFile[item.type]) ||
+        isType(item) ||
+        isBindingElement
+      ) {
+        item.refs = findInternalReferences(item, sourceFile, typeChecker, referencedSymbolsInExport, isBindingElement);
+      }
     }
 
     for (const member of item.members) {
-      member.refs = findInternalReferences(member, sourceFile, typeChecker, referencedSymbolsInExportedTypes);
+      member.refs = findInternalReferences(member, sourceFile, typeChecker, referencedSymbolsInExport);
       member.symbol = undefined;
     }
 

diff --git a/packages/knip/src/typescript/visitors/exports/exportKeyword.ts b/packages/knip/src/typescript/visitors/exports/exportKeyword.ts
@@ -37,6 +37,8 @@ export default visit(
                     : undefined;
                   return {
                     node: element,
+                    // @ts-expect-error We'll use the symbol in `findInternalReferences`
+                    symbol: element.symbol,
                     identifier: element.name.escapedText.toString(),
                     type: SymbolType.UNKNOWN,
                     pos: element.name.getStart(),
@@ -54,6 +56,8 @@ export default visit(
                   const fix = isFixExports ? [element.getStart(), element.getEnd(), FIX_FLAGS.NONE] : undefined;
                   return {
                     node: element,
+                    // @ts-expect-error We'll use the symbol in `findInternalReferences`
+                    symbol: element.symbol,
                     identifier: element.getText(),
                     type: SymbolType.UNKNOWN,
                     pos: element.getStart(),