From 1109cbf67de740cb1eb21e9eea52b80b48329c65 Mon Sep 17 00:00:00 2001 From: Lars Kappert Date: Thu, 26 Sep 2024 22:46:47 +0200 Subject: [PATCH] Add support for enum and class members --- packages/knip/fixtures/fix-members/class.ts | 20 +++++ packages/knip/fixtures/fix-members/enums.ts | 11 +++ packages/knip/fixtures/fix-members/index.js | 9 +++ .../knip/fixtures/fix-members/package.json | 3 + packages/knip/src/IssueFixer.ts | 2 +- packages/knip/src/index.ts | 8 +- .../visitors/exports/exportKeyword.ts | 4 +- packages/knip/src/util/remove-export.ts | 4 +- packages/knip/test/fix-members.test.ts | 80 +++++++++++++++++++ packages/knip/test/util/remove-export.test.ts | 17 ++++ 10 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 packages/knip/fixtures/fix-members/class.ts create mode 100644 packages/knip/fixtures/fix-members/enums.ts create mode 100644 packages/knip/fixtures/fix-members/index.js create mode 100644 packages/knip/fixtures/fix-members/package.json create mode 100644 packages/knip/test/fix-members.test.ts diff --git a/packages/knip/fixtures/fix-members/class.ts b/packages/knip/fixtures/fix-members/class.ts new file mode 100644 index 000000000..d09b8ad5e --- /dev/null +++ b/packages/knip/fixtures/fix-members/class.ts @@ -0,0 +1,20 @@ +export class Rectangle { + constructor( + public width: number, + public height: number + ) {} + + static Key = 1; + + public get unusedGetter(): string { + return 'unusedGetter'; + } + + private set unusedSetter(w: number) { + this.width = w; + } + + area() { + return this.width * this.height; + } +} diff --git a/packages/knip/fixtures/fix-members/enums.ts b/packages/knip/fixtures/fix-members/enums.ts new file mode 100644 index 000000000..833db90e9 --- /dev/null +++ b/packages/knip/fixtures/fix-members/enums.ts @@ -0,0 +1,11 @@ +export enum Directions { + North = 1, + East = 2, + South = 3, + West = 4, +} + +export enum Fruits { + apple = 'apple', + orange = 'orange', +} diff --git a/packages/knip/fixtures/fix-members/index.js b/packages/knip/fixtures/fix-members/index.js new file mode 100644 index 000000000..7b88240cb --- /dev/null +++ b/packages/knip/fixtures/fix-members/index.js @@ -0,0 +1,9 @@ +import { Rectangle } from './class.js'; +import { Directions, Fruits } from './enums.js'; + +Directions.East; + +Fruits.apple; + +const rect = new Rectangle(); +rect.area(); diff --git a/packages/knip/fixtures/fix-members/package.json b/packages/knip/fixtures/fix-members/package.json new file mode 100644 index 000000000..a94e8ccdf --- /dev/null +++ b/packages/knip/fixtures/fix-members/package.json @@ -0,0 +1,3 @@ +{ + "name": "@fixtures/fix-members" +} diff --git a/packages/knip/src/IssueFixer.ts b/packages/knip/src/IssueFixer.ts index 1638ab315..8c4996a62 100644 --- a/packages/knip/src/IssueFixer.ts +++ b/packages/knip/src/IssueFixer.ts @@ -54,7 +54,7 @@ export class IssueFixer { const relPath = relative(filePath); const types = [ - ...(this.isFixUnusedTypes ? (['types', 'nsTypes'] as const) : []), + ...(this.isFixUnusedTypes ? (['types', 'nsTypes', 'classMembers', 'enumMembers'] as const) : []), ...(this.isFixUnusedExports ? (['exports', 'nsExports'] as const) : []), ]; diff --git a/packages/knip/src/index.ts b/packages/knip/src/index.ts index 1dea0e46b..6d68e44e4 100644 --- a/packages/knip/src/index.ts +++ b/packages/knip/src/index.ts @@ -445,7 +445,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { if (!isReferenced) { if (isIgnored) continue; - collector.addIssue({ + const isIssueAdded = collector.addIssue({ type: 'enumMembers', filePath, workspace: workspace.name, @@ -455,6 +455,8 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { line: member.line, col: member.col, }); + + if (isFix && isIssueAdded && member.fix) fixer.addUnusedTypeNode(filePath, [member.fix]); } else if (isIgnored) { for (const tagName of exportedItem.jsDocTags) { if (tags[1].includes(tagName.replace(/^\@/, ''))) { @@ -481,7 +483,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { continue; } - collector.addIssue({ + const isIssueAdded = collector.addIssue({ type: 'classMembers', filePath, workspace: workspace.name, @@ -491,6 +493,8 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => { line: member.line, col: member.col, }); + + if (isFix && isIssueAdded && member.fix) fixer.addUnusedTypeNode(filePath, [member.fix]); } } diff --git a/packages/knip/src/typescript/visitors/exports/exportKeyword.ts b/packages/knip/src/typescript/visitors/exports/exportKeyword.ts index db8b1c62c..05f708033 100644 --- a/packages/knip/src/typescript/visitors/exports/exportKeyword.ts +++ b/packages/knip/src/typescript/visitors/exports/exportKeyword.ts @@ -100,7 +100,7 @@ export default visit( // Naive, but [does.the.job()] pos: member.name.getStart() + (ts.isComputedPropertyName(member.name) ? 1 : 0), type: SymbolType.MEMBER, - fix: undefined, + fix: isFixTypes ? ([member.getStart(), member.getEnd(), FIX_FLAGS.NONE] as Fix) : undefined, })) : []; @@ -130,7 +130,7 @@ export default visit( identifier: stripQuotes(member.name.getText()), pos: member.name.getStart(), type: SymbolType.MEMBER, - fix: undefined, + fix: isFixTypes ? ([member.getStart(), member.getEnd(), FIX_FLAGS.OBJECT_BINDING] as Fix) : undefined, })); return { node, identifier, type: SymbolType.ENUM, pos, members, fix }; diff --git a/packages/knip/src/util/remove-export.ts b/packages/knip/src/util/remove-export.ts index d213c59ce..5f73a72c3 100644 --- a/packages/knip/src/util/remove-export.ts +++ b/packages/knip/src/util/remove-export.ts @@ -34,8 +34,8 @@ export const removeExport = ({ text, start, end, flags }: FixerOptions) => { if (flags % FIX_FLAGS.NONE) return beforeStart + afterEnd; - const exportKeyword = text.substring(start, end).trim(); - if (exportKeyword === 'export' || exportKeyword === 'export default') return beforeStart + afterEnd; + const subject = text.substring(start, end).trim(); + if (subject === 'export' || subject === 'export default') return beforeStart + afterEnd; let closingBracketOffset = -1; let commaOffset = -1; diff --git a/packages/knip/test/fix-members.test.ts b/packages/knip/test/fix-members.test.ts new file mode 100644 index 000000000..38fc9112e --- /dev/null +++ b/packages/knip/test/fix-members.test.ts @@ -0,0 +1,80 @@ +import { test } from 'bun:test'; +import assert from 'node:assert/strict'; +import { readFile, writeFile } from 'node:fs/promises'; +import { main } from '../src/index.js'; +import { join, resolve } from '../src/util/path.js'; +import baseArguments from './helpers/baseArguments.js'; + +const cwd = resolve('fixtures/fix-members'); + +const readContents = async (fileName: string) => await readFile(join(cwd, fileName), 'utf8'); + +test('Remove exports and dependencies', async () => { + const tests = [ + [ + 'class.ts', + await readContents('class.ts'), + `export class Rectangle { + constructor( + public width: number, + public height: number + ) {} + +` + + ' \n\n ' + + ` + + private set unusedSetter(w: number) { + this.width = w; + } + + area() { + return this.width * this.height; + } +} +`, + ], + [ + 'enums.ts', + await readContents('enums.ts'), + `export enum Directions { +` + + ' ' + + ` + East = 2, +` + + ' \n ' + + ` +} + +export enum Fruits { + apple = 'apple', +` + + ' ' + + ` +} +`, + ], + ]; + + const { issues } = await main({ + ...baseArguments, + cwd, + includedIssueTypes: ['classMembers'], + isFix: true, + }); + + assert(issues.enumMembers['enums.ts']['orange']); + assert(issues.enumMembers['enums.ts']['North']); + assert(issues.enumMembers['enums.ts']['South']); + assert(issues.enumMembers['enums.ts']['West']); + assert(issues.classMembers['class.ts']['Key']); + assert(issues.classMembers['class.ts']['unusedGetter']); + + for (const [fileName, before, after] of tests) { + const filePath = join(cwd, fileName); + const originalFile = await readFile(filePath); + assert.equal(String(originalFile), after); + await writeFile(filePath, before); + } +}); diff --git a/packages/knip/test/util/remove-export.test.ts b/packages/knip/test/util/remove-export.test.ts index bc8d823f7..23113951c 100644 --- a/packages/knip/test/util/remove-export.test.ts +++ b/packages/knip/test/util/remove-export.test.ts @@ -47,6 +47,23 @@ test('Clean export (FIX_FLAGS.OBJECT_BINDING)', () => { } }); +test('Clean export (FIX_FLAGS.OBJECT_BINDING)', () => { + { + const text = 'export enum E { AB = 1, CD = 2 }'; + assert.deepEqual(removeExport(getOpts(text, 'CD = 2', 1)), 'export enum E { AB = 1, }'); + } + + { + const text = "export enum E { AB = 'AB', CD = 'CD', }"; + assert.deepEqual(removeExport(getOpts(text, "AB = 'AB'", 1)), "export enum E { CD = 'CD', }"); + } + + { + const text = 'export const { AB: A_B, CD: C_D } = fn();'; + assert.deepEqual(removeExport(getOpts(text, 'AB: A_B', 1)), 'export const { CD: C_D } = fn();'); + } +}); + test('Clean export (FIX_FLAGS.EMPTY_DECLARATION)', () => { { const text = 'export { AB }';