Refactor for reuse/readabilty, more tests

webpro-nl · Oct 2, 2024 · 20c2390 · 20c2390
1 parent 0f786f4
commit 20c2390
Show file tree

Hide file tree

Showing 18 changed files with 333 additions and 206 deletions.
diff --git a/packages/knip/fixtures/fix/default-x.js b/packages/knip/fixtures/fix/default-x.js
@@ -0,0 +1,5 @@
+const x = 1;
+
+export default x;
+
+export const dx = 1;
diff --git a/packages/knip/fixtures/fix/default.js b/packages/knip/fixtures/fix/default.js
@@ -0,0 +1,2 @@
+export default 1;
+export const d = 1;
diff --git a/packages/knip/fixtures/fix/index.js b/packages/knip/fixtures/fix/index.js
@@ -1,10 +1,14 @@
+import { d } from './default';
+import { dx } from './default-x';
 import _ from 'lodash';
 import { z, f, g, i } from './mod';
 import { USED } from './access';
 import { identifier } from './exports';
 import { a } from './ignored';
 import * as NS from './reexports';
 
+d;
+dx;
 _;
 z;
 f;
@@ -14,4 +18,6 @@ USED;
 identifier;
 a;
 NS.One;
+NS.Rectangle;
 NS.Nine;
+NS.setter;
diff --git a/packages/knip/fixtures/fix/reexported.ts b/packages/knip/fixtures/fix/reexported.ts
@@ -7,13 +7,20 @@ export { Two, Three };
 
 export { Four as Fourth, Five as Fifth };
 
+export { Four as Rectangle, Five as Pentagon };
+
 type Six = any;
 type Seven = unknown;
 const Eight = 8;
 const Nine = 9;
 type Ten = unknown[];
-;
+
+export type { Six };
 
 export { type Seven, Eight, Nine, type Ten };
 
 export const One = 1;
+
+const fn = () => ({ get: () => 1, set: () => 1 });
+
+export const { get: getter, set: setter } = fn();
diff --git a/packages/knip/fixtures/fix/reexports.js b/packages/knip/fixtures/fix/reexports.js
@@ -1,5 +1,5 @@
 export { RangeSlider } from './reexported';
 export { Rating } from './reexported';
-export { One, Six, Seven, Eight, Nine, Ten } from './reexported';
+export { One, Rectangle, Six, Seven, Eight, Nine, Ten, getter, setter } from './reexported';
 export { Col, Col as KCol } from './reexported';
 export { Row as KRow, Row } from './reexported';
