diff --git a/packages/docs/src/content/docs/features/auto-fix.mdx b/packages/docs/src/content/docs/features/auto-fix.mdx index 1e1379a8d..86fd2c019 100644 --- a/packages/docs/src/content/docs/features/auto-fix.mdx +++ b/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 diff --git a/packages/knip/fixtures/fix/index.js b/packages/knip/fixtures/fix/index.js index a45551e22..60b2c9410 100644 --- a/packages/knip/fixtures/fix/index.js +++ b/packages/knip/fixtures/fix/index.js @@ -1,5 +1,5 @@ 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'; @@ -7,6 +7,9 @@ import * as NS from './reexports'; _; z; +f; +g; +i; USED; identifier; a; diff --git a/packages/knip/fixtures/fix/mod.ts b/packages/knip/fixtures/fix/mod.ts index db57bfeed..2262ef4b5 100644 --- a/packages/knip/fixtures/fix/mod.ts +++ b/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 {} diff --git a/packages/knip/fixtures/fix/reexported.js b/packages/knip/fixtures/fix/reexported.js deleted file mode 100644 index e0be4d41b..000000000 --- a/packages/knip/fixtures/fix/reexported.js +++ /dev/null @@ -1,6 +0,0 @@ -const Two = 2; -const Three = 2; - -export { Two, Three }; - -export const One = 1; diff --git a/packages/knip/fixtures/fix/reexported.ts b/packages/knip/fixtures/fix/reexported.ts new file mode 100644 index 000000000..e751b8b78 --- /dev/null +++ b/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; diff --git a/packages/knip/src/IssueFixer.ts b/packages/knip/src/IssueFixer.ts index e9d93779b..f38bd8f10 100644 --- a/packages/knip/src/IssueFixer.ts +++ b/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> = new Map(); - unusedExportNodes: Map> = new Map(); + unusedTypeNodes: Map> = new Map(); + unusedExportNodes: Map> = 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); } diff --git a/packages/knip/src/index.ts b/packages/knip/src/index.ts index 949f9f82a..1dea0e46b 100644 --- a/packages/knip/src/index.ts +++ b/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; diff --git a/packages/knip/src/types/exports.ts b/packages/knip/src/types/exports.ts index d61216c47..53f1e5545 100644 --- 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]; +type ExportPosTuple = [number, number, boolean]; export type Fix = ExportPosTuple | undefined; export type Fixes = Array; diff --git a/packages/knip/src/typescript/visitors/exports/exportAssignment.ts b/packages/knip/src/typescript/visitors/exports/exportAssignment.ts index 22981a6c1..d56b8e13a 100644 --- a/packages/knip/src/typescript/visitors/exports/exportAssignment.ts +++ b/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 }; diff --git a/packages/knip/src/typescript/visitors/exports/exportDeclaration.ts b/packages/knip/src/typescript/visitors/exports/exportDeclaration.ts index 1110972ca..a87f69d9f 100644 --- a/packages/knip/src/typescript/visitors/exports/exportDeclaration.ts +++ b/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 }; }); } diff --git a/packages/knip/src/typescript/visitors/exports/exportKeyword.ts b/packages/knip/src/typescript/visitors/exports/exportKeyword.ts index 7c724e57f..c298f831b 100644 --- a/packages/knip/src/typescript/visitors/exports/exportKeyword.ts +++ b/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) diff --git a/packages/knip/src/typescript/visitors/exports/exportsAccessExpression.ts b/packages/knip/src/typescript/visitors/exports/exportsAccessExpression.ts index e94d315d7..8374c28a5 100644 --- a/packages/knip/src/typescript/visitors/exports/exportsAccessExpression.ts +++ b/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, diff --git a/packages/knip/src/typescript/visitors/exports/moduleExportsAccessExpression.ts b/packages/knip/src/typescript/visitors/exports/moduleExportsAccessExpression.ts index edd59db81..5f4486a5b 100644 --- a/packages/knip/src/typescript/visitors/exports/moduleExportsAccessExpression.ts +++ b/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, diff --git a/packages/knip/src/util/clean-export.ts b/packages/knip/src/util/clean-export.ts new file mode 100644 index 000000000..34ab2abf1 --- /dev/null +++ b/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)); +}; diff --git a/packages/knip/test/fix.test.ts b/packages/knip/test/fix.test.ts index a49ead74f..c246b316e 100644 --- a/packages/knip/test/fix.test.ts +++ b/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 {} diff --git a/packages/knip/test/util/clean-export.test.ts b/packages/knip/test/util/clean-export.test.ts new file mode 100644 index 000000000..8920f8e4a --- /dev/null +++ b/packages/knip/test/util/clean-export.test.ts @@ -0,0 +1,55 @@ +import { test } from 'bun:test'; +import assert from 'node:assert/strict'; +import { cleanExport } from '../../src/util/clean-export.js'; + +const getOpts = (text: string, value: string) => { + const start = text.indexOf(value); + return { text, start, end: start + 2, isCleanable: true }; +}; + +test('fixer', () => { + { + const text = 'export { AB }'; + assert.deepEqual(cleanExport(getOpts(text, 'AB')), ''); + } + + { + const text = 'export { AB, CD }'; + assert.deepEqual(cleanExport(getOpts(text, 'AB')), 'export { CD }'); + } + + { + const text = 'export { AB, CD, EF }'; + assert.deepEqual(cleanExport(getOpts(text, 'CD')), 'export { AB, EF }'); + } + + { + const text = 'export { AB, CD } from "specifier"'; + assert.deepEqual(cleanExport(getOpts(text, 'CD')), 'export { AB, } from "specifier"'); + } + + { + const text = 'export { AB, CD, EF } from "specifier"'; + assert.deepEqual(cleanExport(getOpts(text, 'CD')), 'export { AB, EF } from "specifier"'); + } + + { + const text = 'export { AB } from "specifier"'; + assert.deepEqual(cleanExport(getOpts(text, 'AB')), ''); + } + + { + const text = "export { AB } from './specifier'"; + assert.deepEqual(cleanExport(getOpts(text, 'AB')), ''); + } + + { + const text = 'export{AB}from"specifier"'; + assert.deepEqual(cleanExport(getOpts(text, 'AB')), ''); + } + + { + const text = 'export { AB } from "specifier"'; + assert.deepEqual(cleanExport(getOpts(text, 'AB')), ''); + } +});