Improve unused export fixer, add tests

packages/docs/src/content/docs/features/auto-fix.mdx

@@ -110,6 +110,18 @@ The `export` keyword for unused exports is removed:
 
 The `default` keyword was also removed here.
 
+Knip cleans up the whole or part of export declarations:
+
+```diff title="file.js"
+const Snake = 'snake';
+const Owl = 'owl';
+const Hawk = 'tuah';
+-export { Snake, Owl };
+-export { Hawk };
++;
++;
+```
+
 Knip cleans up the whole or part of re-exports:
 
 ```diff title="file.js"
@@ -118,7 +130,7 @@ Knip cleans up the whole or part of re-exports:
 +export { Elephant } from './jungle'
 ```
 
-Sometimes lines can be removed completely:
+Lines like the following are removed completely:
 
 ```diff title="file.js"
 -module.exports.UNUSED = 1;
@@ -144,18 +156,22 @@ Unused dependencies are removed from `package.json`:
  }
 ```
 
+Knip does not remove empty export declaration bindings, it only removes the
+individual exported items:
+
+```diff title="file.js"
+-export const { a, b  } = { a, b };
++export const { ,  } = { a, b };
+
+-export const [c, d] = [c, d];
++export const [, ] = [c, d];
+```
+
 ## What's not included
 
 Operations that auto-fix does not yet perform include:
 
 - Add unlisted (dev) dependencies to `package.json`
 - Remove unused class and enum members
-- Remove empty export declarations for less common cases, e.g.:
-
-```ts
-export const { ,  } = { a, b };
-
-export const [, ] = [c, d];
-```
 
 [1]: ../reference/issue-types.md

packages/knip/fixtures/fix/index.js

@@ -1,12 +1,15 @@
 import _ from 'lodash';
-import { z } from './mod';
+import { z, f, g, i } from './mod';
 import { USED } from './access';
 import { identifier } from './exports';
 import { a } from './ignored';
 import * as NS from './reexports';
 
 _;
 z;
+f;
+g;
+i;
 USED;
 identifier;
 a;

packages/knip/fixtures/fix/mod.ts

@@ -9,7 +9,11 @@ export const z = x + y;
 
 export const { a, b } = { a: 1, b: 1 };
 
-export const [c, d] = [1, 2];
+export const [c, d] = [3, 4];
+
+export const [e, f] = [5, 6];
+
+export const [g, h, i] = [7, 8, 9];
 
 export default class MyClass {}
 

packages/knip/fixtures/fix/reexported.ts

@@ -0,0 +1,10 @@
+const Two = 2;
+const Three = 3;
+const Four = 4;
+const Five = 5;
+
+export { Two, Three };
+
+export { Four as Fourth, Five as Fifth };
+
+export const One = 1;

packages/knip/src/IssueFixer.ts

@@ -1,6 +1,7 @@
 import { readFile, rm, writeFile } from 'node:fs/promises';
 import type { Fixes } from './types/exports.js';
 import type { Issues } from './types/issues.js';
+import { cleanExport } from './util/clean-export.js';
 import { load, save } from './util/package-json.js';
 import { join, relative } from './util/path.js';
 