diff --git a/packages/knip/src/IssueFixer.ts b/packages/knip/src/IssueFixer.ts
@@ -1,9 +1,9 @@
 import { readFile, rm, writeFile } from 'node:fs/promises';
 import type { Fix, 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';
+import { removeExport } from './util/remove-export.js';
 
 interface Fixer {
   isEnabled: boolean;
@@ -46,7 +46,7 @@ export class IssueFixer {
 
   public async fixIssues(issues: Issues) {
     await this.removeUnusedFiles(issues);
-    await this.removeUnusedExportKeywords(issues);
+    await this.removeUnusedExports(issues);
     await this.removeUnusedDependencies(issues);
   }
 
@@ -74,7 +74,7 @@ export class IssueFixer {
     }
   }
 
-  private async removeUnusedExportKeywords(issues: Issues) {
+  private async removeUnusedExports(issues: Issues) {
     const filePaths = new Set([...this.unusedTypeNodes.keys(), ...this.unusedExportNodes.keys()]);
     for (const filePath of filePaths) {
       const types = (this.isFixUnusedTypes && this.unusedTypeNodes.get(filePath)) || [];
@@ -83,7 +83,7 @@ export class IssueFixer {
 
       if (exportPositions.length > 0) {
         const sourceFileText = exportPositions.reduce(
-          (text, [start, end, isCleanable]) => cleanExport({ text, start, end, isCleanable: Boolean(isCleanable) }),
+          (text, [start, end, flags]) => removeExport({ text, start, end, flags }),
           await readFile(filePath, 'utf-8')
         );
 

diff --git a/packages/knip/src/constants.ts b/packages/knip/src/constants.ts
@@ -202,3 +202,9 @@ export const ISSUE_TYPE_TITLE: Record<IssueType, string> = {
   classMembers: 'Unused exported class members',
   duplicates: 'Duplicate exports',
 };
+
+export const FIX_FLAGS = {
+  NONE: 0,
+  OBJECT_BINDING: 1 << 0, // remove next comma
+  EMPTY_DECLARATION: 1 << 1, // remove declaration if empty
+} as const;
diff --git a/packages/knip/src/types/exports.ts b/packages/knip/src/types/exports.ts
@@ -3,7 +3,7 @@ import type { SymbolType } from './issues.js';
 
 type Identifier = string;
 
-type ExportPosTuple = [number, number, boolean];
+type ExportPosTuple = [number, number, number];
 export type Fix = ExportPosTuple | undefined;
 export type Fixes = Array<ExportPosTuple>;
 

diff --git a/packages/knip/src/typescript/visitors/exports/exportAssignment.ts b/packages/knip/src/typescript/visitors/exports/exportAssignment.ts
@@ -1,4 +1,5 @@
 import ts from 'typescript';
+import { FIX_FLAGS } from '../../../constants.js';
 import type { Fix } from '../../../types/exports.js';
 import { SymbolType } from '../../../types/issues.js';
 import { exportVisitor as visit } from '../index.js';
@@ -11,7 +12,7 @@ export default visit(
       // export default 1;
       // export = identifier;
       const pos = node.getChildAt(1).getStart();
-      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd() + 1, false] : undefined;
+      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd() + 1, FIX_FLAGS.NONE] : 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 };

diff --git a/packages/knip/src/typescript/visitors/exports/exportDeclaration.ts b/packages/knip/src/typescript/visitors/exports/exportDeclaration.ts
@@ -1,4 +1,5 @@
 import ts from 'typescript';
+import { FIX_FLAGS } from '../../../constants.js';
 import type { Fix } from '../../../types/exports.js';
 import { SymbolType } from '../../../types/issues.js';
 import type { BoundSourceFile } from '../../SourceFile.js';
@@ -25,7 +26,7 @@ export default visit(
           const type = element.isTypeOnly ? SymbolType.TYPE : nodeType;
           const fix: Fix =
             (isFixExports && type !== SymbolType.TYPE) || (isFixTypes && type === SymbolType.TYPE)
-              ? [element.getStart(), element.getEnd(), true]
+              ? [element.getStart(), element.getEnd(), FIX_FLAGS.OBJECT_BINDING | FIX_FLAGS.EMPTY_DECLARATION]
               : undefined;
           return { node: element, symbol, identifier, type, pos, fix };
         });

diff --git a/packages/knip/src/typescript/visitors/exports/exportKeyword.ts b/packages/knip/src/typescript/visitors/exports/exportKeyword.ts
@@ -1,4 +1,5 @@
 import ts from 'typescript';
+import { FIX_FLAGS } from '../../../constants.js';
 import type { Fix } from '../../../types/exports.js';
 import { SymbolType } from '../../../types/issues.js';
 import { compact } from '../../../util/array.js';
@@ -18,10 +19,10 @@ export default visit(
 
     if (exportKeyword) {
       const getFix = (node: ts.Node, defaultKeyword?: ts.Node): Fix =>
-        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);
+        isFixExports ? [node.getStart(), (defaultKeyword ?? node).getEnd() + 1, FIX_FLAGS.NONE] : undefined;
+
+      const getTypeFix = (node: ts.Node): Fix =>
+        isFixTypes ? [node.getStart(), node.getEnd() + 1, FIX_FLAGS.NONE] : undefined;
 
       if (ts.isVariableStatement(node)) {
         // @ts-expect-error TODO Issue seems caused by mismatch between returned `node` types (but all ts.Node)
@@ -31,7 +32,9 @@ export default visit(
             return compact(
               declaration.name.elements.map(element => {
                 if (ts.isIdentifier(element.name)) {
-                  const fix = getElementFix(element);
+                  const fix = isFixExports
+                    ? [element.getStart(), element.getEnd(), FIX_FLAGS.OBJECT_BINDING]
+                    : undefined;
                   return {
                     node: element,
                     identifier: element.name.escapedText.toString(),
@@ -48,7 +51,7 @@ export default visit(
             return compact(
               declaration.name.elements.map(element => {
                 if (ts.isBindingElement(element)) {
-                  const fix = getElementFix(element);
+                  const fix = isFixExports ? [element.getStart(), element.getEnd(), FIX_FLAGS.NONE] : undefined;
                   return {
                     node: element,
                     identifier: element.getText(),

diff --git a/packages/knip/src/typescript/visitors/exports/exportsAccessExpression.ts b/packages/knip/src/typescript/visitors/exports/exportsAccessExpression.ts
@@ -1,4 +1,5 @@
 import ts from 'typescript';
+import { FIX_FLAGS } from '../../../constants.js';
 import type { Fix } from '../../../types/exports.js';
 import { SymbolType } from '../../../types/issues.js';
 import { isJS } from '../helpers.js';
@@ -10,7 +11,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(), false] : undefined;
+      const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), FIX_FLAGS.NONE] : undefined;
       return {
         node: node.left.name,
         identifier,

diff --git a/packages/knip/src/typescript/visitors/exports/moduleExportsAccessExpression.ts b/packages/knip/src/typescript/visitors/exports/moduleExportsAccessExpression.ts
@@ -1,4 +1,5 @@
 import ts from 'typescript';
+import { FIX_FLAGS } from '../../../constants.js';
 import type { Fix } from '../../../types/exports.js';
 import { SymbolType } from '../../../types/issues.js';
 import { hasRequireCall, isModuleExportsAccess, stripQuotes } from '../../ast-helpers.js';
@@ -16,7 +17,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(), false] : undefined;
+          const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), FIX_FLAGS.NONE] : undefined;
           return {
             node: node.expression.left.name,
             identifier,
@@ -30,7 +31,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(), true] : undefined;
+              const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), FIX_FLAGS.NONE] : undefined;
               return { node, identifier: node.getText(), type: SymbolType.UNKNOWN, pos: node.getStart(), fix };
             });
           }
@@ -52,7 +53,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(), false] : undefined;
+        const fix: Fix = isFixExports ? [node.getStart(), node.getEnd(), FIX_FLAGS.NONE] : undefined;
         return {
           node: node.expression.left.argumentExpression,
           identifier,

diff --git a/packages/knip/src/util/clean-export.ts b/packages/knip/src/util/clean-export.ts