diff --git a/lib/util/edit.test.ts b/lib/util/edit.test.ts index bc0c9fb..212903f 100644 --- a/lib/util/edit.test.ts +++ b/lib/util/edit.test.ts @@ -1278,7 +1278,7 @@ export {};\n`, describe('namespace import', () => { it('should not remove export for namespace import if its used in some other file', async () => { const fileService = new MemoryFileService(); - fileService.set('/app/main.ts', `import * as a from './a';`); + fileService.set('/app/main.ts', `import * as a from './a'; a.a;`); fileService.set('/app/a.ts', `export const a = 'a';`); await edit({ @@ -1289,6 +1289,26 @@ export {};\n`, assert.equal(fileService.get('/app/a.ts'), `export const a = 'a';`); }); + + it('should remove export used with namespace import even when some exports are used', async () => { + const fileService = new MemoryFileService(); + fileService.set('/app/main.ts', `import * as a from './a'; a.a;`); + fileService.set( + '/app/a.ts', + `export const a = 'a'; export const b = 'b';`, + ); + + await edit({ + fileService, + recursive, + entrypoints: ['/app/main.ts'], + }); + + assert.equal( + fileService.get('/app/a.ts'), + `export const a = 'a'; const b = 'b';`, + ); + }); }); describe('locally used declaration but not used in any other file', () => { diff --git a/lib/util/namespaceUsage.test.ts b/lib/util/namespaceUsage.test.ts new file mode 100644 index 0000000..5a4b612 --- /dev/null +++ b/lib/util/namespaceUsage.test.ts @@ -0,0 +1,119 @@ +import { describe, it } from 'node:test'; +import { namespaceUsage } from './namespaceUsage.js'; +import ts from 'typescript'; +import assert from 'node:assert/strict'; + +describe('namespaceUsage', () => { + it('should return namespace usage for a simple file', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +b.x;`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['x']); + }); + + it('should return multiple namespace usages', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +b.x; +b.y;`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['x', 'y']); + }); + + it('should return asterisk if the namespace identifier is used', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +b; +b.x;`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['*']); + }); + + it('should work with function calls on properties', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +b.x(); +b.y.z();`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['x', 'y']); + }); + + it('should return an asterisk when the namespace is assigned to a variable', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +const c = b; +c.x;`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['*']); + }); + + it('should return the correct results when there is a symbol with the same name', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +export function f() { + const b = { y: 1 }; + b.y; +} + +b.x;`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['x']); + }); + + it('should return an empty array when the namespace is not used', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +const c = 1;`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), []); + }); + + it('should return asterisk when the namespace is used in a object shorthand', () => { + const sourceFile = ts.createSourceFile( + '/app/a.ts', + `import * as b from './b'; +const c = { b };`, + ts.ScriptTarget.ESNext, + ); + + const result = namespaceUsage({ sourceFile }); + + assert.deepEqual(result.get('b'), ['*']); + }); +}); diff --git a/lib/util/namespaceUsage.ts b/lib/util/namespaceUsage.ts new file mode 100644 index 0000000..14d795f --- /dev/null +++ b/lib/util/namespaceUsage.ts @@ -0,0 +1,90 @@ +import ts from 'typescript'; +import { memoize } from './memoize.js'; + +const fn = ({ sourceFile }: { sourceFile: ts.SourceFile }) => { + const program = createProgram({ sourceFile }); + const checker = program.getTypeChecker(); + + const result = new Map(); + + const visit = (node: ts.Node) => { + if (ts.isIdentifier(node)) { + const symbol = checker.getSymbolAtLocation(node); + let declaration = symbol?.declarations?.find((d) => d); + + // if it's a shorthand property assignment, we need to find the actual declaration + // ref. https://github.com/microsoft/TypeScript/blob/f69580f82146bebfb2bee8c7b8666af0e04c7e34/src/services/goToDefinition.ts#L253 + while (declaration && ts.isShorthandPropertyAssignment(declaration)) { + const s = checker.getShorthandAssignmentValueSymbol(declaration); + declaration = s?.declarations?.find((d) => d); + } + + if (declaration && ts.isNamespaceImport(declaration)) { + switch (true) { + case ts.isNamespaceImport(node.parent): { + // it's the import statement itself + break; + } + case ts.isPropertyAccessExpression(node.parent): { + const usage = node.parent.name.text; + const importedNamespace = declaration.name.text; + const prev = result.get(importedNamespace) || []; + + if (!prev.includes('*')) { + result.set(importedNamespace, [...prev, usage]); + } + + break; + } + default: { + result.set(declaration.name.text, ['*']); + break; + } + } + } + } + + node.forEachChild(visit); + }; + + sourceFile.forEachChild(visit); + + return { + get(name: string) { + return result.get(name) || []; + }, + }; +}; + +const createProgram = ({ sourceFile }: { sourceFile: ts.SourceFile }) => { + const compilerHost: ts.CompilerHost = { + getSourceFile: (fileName) => { + if (fileName === sourceFile.fileName) { + return sourceFile; + } + + return undefined; + }, + getDefaultLibFileName: (o) => ts.getDefaultLibFilePath(o), + writeFile: () => { + throw new Error('not implemented'); + }, + getCurrentDirectory: () => '/', + fileExists: (fileName) => fileName === sourceFile.fileName, + readFile: (fileName) => + fileName === sourceFile.fileName ? sourceFile.text : undefined, + getCanonicalFileName: (fileName) => + ts.sys.useCaseSensitiveFileNames ? fileName : fileName.toLowerCase(), + useCaseSensitiveFileNames: () => true, + getNewLine: () => '\n', + }; + + // for now, not passing the user's ts.CompilerOptions to ts.createProgram should work + const program = ts.createProgram([sourceFile.fileName], {}, compilerHost); + + return program; +}; + +export const namespaceUsage = memoize(fn, { + key: ({ sourceFile }) => `${sourceFile.fileName}::${sourceFile.text}`, +}); diff --git a/lib/util/parseFile.test.ts b/lib/util/parseFile.test.ts index c152322..18336ef 100644 --- a/lib/util/parseFile.test.ts +++ b/lib/util/parseFile.test.ts @@ -101,15 +101,44 @@ describe('parseFile', () => { }); }); - it('should collect namespace imports', () => { - const { imports } = parseFile({ - file: '/app/a.ts', - content: 'import * as b from "./b";', - destFiles: new Set(['/app/b.ts']), - }); - - assert.deepEqual(imports, { - '/app/b.ts': ['*'], + describe('namespace imports', () => { + it('should collect namespace imports with property accesses only', () => { + const { imports } = parseFile({ + file: '/app/a.ts', + content: `import * as b from "./b"; +b.x;`, + destFiles: new Set(['/app/b.ts']), + }); + + assert.deepEqual(imports, { + '/app/b.ts': ['x'], + }); + }); + + it('should collect namespace imports with reference to the namespace itself', () => { + const { imports } = parseFile({ + file: '/app/a.ts', + content: `import * as b from "./b"; +b.x; +b;`, + destFiles: new Set(['/app/b.ts']), + }); + + assert.deepEqual(imports, { + '/app/b.ts': ['*'], + }); + }); + + it('should not include namespace import when its not referenced', () => { + const { imports } = parseFile({ + file: '/app/a.ts', + content: `import * as b from "./b";`, + destFiles: new Set(['/app/b.ts']), + }); + + assert.deepEqual(imports, { + '/app/b.ts': [], + }); }); }); diff --git a/lib/util/parseFile.ts b/lib/util/parseFile.ts index 0afa221..7d2cce5 100644 --- a/lib/util/parseFile.ts +++ b/lib/util/parseFile.ts @@ -1,5 +1,6 @@ import ts from 'typescript'; import { memoize } from './memoize.js'; +import { namespaceUsage } from './namespaceUsage.js'; const getLeadingComment = (node: ts.Node) => { const fullText = node.getSourceFile().getFullText(); @@ -557,7 +558,10 @@ const fn = ({ node.importClause?.namedBindings?.kind === ts.SyntaxKind.NamespaceImport ) { imports[resolved] ||= []; - imports[resolved]?.push('*'); + const usage = namespaceUsage({ sourceFile }); + imports[resolved]?.push( + ...usage.get(node.importClause.namedBindings.name.text), + ); return; } diff --git a/package-lock.json b/package-lock.json index 395499f..46f0b29 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2132,9 +2132,10 @@ } }, "node_modules/typescript": { - "version": "5.2.2", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.2.2.tgz", - "integrity": "sha512-mI4WrpHsbCIcwT9cF4FZvr80QUeKvsUsUvKDoR+X/7XHQH98xYD8YHZg7ANtz2GtZt/CBq2QJ0thkGJMHfqc1w==", + "version": "5.7.2", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.7.2.tgz", + "integrity": "sha512-i5t66RHxDvVN40HfDd1PsEThGNnlMCMT3jMUuoh9/0TaqWevNontacunWyN02LA9/fIbEWlcHZcgTKb9QoaLfg==", + "license": "Apache-2.0", "peer": true, "bin": { "tsc": "bin/tsc", @@ -3555,9 +3556,9 @@ } }, "typescript": { - "version": "5.2.2", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.2.2.tgz", - "integrity": "sha512-mI4WrpHsbCIcwT9cF4FZvr80QUeKvsUsUvKDoR+X/7XHQH98xYD8YHZg7ANtz2GtZt/CBq2QJ0thkGJMHfqc1w==", + "version": "5.7.2", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.7.2.tgz", + "integrity": "sha512-i5t66RHxDvVN40HfDd1PsEThGNnlMCMT3jMUuoh9/0TaqWevNontacunWyN02LA9/fIbEWlcHZcgTKb9QoaLfg==", "peer": true }, "typescript-eslint": {