@@ -19,8 +20,8 @@ export class IssueFixer {
   isFixUnusedTypes = true;
   isFixUnusedExports = true;
 
-  unusedTypeNodes: Map<string, Set<[number, number]>> = new Map();
-  unusedExportNodes: Map<string, Set<[number, number]>> = new Map();
+  unusedTypeNodes: Map<string, Set<[number, number, boolean]>> = new Map();
+  unusedExportNodes: Map<string, Set<[number, number, boolean]>> = new Map();
 
   constructor({ isEnabled, cwd, fixTypes = [], isRemoveFiles }: Fixer) {
     this.isEnabled = isEnabled;
@@ -83,15 +84,13 @@ export class IssueFixer {
 
       if (exportPositions.length > 0) {
         const sourceFileText = exportPositions.reduce(
-          (text, [start, end]) => text.substring(0, start) + text.substring(end),
+          (text, [start, end, isCleanable]) => {
+            return cleanExport({ text, start, end, isCleanable: Boolean(isCleanable) });
+          },
           await readFile(filePath, 'utf-8')
         );
 
-        const withoutEmptyReExports = sourceFileText
-          .replaceAll(/export \{[ ,]+\} from ('|")[^'"]+('|");?\r?\n?/g, '')
-          .replaceAll(/export \{[ ,]+\};?\r?\n?/g, '');
-
-        await writeFile(filePath, withoutEmptyReExports);
+        await writeFile(filePath, sourceFileText);
 
         this.markExportFixed(issues, filePath);
       }

packages/knip/src/index.ts

@@ -395,7 +395,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
           const importsForExport = file.imported;
 
           for (const [identifier, exportedItem] of exportItems.entries()) {
-            if (exportedItem.isReExport) continue;
+            if (!isFix && exportedItem.isReExport) continue;
 
             // Skip tagged exports
             if (shouldIgnore(exportedItem.jsDocTags)) continue;

packages/knip/src/types/exports.ts

@@ -3,7 +3,7 @@ import type { SymbolType } from './issues.js';
 
 type Identifier = string;
 
-type ExportPosTuple = [number, number];
+type ExportPosTuple = [number, number, boolean];
 export type Fix = ExportPosTuple | undefined;
 export type Fixes = Array<ExportPosTuple>;
 

packages/knip/src/typescript/visitors/exports/exportAssignment.ts

@@ -11,7 +11,7 @@ export default visit(
       // export default 1;
       // export = identifier;
       const pos = node.getChildAt(1).getStart();
-      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd() + 1] : undefined;
+      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd() + 1, false] : undefined;
       // @ts-expect-error We need the symbol in `addExport`
       const symbol = node.getSourceFile().locals?.get(node.expression.escapedText);
       return { node, symbol, identifier: 'default', type: SymbolType.UNKNOWN, pos, fix };

packages/knip/src/typescript/visitors/exports/exportDeclaration.ts

@@ -22,7 +22,7 @@ export default visit(
           // const symbol = element.symbol ?? declarations?.get(identifier)?.find((d: ts.Node) => d !== element)?.symbol;
           const symbol = declarations?.get(propName ?? identifier)?.[0]?.symbol;
           const pos = element.name.pos;
-          const fix: Fix = isFixExports || isFixTypes ? [element.getStart(), element.getEnd()] : undefined;
+          const fix: Fix = isFixExports || isFixTypes ? [element.getStart(), element.getEnd(), true] : undefined;
           return { node: element, symbol, identifier, type, pos, fix };
         });
       }

packages/knip/src/typescript/visitors/exports/exportKeyword.ts

@@ -18,9 +18,10 @@ export default visit(
 
     if (exportKeyword) {
       const getFix = (node: ts.Node, defaultKeyword?: ts.Node): Fix =>
-        isFixExports ? [node.getStart(), (defaultKeyword ?? node).getEnd() + 1] : undefined;
-      const getElementFix = (node: ts.Node): Fix => (isFixExports ? [node.getStart(), node.getEnd()] : undefined);
-      const getTypeFix = (node: ts.Node): Fix => (isFixTypes ? [node.getStart(), node.getEnd() + 1] : undefined);
+        isFixExports ? [node.getStart(), (defaultKeyword ?? node).getEnd() + 1, false] : undefined;
+      const getElementFix = (node: ts.Node): Fix =>
+        isFixExports ? [node.getStart(), node.getEnd(), false] : undefined;
+      const getTypeFix = (node: ts.Node): Fix => (isFixTypes ? [node.getStart(), node.getEnd() + 1, false] : undefined);
 
       if (ts.isVariableStatement(node)) {
         // @ts-expect-error TODO Issue seems caused by mismatch between returned `node` types (but all ts.Node)

packages/knip/src/typescript/visitors/exports/exportsAccessExpression.ts

@@ -10,7 +10,7 @@ export default visit(isJS, (node, { isFixExports }) => {
       // Pattern: exports.NAME
       const identifier = node.left.name.getText();
       const pos = node.left.name.pos;
-      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd()] : undefined;
+      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), false] : undefined;
       return {
         node: node.left.name,
         identifier,

packages/knip/src/typescript/visitors/exports/moduleExportsAccessExpression.ts

@@ -16,7 +16,7 @@ export default visit(isJS, (node, { isFixExports }) => {
           // Pattern: module.exports.NAME
           const identifier = node.expression.left.name.getText();
           const pos = node.expression.left.name.pos;
-          const fix: Fix = isFixExports ? [node.getStart(), node.getEnd()] : undefined;
+          const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), false] : undefined;
           return {
             node: node.expression.left.name,
             identifier,
@@ -30,7 +30,7 @@ export default visit(isJS, (node, { isFixExports }) => {
           if (ts.isObjectLiteralExpression(expr) && expr.properties.every(ts.isShorthandPropertyAssignment)) {
             // Pattern: module.exports = { identifier, identifier2 }
             return expr.properties.map(node => {
-              const fix: Fix = isFixExports ? [node.getStart(), node.getEnd()] : undefined;
+              const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), true] : undefined;
               return { node, identifier: node.getText(), type: SymbolType.UNKNOWN, pos: node.getStart(), fix };
             });
           }
@@ -52,7 +52,7 @@ export default visit(isJS, (node, { isFixExports }) => {
         // Pattern: module.exports['NAME']
         const identifier = stripQuotes(node.expression.left.argumentExpression.getText());
         const pos = node.expression.left.argumentExpression.pos;
-        const fix: Fix = isFixExports ? [node.getStart(), node.getEnd()] : undefined;
+        const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), false] : undefined;
         return {
           node: node.expression.left.argumentExpression,
           identifier,

packages/knip/src/util/clean-export.ts

@@ -0,0 +1,64 @@
+interface FixerOptions {
+  text: string;
+  start: number;
+  end: number;
+  isCleanable: boolean;
+}
+
+export const cleanExport = ({ text, start, end, isCleanable }: FixerOptions) => {
+  const beforeStart = text.substring(0, start);
+  const afterEnd = text.substring(end);
+  const exportKeyword = text.substring(start, end).trim();
+
+  if (exportKeyword === 'export' || exportKeyword === 'export default') return beforeStart + afterEnd;
+  if (!isCleanable) return beforeStart + afterEnd;
+
+  let bracketOpenIndex = -1;
+  let bracketCloseIndex = -1;
+  let commaIndex = -1;
+
+  let i = 0;
+  while (i <= afterEnd.length) {
+    const char = afterEnd[i];
+    if (char === ',') {
+      commaIndex = i;
+    } else if (char === '}') {
+      bracketCloseIndex = i;
+      break;
+    } else if (!/\s/.test(char)) break;
+    i++;
+  }
+
+  if (bracketCloseIndex === -1) {
+    return beforeStart + (commaIndex === -1 ? afterEnd : afterEnd.substring(commaIndex + 1));
+  }
+
+  let j = beforeStart.length - 1;
+  while (j >= 0) {
+    const char = beforeStart[j];
+    if (char === '{') {
+      bracketOpenIndex = j;
+      break;
+    }
+    if (!/\s/.test(char)) break;
+    j--;
+  }
+
+  if (bracketCloseIndex !== -1 && bracketOpenIndex !== -1) {
+    const toBracket = beforeStart.substring(0, bracketOpenIndex).trim();
+    if (toBracket.endsWith('export')) {
+      const fromBracket = afterEnd.substring(bracketCloseIndex + 1).trim();
+      if (fromBracket.startsWith('from')) {
+        const quoteMatch = afterEnd.match(/['"].*?['"]/);
+        if (quoteMatch?.index) {
+          const fromSpecifierLength = quoteMatch.index + quoteMatch[0].length;
+          return toBracket.substring(0, toBracket.length - 6) + afterEnd.substring(fromSpecifierLength);
+        }
+      }
+
+      return toBracket.substring(0, toBracket.length - 6) + afterEnd.substring(bracketCloseIndex + 1);
+    }
+  }
+
+  return beforeStart + (commaIndex === -1 ? afterEnd : afterEnd.substring(commaIndex + 1));
+};

packages/knip/test/fix.test.ts

@@ -25,7 +25,11 @@ export const z = x + y;
 
 export const { ,  } = { a: 1, b: 1 };
 
-export const [, ] = [1, 2];
+export const [, ] = [3, 4];
+
+export const [, f] = [5, 6];
+
+export const [g, , i] = [7, 8, 9];
 
 class MyClass {}
 
@@ -53,19 +57,24 @@ module.exports = { identifier,  };
     [
       'reexports.js',
       await readContents('reexports.js'),
-      `export { RangeSlider } from './reexported';
-export { Rating } from './reexported';
+      `;
+;
 export { One } from './reexported';
-export { Col, Col as KCol } from './reexported';
-export { Row as KRow, Row } from './reexported';
+;
+;
 `,
     ],
     [
-      'reexported.js',
-      await readContents('reexported.js'),
+      'reexported.ts',
+      await readContents('reexported.ts'),
       `const Two = 2;
-const Three = 2;
+const Three = 3;
+const Four = 4;
+const Five = 5;
+
+;
 
+;
 
 export const One = 1;
 `,
@@ -102,8 +111,8 @@ export const One = 1;
   assert(issues.exports['mod.ts']['default']);
   assert(issues.exports['mod.ts']['x']);
   assert(issues.exports['mod.ts']['y']);
-  assert(issues.exports['reexported.js']['Three']);
-  assert(issues.exports['reexported.js']['Two']);
+  assert(issues.exports['reexported.ts']['Three']);
+  assert(issues.exports['reexported.ts']['Two']);
 
   // check ignore
   assert(issues.exports['ignored.ts'] === undefined);
@@ -138,7 +147,11 @@ export const z = x + y;
 
 export const { a, b } = { a: 1, b: 1 };
 
-export const [c, d] = [1, 2];
+export const [c, d] = [3, 4];
+
+export const [e, f] = [5, 6];
+
+export const [g, h, i] = [7, 8, 9];
 
 export default class MyClass {}