From 02c342147b74a1b26c466fb893cb12b427922fbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Espiau?= <7319147+FredericEspiau@users.noreply.github.com> Date: Mon, 16 Dec 2024 15:01:06 +0100 Subject: [PATCH 1/3] fix: prevent removing decorators on classes --- lib/util/edit.test.ts | 59 +++++++++++++++++++++++++++++++++++++++++++ lib/util/parseFile.ts | 36 ++++++++++++++++++++------ 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/lib/util/edit.test.ts b/lib/util/edit.test.ts index fb2abab..e3e651d 100644 --- a/lib/util/edit.test.ts +++ b/lib/util/edit.test.ts @@ -289,6 +289,65 @@ function a2() {}`, }); describe('class declaration', () => { + it('should not remove decorators when exports are deleted', () => { + const fileService = new MemoryFileService(); + + fileService.set('/app/main.ts', ``); + fileService.set( + '/app/a.ts', + `@myDecorator +export class A {} +const a = new A(); +console.log(a);`, + ); + fileService.set( + '/app/b.ts', + `@myDecorator +export default class B {} +const b = new B(); +console.log(b);`, + ); + fileService.set( + '/app/c.ts', + `@firstDecorator +@secondDecorator(() => [WithArgument]) +export default class C {} +const c = new C(); +console.log(c);`, + ); + + edit({ + fileService, + recursive, + entrypoints: ['/app/main.ts'], + }); + + assert.equal( + fileService.get('/app/a.ts'), + `@myDecorator +class A {} +const a = new A(); +console.log(a);`, + ); + + assert.equal( + fileService.get('/app/b.ts'), + `@myDecorator +class B {} +const b = new B(); +console.log(b);`, + ); + + assert.equal( + fileService.get('/app/c.ts'), + `@firstDecorator +@secondDecorator(() => [WithArgument]) +class C {} +const c = new C(); +console.log(c);`, + ); + }); + it('should not remove export for class if its used in some other file', () => { const fileService = new MemoryFileService(); diff --git a/lib/util/parseFile.ts b/lib/util/parseFile.ts index 7d2cce5..4dec1e5 100644 --- a/lib/util/parseFile.ts +++ b/lib/util/parseFile.ts @@ -83,27 +83,47 @@ const getChange = ( }; } - // we want to correctly remove 'default' when its a default export so we get the syntaxList node instead of the exportKeyword node - // note: the first syntaxList node should contain the export keyword + /** + * The syntax list contains the keywords that can be found before the actual declaration. + * We want to remove everything that is not a decorator. + */ const syntaxListIndex = node .getChildren() .findIndex((n) => n.kind === ts.SyntaxKind.SyntaxList); - const syntaxList = node.getChildren()[syntaxListIndex]; + const syntaxList = node?.getChildren()[syntaxListIndex]; + + if (!syntaxList) { + throw new Error('Syntaxlist missing'); + } + + const firstKeywordToDeleteIndex = syntaxList + .getChildren() + .findIndex((n) => n.kind !== ts.SyntaxKind.Decorator); + + const firstKeywordToDelete = + syntaxList.getChildren()[firstKeywordToDeleteIndex]; + + if (!firstKeywordToDelete) { + throw new Error( + 'Unexpected syntax list when looking for keywords after decorators', + ); + } + const nextSibling = node.getChildren()[syntaxListIndex + 1]; - if (!syntaxList || !nextSibling) { - throw new Error('Unexpected syntax'); + if (!nextSibling) { + throw new Error('No sibling after syntax list'); } return { code: node .getSourceFile() .getFullText() - .slice(syntaxList.getStart(), nextSibling.getStart()), + .slice(firstKeywordToDelete.getStart(), nextSibling.getStart()), span: { - start: syntaxList.getStart(), - length: nextSibling.getStart() - syntaxList.getStart(), + start: firstKeywordToDelete.getStart(), + length: nextSibling.getStart() - firstKeywordToDelete.getStart(), }, }; }; From 145a30abadf279335ef09fa0e73ba18d64a411a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Espiau?= <7319147+FredericEspiau@users.noreply.github.com> Date: Mon, 16 Dec 2024 17:36:36 +0100 Subject: [PATCH 2/3] chore: review comments --- lib/util/edit.test.ts | 24 ++++++------------------ lib/util/parseFile.ts | 4 ++-- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/util/edit.test.ts b/lib/util/edit.test.ts index e3e651d..34fe9d2 100644 --- a/lib/util/edit.test.ts +++ b/lib/util/edit.test.ts @@ -296,24 +296,18 @@ function a2() {}`, fileService.set( '/app/a.ts', `@myDecorator -export class A {} -const a = new A(); -console.log(a);`, +export class A {}`, ); fileService.set( '/app/b.ts', `@myDecorator -export default class B {} -const b = new B(); -console.log(b);`, +export default class B {}`, ); fileService.set( '/app/c.ts', `@firstDecorator @secondDecorator(() => [WithArgument]) -export default class C {} -const c = new C(); -console.log(c);`, +export default class C {}`, ); edit({ @@ -325,26 +319,20 @@ console.log(c);`, assert.equal( fileService.get('/app/a.ts'), `@myDecorator -class A {} -const a = new A(); -console.log(a);`, +class A {}`, ); assert.equal( fileService.get('/app/b.ts'), `@myDecorator -class B {} -const b = new B(); -console.log(b);`, +class B {}`, ); assert.equal( fileService.get('/app/c.ts'), `@firstDecorator @secondDecorator(() => [WithArgument]) -class C {} -const c = new C(); -console.log(c);`, +class C {}`, ); }); diff --git a/lib/util/parseFile.ts b/lib/util/parseFile.ts index 4dec1e5..f1923a5 100644 --- a/lib/util/parseFile.ts +++ b/lib/util/parseFile.ts @@ -91,10 +91,10 @@ const getChange = ( .getChildren() .findIndex((n) => n.kind === ts.SyntaxKind.SyntaxList); - const syntaxList = node?.getChildren()[syntaxListIndex]; + const syntaxList = node.getChildren()[syntaxListIndex]; if (!syntaxList) { - throw new Error('Syntaxlist missing'); + throw new Error('syntaxlist missing'); } const firstKeywordToDeleteIndex = syntaxList From c1303ce7983d36168b5bfbe78c36c4dece97642f Mon Sep 17 00:00:00 2001 From: Kazushi Konosu Date: Tue, 17 Dec 2024 10:58:53 +0900 Subject: [PATCH 3/3] Update lib/util/parseFile.ts --- lib/util/parseFile.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util/parseFile.ts b/lib/util/parseFile.ts index f1923a5..8b6430d 100644 --- a/lib/util/parseFile.ts +++ b/lib/util/parseFile.ts @@ -94,7 +94,7 @@ const getChange = ( const syntaxList = node.getChildren()[syntaxListIndex]; if (!syntaxList) { - throw new Error('syntaxlist missing'); + throw new Error('syntaxList missing'); } const firstKeywordToDeleteIndex = syntaxList