From 2661880371e415c6eeae8b506387116ec7f2c039 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 14 Oct 2025 17:12:44 -0400 Subject: [PATCH 1/2] refactor(@schematics/angular): implement type-safe TODO notes in jasmine-to-vitest This commit refactors the `jasmine-to-vitest` schematic to use a centralized and type-safe system for generating "TODO" comments. Previously, TODO messages were hardcoded strings scattered across various transformer files. This new system improves maintainability, consistency, and developer experience. Key changes include: - A new `utils/todo-notes.ts` file acts as a single source of truth for all TODO messages, categories, and optional documentation URLs. - Advanced mapped types (`TodoCategory`, `TodoContextMap`) are used to infer types directly from the configuration, ensuring that all calls are type-safe and preventing runtime errors. - The `addTodoComment` helper is now overloaded to handle both static and dynamic (context-aware) messages, ensuring that specific details (like an unsupported function name) are included in the comment. - All transformers have been updated to use this new system, resulting in cleaner, more consistent, and more maintainable code. --- .../test-file-transformer.integration_spec.ts | 6 +- .../transformers/jasmine-lifecycle.ts | 8 +- .../transformers/jasmine-lifecycle_spec.ts | 8 +- .../transformers/jasmine-matcher.ts | 50 +++--- .../transformers/jasmine-matcher_spec.ts | 2 +- .../transformers/jasmine-misc.ts | 57 +++---- .../transformers/jasmine-misc_spec.ts | 4 +- .../transformers/jasmine-spy.ts | 57 +++---- .../transformers/jasmine-spy_spec.ts | 15 +- .../jasmine-vitest/utils/comment-helpers.ts | 44 ++++- .../jasmine-vitest/utils/refactor-reporter.ts | 3 +- .../utils/refactor-reporter_spec.ts | 10 +- .../jasmine-vitest/utils/todo-notes.ts | 152 ++++++++++++++++++ 13 files changed, 287 insertions(+), 129 deletions(-) create mode 100644 packages/schematics/angular/refactor/jasmine-vitest/utils/todo-notes.ts diff --git a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts index 56f5d5701fa8..a440eadbbf48 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.integration_spec.ts @@ -362,7 +362,7 @@ describe('Jasmine to Vitest Transformer - Integration Tests', () => { }); it.skip('should handle pending tests', () => { - // TODO: vitest-migration: The pending() function was converted to a skipped test (\`it.skip\`). + // TODO: vitest-migration: The pending() function was converted to a skipped test (\`it.skip\`). See: https://vitest.dev/api/vi.html#it-skip // pending('This test is not yet implemented.'); }); @@ -422,7 +422,7 @@ describe('Jasmine to Vitest Transformer - Integration Tests', () => { const vitestCode = ` describe('Unsupported Features', () => { beforeAll(() => { - // TODO: vitest-migration: jasmine.addMatchers is not supported. Please manually migrate to expect.extend(). + // TODO: vitest-migration: jasmine.addMatchers is not supported. Please manually migrate to expect.extend(). See: https://vitest.dev/api/expect.html#expect-extend jasmine.addMatchers({ toBeAwesome: () => ({ compare: (actual) => ({ pass: actual === 'awesome' }) @@ -437,7 +437,7 @@ describe('Jasmine to Vitest Transformer - Integration Tests', () => { it('should handle spyOnAllFunctions', () => { const myObj = { func1: () => {}, func2: () => {} }; - // TODO: vitest-migration: Vitest does not have a direct equivalent for jasmine.spyOnAllFunctions(). Please spy on individual methods manually using vi.spyOn(). + // TODO: vitest-migration: Vitest does not have a direct equivalent for jasmine.spyOnAllFunctions(). Please spy on individual methods manually using vi.spyOn(). See: https://vitest.dev/api/vi.html#vi-spyon jasmine.spyOnAllFunctions(myObj); myObj.func1(); expect(myObj.func1).toHaveBeenCalled(); diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle.ts index 9ddf2d3681e0..235eef129d11 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle.ts @@ -84,11 +84,9 @@ export function transformPending( bodyNode, 'Converted `pending()` to a skipped test (`it.skip`).', ); - reporter.recordTodo('pending'); - addTodoComment( - replacement, - 'The pending() function was converted to a skipped test (`it.skip`).', - ); + const category = 'pending'; + reporter.recordTodo(category); + addTodoComment(replacement, category); ts.addSyntheticLeadingComment( replacement, ts.SyntaxKind.SingleLineCommentTrivia, diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle_spec.ts index 8b54f99163ad..3e91e7b74d30 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-lifecycle_spec.ts @@ -190,8 +190,8 @@ describe('Jasmine to Vitest Transformer', () => { `, expected: ` it.skip('is a work in progress', () => { - // TODO: vitest-migration: The pending() function was converted to a skipped test (\`it.skip\`). - // pending('Not yet implemented'); + // TODO: vitest-migration: The pending() function was converted to a skipped test (\`it.skip\`). See: https://vitest.dev/api/vi.html#it-skip + // pending('Not yet implemented'); }); `, }, @@ -204,8 +204,8 @@ describe('Jasmine to Vitest Transformer', () => { `, expected: ` it.skip('is a work in progress', function() { - // TODO: vitest-migration: The pending() function was converted to a skipped test (\`it.skip\`). - // pending('Not yet implemented'); + // TODO: vitest-migration: The pending() function was converted to a skipped test (\`it.skip\`). See: https://vitest.dev/api/vi.html#it-skip + // pending('Not yet implemented'); }); `, }, diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher.ts index dd4cb0a122ea..00ea5fa0e775 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher.ts @@ -51,22 +51,17 @@ export function transformSyntacticSugarMatchers( const matcherName = pae.name.text; if (matcherName === 'toHaveSpyInteractions') { - reporter.recordTodo('toHaveSpyInteractions'); - addTodoComment( - node, - 'Unsupported matcher ".toHaveSpyInteractions()" found. ' + - 'Please migrate this manually by checking the `mock.calls.length` of the individual spies.', - ); + const category = 'toHaveSpyInteractions'; + reporter.recordTodo(category); + addTodoComment(node, category); return node; } if (matcherName === 'toThrowMatching') { - reporter.recordTodo('toThrowMatching'); - addTodoComment( - node, - 'Unsupported matcher ".toThrowMatching()" found. Please migrate this manually.', - ); + const category = 'toThrowMatching'; + reporter.recordTodo(category); + addTodoComment(node, category, { name: matcherName }); return node; } @@ -303,18 +298,13 @@ export function transformExpectAsync( if (matcherName) { if (matcherName === 'toBePending') { - reporter.recordTodo('toBePending'); - addTodoComment( - node, - 'Unsupported matcher ".toBePending()" found. Vitest does not have a direct equivalent. ' + - 'Please migrate this manually, for example by using `Promise.race` to check if the promise settles within a short timeout.', - ); + const category = 'toBePending'; + reporter.recordTodo(category); + addTodoComment(node, category); } else { - reporter.recordTodo('unsupported-expect-async-matcher'); - addTodoComment( - node, - `Unsupported expectAsync matcher ".${matcherName}()" found. Please migrate this manually.`, - ); + const category = 'unsupported-expect-async-matcher'; + reporter.recordTodo(category); + addTodoComment(node, category, { name: matcherName }); } } @@ -422,11 +412,9 @@ export function transformArrayWithExactContents( } if (!ts.isArrayLiteralExpression(argument.arguments[0])) { - reporter.recordTodo('arrayWithExactContents-dynamic-variable'); - addTodoComment( - node, - 'Cannot transform jasmine.arrayWithExactContents with a dynamic variable. Please migrate this manually.', - ); + const category = 'arrayWithExactContents-dynamic-variable'; + reporter.recordTodo(category); + addTodoComment(node, category); return node; } @@ -617,11 +605,9 @@ export function transformExpectNothing( const originalText = node.getFullText().trim(); reporter.reportTransformation(sourceFile, node, 'Removed `expect().nothing()` statement.'); - reporter.recordTodo('expect-nothing'); - addTodoComment( - replacement, - 'expect().nothing() has been removed because it is redundant in Vitest. Tests without assertions pass by default.', - ); + const category = 'expect-nothing'; + reporter.recordTodo(category); + addTodoComment(replacement, category); ts.addSyntheticLeadingComment( replacement, ts.SyntaxKind.SingleLineCommentTrivia, diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher_spec.ts index c6d1f431a54e..6bb4befbaced 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-matcher_spec.ts @@ -168,7 +168,7 @@ describe('Jasmine to Vitest Transformer', () => { { description: 'should add a TODO for toThrowMatching', input: `expect(() => {}).toThrowMatching((e) => e.message === 'foo');`, - expected: `// TODO: vitest-migration: Unsupported matcher ".toThrowMatching()" found. Please migrate this manually. + expected: `// TODO: vitest-migration: Unsupported matcher ".toThrowMatching()" found. Please migrate this manually. See: https://vitest.dev/api/expect.html#tothrowerror expect(() => {}).toThrowMatching((e) => e.message === 'foo');`, }, { diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc.ts index effcf506e1c2..dd6e27c63943 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc.ts @@ -18,6 +18,7 @@ import { createViCallExpression } from '../utils/ast-helpers'; import { getJasmineMethodName, isJasmineCallExpression } from '../utils/ast-validation'; import { addTodoComment } from '../utils/comment-helpers'; import { RefactorContext } from '../utils/refactor-context'; +import { TodoCategory } from '../utils/todo-notes'; export function transformTimerMocks( node: ts.Node, @@ -140,56 +141,42 @@ export function transformGlobalFunctions( node, `Found unsupported global function \`${functionName}\`.`, ); - reporter.recordTodo(functionName); - addTodoComment( - node, - `Unsupported global function \`${functionName}\` found. This function is used for custom reporters in Jasmine ` + - 'and has no direct equivalent in Vitest.', - ); + const category = 'unsupported-global-function'; + reporter.recordTodo(category); + addTodoComment(node, category, { name: functionName }); } return node; } -const JASMINE_UNSUPPORTED_CALLS = new Map([ - [ - 'addMatchers', - 'jasmine.addMatchers is not supported. Please manually migrate to expect.extend().', - ], - [ - 'addCustomEqualityTester', - 'jasmine.addCustomEqualityTester is not supported. Please manually migrate to expect.addEqualityTesters().', - ], - [ - 'mapContaining', - 'jasmine.mapContaining is not supported. Vitest does not have a built-in matcher for Maps.' + - ' Please manually assert the contents of the Map.', - ], - [ - 'setContaining', - 'jasmine.setContaining is not supported. Vitest does not have a built-in matcher for Sets.' + - ' Please manually assert the contents of the Set.', - ], +const UNSUPPORTED_JASMINE_CALLS_CATEGORIES = new Set([ + 'addMatchers', + 'addCustomEqualityTester', + 'mapContaining', + 'setContaining', ]); +// A type guard to ensure that the methodName is one of the categories handled by this transformer. +function isUnsupportedJasmineCall( + methodName: string, +): methodName is 'addMatchers' | 'addCustomEqualityTester' | 'mapContaining' | 'setContaining' { + return UNSUPPORTED_JASMINE_CALLS_CATEGORIES.has(methodName as TodoCategory); +} + export function transformUnsupportedJasmineCalls( node: ts.Node, { sourceFile, reporter }: RefactorContext, ): ts.Node { const methodName = getJasmineMethodName(node); - if (!methodName) { - return node; - } - const message = JASMINE_UNSUPPORTED_CALLS.get(methodName); - if (message) { + if (methodName && isUnsupportedJasmineCall(methodName)) { reporter.reportTransformation( sourceFile, node, `Found unsupported call \`jasmine.${methodName}\`.`, ); reporter.recordTodo(methodName); - addTodoComment(node, message); + addTodoComment(node, methodName); } return node; @@ -238,11 +225,9 @@ export function transformUnknownJasmineProperties( node, `Found unknown jasmine property \`jasmine.${propName}\`.`, ); - reporter.recordTodo(`unknown-jasmine-property: ${propName}`); - addTodoComment( - node, - `Unsupported jasmine property "${propName}" found. Please migrate this manually.`, - ); + const category = 'unknown-jasmine-property'; + reporter.recordTodo(category); + addTodoComment(node, category, { name: propName }); } } diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc_spec.ts index 6b2a92b57fd1..e23f140de382 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-misc_spec.ts @@ -108,7 +108,7 @@ describe('Jasmine to Vitest Transformer', () => { }); `, expected: ` - // TODO: vitest-migration: jasmine.addMatchers is not supported. Please manually migrate to expect.extend(). + // TODO: vitest-migration: jasmine.addMatchers is not supported. Please manually migrate to expect.extend(). See: https://vitest.dev/api/expect.html#expect-extend jasmine.addMatchers({ toBeDivisibleByTwo: function () { return { @@ -141,7 +141,7 @@ describe('Jasmine to Vitest Transformer', () => { }); `, // eslint-disable-next-line max-len - expected: `// TODO: vitest-migration: jasmine.addCustomEqualityTester is not supported. Please manually migrate to expect.addEqualityTesters(). + expected: `// TODO: vitest-migration: jasmine.addCustomEqualityTester is not supported. Please manually migrate to expect.addEqualityTesters(). See: https://vitest.dev/api/expect.html#expect-addequalitytesters jasmine.addCustomEqualityTester((a, b) => { return a.toString() === b.toString(); }); diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts index 2d13a7474104..a3f95e7786d1 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy.ts @@ -146,12 +146,12 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts. return ts.factory.createCallExpression(newExpression, undefined, [arrowFunction]); } - default: - reporter.recordTodo('unsupported-spy-strategy'); - addTodoComment( - node, - `Unsupported spy strategy ".and.${strategyName}()" found. Please migrate this manually.`, - ); + default: { + const category = 'unsupported-spy-strategy'; + reporter.recordTodo(category); + addTodoComment(node, category, { name: strategyName }); + break; + } } if (newMethodName) { @@ -184,20 +184,18 @@ export function transformSpies(node: ts.Node, refactorCtx: RefactorContext): ts. // jasmine.createSpy(name, originalFn) -> vi.fn(originalFn) return createViCallExpression('fn', node.arguments.length > 1 ? [node.arguments[1]] : []); - case 'spyOnAllFunctions': + case 'spyOnAllFunctions': { reporter.reportTransformation( sourceFile, node, 'Found unsupported `jasmine.spyOnAllFunctions()`.', ); - reporter.recordTodo('spyOnAllFunctions'); - addTodoComment( - node, - 'Vitest does not have a direct equivalent for jasmine.spyOnAllFunctions().' + - ' Please spy on individual methods manually using vi.spyOn().', - ); + const category = 'spyOnAllFunctions'; + reporter.recordTodo(category); + addTodoComment(node, category); return node; + } } return node; @@ -218,11 +216,9 @@ export function transformCreateSpyObj( ); if (node.arguments.length < 2) { - reporter.recordTodo('createSpyObj-single-argument'); - addTodoComment( - node, - 'jasmine.createSpyObj called with a single argument is not supported for transformation.', - ); + const category = 'createSpyObj-single-argument'; + reporter.recordTodo(category); + addTodoComment(node, category); return node; } @@ -236,11 +232,9 @@ export function transformCreateSpyObj( } else if (ts.isObjectLiteralExpression(methods)) { properties = createSpyObjWithObject(methods); } else { - reporter.recordTodo('createSpyObj-dynamic-variable'); - addTodoComment( - node, - 'Cannot transform jasmine.createSpyObj with a dynamic variable. Please migrate this manually.', - ); + const category = 'createSpyObj-dynamic-variable'; + reporter.recordTodo(category); + addTodoComment(node, category); return node; } @@ -249,11 +243,9 @@ export function transformCreateSpyObj( if (ts.isObjectLiteralExpression(propertiesArg)) { properties.push(...(propertiesArg.properties as unknown as ts.PropertyAssignment[])); } else { - reporter.recordTodo('createSpyObj-dynamic-property-map'); - addTodoComment( - node, - 'Cannot transform jasmine.createSpyObj with a dynamic property map. Please migrate this manually.', - ); + const category = 'createSpyObj-dynamic-property-map'; + reporter.recordTodo(category); + addTodoComment(node, category); } } @@ -426,12 +418,9 @@ export function transformSpyCallInspection( !ts.isIdentifier(node.parent.name) || node.parent.name.text !== 'args' ) { - reporter.recordTodo('mostRecent-without-args'); - addTodoComment( - node, - 'Direct usage of mostRecent() is not supported.' + - ' Please refactor to access .args directly or use vi.mocked(spy).mock.lastCall.', - ); + const category = 'mostRecent-without-args'; + reporter.recordTodo(category); + addTodoComment(node, category); } return node; diff --git a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts index d54f2c271252..7833f6310496 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/transformers/jasmine-spy_spec.ts @@ -73,7 +73,7 @@ describe('Jasmine to Vitest Transformer', () => { description: 'should add a TODO for jasmine.spyOnAllFunctions(object)', input: `jasmine.spyOnAllFunctions(myObject);`, // eslint-disable-next-line max-len - expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for jasmine.spyOnAllFunctions(). Please spy on individual methods manually using vi.spyOn(). + expected: `// TODO: vitest-migration: Vitest does not have a direct equivalent for jasmine.spyOnAllFunctions(). Please spy on individual methods manually using vi.spyOn(). See: https://vitest.dev/api/vi.html#vi-spyon jasmine.spyOnAllFunctions(myObject); `, }, @@ -109,6 +109,13 @@ describe('Jasmine to Vitest Transformer', () => { input: `spyOn(service, 'myMethod').and.rejectWith('some error');`, expected: `vi.spyOn(service, 'myMethod').mockRejectedValue('some error');`, }, + { + description: 'should add a TODO for an unsupported spy strategy', + input: `spyOn(service, 'myMethod').and.unknownStrategy();`, + // eslint-disable-next-line max-len + expected: `// TODO: vitest-migration: Unsupported spy strategy ".and.unknownStrategy()" found. Please migrate this manually. See: https://vitest.dev/api/mocked.html#mock +vi.spyOn(service, 'myMethod').and.unknownStrategy();`, + }, ]; testCases.forEach(({ description, input, expected }) => { @@ -132,7 +139,7 @@ describe('Jasmine to Vitest Transformer', () => { description: 'should add a TODO if the second argument is not a literal', input: `const myService = jasmine.createSpyObj('MyService', methodNames);`, expected: ` - // TODO: vitest-migration: Cannot transform jasmine.createSpyObj with a dynamic variable. Please migrate this manually. + // TODO: vitest-migration: Cannot transform jasmine.createSpyObj with a dynamic variable. Please migrate this manually. See: https://vitest.dev/api/vi.html#vi-fn const myService = jasmine.createSpyObj('MyService', methodNames); `, }, @@ -156,7 +163,7 @@ describe('Jasmine to Vitest Transformer', () => { description: 'should add a TODO for jasmine.createSpyObj with only one argument', input: `const myService = jasmine.createSpyObj('MyService');`, expected: ` - // TODO: vitest-migration: jasmine.createSpyObj called with a single argument is not supported for transformation. + // TODO: vitest-migration: jasmine.createSpyObj called with a single argument is not supported for transformation. See: https://vitest.dev/api/vi.html#vi-fn const myService = jasmine.createSpyObj('MyService'); `, }, @@ -249,7 +256,7 @@ describe('Jasmine to Vitest Transformer', () => { description: 'should add a TODO for spy.calls.mostRecent() without .args', input: `const mostRecent = mySpy.calls.mostRecent();`, // eslint-disable-next-line max-len - expected: `// TODO: vitest-migration: Direct usage of mostRecent() is not supported. Please refactor to access .args directly or use vi.mocked(spy).mock.lastCall. + expected: `// TODO: vitest-migration: Direct usage of mostRecent() is not supported. Please refactor to access .args directly or use vi.mocked(spy).mock.lastCall. See: https://vitest.dev/api/mocked.html#mock-lastcall const mostRecent = mySpy.calls.mostRecent();`, }, ]; diff --git a/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts b/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts index c804445ec916..c9595d442b29 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts @@ -7,8 +7,48 @@ */ import ts from '../../../third_party/github.com/Microsoft/TypeScript/lib/typescript'; +import { TODO_NOTES, TodoCategory, TodoContextMap } from './todo-notes'; + +// A helper type to find all `TodoCategory` keys that do not require a context. +type CategoriesWithNoContext = { + [K in TodoCategory]: TodoContextMap[K] extends never ? K : never; +}[TodoCategory]; + +/** + * Adds a TODO comment to a TypeScript node for manual migration. + * This overload handles categories that do not require a context object. + * @param node The AST node to which the comment will be added. + * @param category The category of the TODO, used to look up the message and URL. + */ +export function addTodoComment(node: ts.Node, category: T): void; + +/** + * Adds a TODO comment to a TypeScript node for manual migration. + * This overload handles categories that require a context object, ensuring it is + * provided and correctly typed. + * @param node The AST node to which the comment will be added. + * @param category The category of the TODO, used to look up the message and URL. + * @param context The context object providing dynamic values for the message. + */ +export function addTodoComment( + node: ts.Node, + category: T, + context: TodoContextMap[T], +): void; + +// Implementation that covers both overloads. +export function addTodoComment( + node: ts.Node, + category: TodoCategory, + context?: Record, +): void { + const note = TODO_NOTES[category]; + // The type assertion is safe here because the overloads guarantee the correct context is passed. + const message = + typeof note.message === 'function' ? note.message(context as never) : note.message; + const url = 'url' in note && note.url ? ` See: ${note.url}` : ''; + const commentText = ` TODO: vitest-migration: ${message}${url}`; -export function addTodoComment(node: ts.Node, message: string) { let statement: ts.Node = node; // Attempt to find the containing statement @@ -22,7 +62,7 @@ export function addTodoComment(node: ts.Node, message: string) { ts.addSyntheticLeadingComment( statement, ts.SyntaxKind.SingleLineCommentTrivia, - ` TODO: vitest-migration: ${message}`, + commentText, true, ); } diff --git a/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter.ts b/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter.ts index 55d235e96c25..737abdc0ef94 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter.ts @@ -8,6 +8,7 @@ import { logging } from '@angular-devkit/core'; import ts from '../../../third_party/github.com/Microsoft/TypeScript/lib/typescript'; +import { TodoCategory } from './todo-notes'; export class RefactorReporter { private filesScanned = 0; @@ -29,7 +30,7 @@ export class RefactorReporter { this.filesTransformed++; } - recordTodo(category: string): void { + recordTodo(category: TodoCategory): void { this.todos.set(category, (this.todos.get(category) ?? 0) + 1); } diff --git a/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter_spec.ts index f8b79391f72f..10053135a10e 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/utils/refactor-reporter_spec.ts @@ -34,14 +34,14 @@ describe('RefactorReporter', () => { }); it('should record and count todos by category', () => { - reporter.recordTodo('category-a'); - reporter.recordTodo('category-b'); - reporter.recordTodo('category-a'); + reporter.recordTodo('pending'); + reporter.recordTodo('spyOnAllFunctions'); + reporter.recordTodo('pending'); reporter.printSummary(); expect(logger.warn).toHaveBeenCalledWith('- 3 TODO(s) added for manual review:'); - expect(logger.warn).toHaveBeenCalledWith(' - 2x category-a'); - expect(logger.warn).toHaveBeenCalledWith(' - 1x category-b'); + expect(logger.warn).toHaveBeenCalledWith(' - 2x pending'); + expect(logger.warn).toHaveBeenCalledWith(' - 1x spyOnAllFunctions'); }); it('should not print the todos section if none were recorded', () => { diff --git a/packages/schematics/angular/refactor/jasmine-vitest/utils/todo-notes.ts b/packages/schematics/angular/refactor/jasmine-vitest/utils/todo-notes.ts new file mode 100644 index 000000000000..c0a5eb406e6a --- /dev/null +++ b/packages/schematics/angular/refactor/jasmine-vitest/utils/todo-notes.ts @@ -0,0 +1,152 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +/** + * @fileoverview This file is the single source of truth for all "TODO" notes + * generated by the Jasmine to Vitest schematic. + * + * It defines the `TODO_NOTES` constant, which contains the message and optional + * documentation URL for each category of manual migration task. + * + * The file also exports advanced mapped types (`TodoCategory`, `TodoContextMap`) + * that are inferred directly from the `TODO_NOTES` object. This creates a + * maintainable, type-safe system that ensures consistency across all + * transformers and prevents runtime errors. + */ + +/** + * The central configuration for all "TODO" notes. Each key represents a unique + * `TodoCategory`. + * + * Each entry is an object with: + * - `message`: A string or a function that returns a string. If it's a function, + * it receives a `context` object to generate a dynamic message. + * - `url`: An optional documentation URL that will be appended to the message. + */ +export const TODO_NOTES = { + 'pending': { + message: 'The pending() function was converted to a skipped test (`it.skip`).', + url: 'https://vitest.dev/api/vi.html#it-skip', + }, + 'toHaveSpyInteractions': { + message: + 'Unsupported matcher ".toHaveSpyInteractions()" found. ' + + 'Please migrate this manually by checking the `mock.calls.length` of the individual spies.', + }, + 'toThrowMatching': { + message: (context: { name: string }) => + `Unsupported matcher ".${context.name}()" found. Please migrate this manually.`, + url: 'https://vitest.dev/api/expect.html#tothrowerror', + }, + 'toBePending': { + message: + 'Unsupported matcher ".toBePending()" found. Vitest does not have a direct equivalent. ' + + 'Please migrate this manually, for example by using `Promise.race` to check if the promise settles within a short timeout.', + }, + 'unsupported-expect-async-matcher': { + message: (context: { name: string }) => + `Unsupported expectAsync matcher ".${context.name}()" found. Please migrate this manually.`, + }, + 'arrayWithExactContents-dynamic-variable': { + message: + 'Cannot transform jasmine.arrayWithExactContents with a dynamic variable. Please migrate this manually.', + }, + 'expect-nothing': { + message: + 'expect().nothing() has been removed because it is redundant in Vitest. Tests without assertions pass by default.', + }, + 'unsupported-global-function': { + message: (context: { name: string }) => + `Unsupported global function \`${context.name}\` found. This function is used for custom reporters in Jasmine ` + + 'and has no direct equivalent in Vitest.', + }, + 'addMatchers': { + message: 'jasmine.addMatchers is not supported. Please manually migrate to expect.extend().', + url: 'https://vitest.dev/api/expect.html#expect-extend', + }, + 'addCustomEqualityTester': { + message: + 'jasmine.addCustomEqualityTester is not supported. Please manually migrate to expect.addEqualityTesters().', + url: 'https://vitest.dev/api/expect.html#expect-addequalitytesters', + }, + 'mapContaining': { + message: + 'jasmine.mapContaining is not supported. Vitest does not have a built-in matcher for Maps.' + + ' Please manually assert the contents of the Map.', + }, + 'setContaining': { + message: + 'jasmine.setContaining is not supported. Vitest does not have a built-in matcher for Sets.' + + ' Please manually assert the contents of the Set.', + }, + 'unknown-jasmine-property': { + message: (context: { name: string }) => + `Unsupported jasmine property "${context.name}" found. Please migrate this manually.`, + }, + 'spyOnAllFunctions': { + message: + 'Vitest does not have a direct equivalent for jasmine.spyOnAllFunctions().' + + ' Please spy on individual methods manually using vi.spyOn().', + url: 'https://vitest.dev/api/vi.html#vi-spyon', + }, + 'createSpyObj-single-argument': { + message: + 'jasmine.createSpyObj called with a single argument is not supported for transformation.', + url: 'https://vitest.dev/api/vi.html#vi-fn', + }, + 'createSpyObj-dynamic-variable': { + message: + 'Cannot transform jasmine.createSpyObj with a dynamic variable. Please migrate this manually.', + url: 'https://vitest.dev/api/vi.html#vi-fn', + }, + 'createSpyObj-dynamic-property-map': { + message: + 'Cannot transform jasmine.createSpyObj with a dynamic property map. Please migrate this manually.', + url: 'https://vitest.dev/api/vi.html#vi-fn', + }, + 'unsupported-spy-strategy': { + message: (context: { name: string }) => + `Unsupported spy strategy ".and.${context.name}()" found. Please migrate this manually.`, + url: 'https://vitest.dev/api/mocked.html#mock', + }, + 'mostRecent-without-args': { + message: + 'Direct usage of mostRecent() is not supported.' + + ' Please refactor to access .args directly or use vi.mocked(spy).mock.lastCall.', + url: 'https://vitest.dev/api/mocked.html#mock-lastcall', + }, +} as const; + +/** + * A union type of all possible "TODO" categories. + * It is derived from the keys of the `TODO_NOTES` object to ensure that only + * valid categories can be used throughout the transformers. + */ +export type TodoCategory = keyof typeof TODO_NOTES; + +/** + * A mapped type that creates a map from a `TodoCategory` to the type of the + * context object that its message function expects. This provides strong type + * safety for calls to `addTodoComment`. + * + * It works by checking if the `message` property for a given category is a + * function. If it is, it uses `infer` to extract the type of the first + * parameter (`P`). If it's not a function, it resolves to `never`. + * + * @example + * // `Context` will be `{ name: string }` + * type Context = TodoContextMap['unknown-jasmine-property']; + * + * // `Context` will be `never` + * type Context = TodoContextMap['pending']; + */ +export type TodoContextMap = { + [K in TodoCategory]: (typeof TODO_NOTES)[K]['message'] extends (context: infer P) => string + ? P + : never; +}; From a68f0a0b6f061143957042eecb07f2b3b7918cc9 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 14 Oct 2025 19:54:19 -0400 Subject: [PATCH 2/2] test(@schematics/angular): add unit tests for `addTodoComment` helper This commit introduces unit tests for the `addTodoComment` helper function in the `jasmine-to-vitest` schematic. It also adds detailed comments to the function's AST traversal logic to improve clarity. --- .../jasmine-vitest/utils/comment-helpers.ts | 6 +- .../utils/comment-helpers_spec.ts | 68 +++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers_spec.ts diff --git a/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts b/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts index c9595d442b29..3f37f9a5a383 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers.ts @@ -51,7 +51,11 @@ export function addTodoComment( let statement: ts.Node = node; - // Attempt to find the containing statement + // Traverse up the AST to find the containing statement for the node. + // This ensures that the comment is placed before the entire statement, + // rather than being attached to a deeply nested node. For example, if the + // node is an `Identifier`, we want the comment on the `VariableStatement` + // or `ExpressionStatement` that contains it. while (statement.parent && !ts.isBlock(statement.parent) && !ts.isSourceFile(statement.parent)) { if (ts.isExpressionStatement(statement) || ts.isVariableStatement(statement)) { break; diff --git a/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers_spec.ts new file mode 100644 index 000000000000..50c0093c0208 --- /dev/null +++ b/packages/schematics/angular/refactor/jasmine-vitest/utils/comment-helpers_spec.ts @@ -0,0 +1,68 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import ts from '../../../third_party/github.com/Microsoft/TypeScript/lib/typescript'; +import { addTodoComment } from './comment-helpers'; + +describe('addTodoComment', () => { + function createTestHarness(sourceText: string) { + const sourceFile = ts.createSourceFile('test.ts', sourceText, ts.ScriptTarget.Latest, true); + const printer = ts.createPrinter({ newLine: ts.NewLineKind.LineFeed }); + + return { + sourceFile, + run(node: ts.Node, category: 'pending') { + addTodoComment(node, category); + + return printer.printFile(sourceFile); + }, + }; + } + + it('should add a comment before the containing ExpressionStatement', () => { + const sourceText = `myFunction();`; + const { sourceFile, run } = createTestHarness(sourceText); + const callExpression = (sourceFile.statements[0] as ts.ExpressionStatement).expression; + + const result = run(callExpression, 'pending'); + + expect(result).toContain( + '// TODO: vitest-migration: The pending() function was converted to a skipped test (`it.skip`). See: https://vitest.dev/api/vi.html#it-skip', + ); + expect(result.trim().startsWith('// TODO')).toBe(true); + }); + + it('should find the top-level statement for a deeply nested node', () => { + const sourceText = `const result = myObject.prop.method();`; + const { sourceFile, run } = createTestHarness(sourceText); + + // Get a deeply nested identifier + const varDeclaration = (sourceFile.statements[0] as ts.VariableStatement).declarationList + .declarations[0]; + const methodIdentifier = ( + (varDeclaration.initializer as ts.CallExpression).expression as ts.PropertyAccessExpression + ).name; + + const result = run(methodIdentifier, 'pending'); + + expect(result.trim().startsWith('// TODO')).toBe(true); + expect(result).toContain('const result = myObject.prop.method()'); + }); + + it('should add a comment before a VariableStatement', () => { + const sourceText = `const mySpy = jasmine.createSpy();`; + const { sourceFile, run } = createTestHarness(sourceText); + const varDeclaration = (sourceFile.statements[0] as ts.VariableStatement).declarationList + .declarations[0]; + + const result = run(varDeclaration, 'pending'); + + expect(result.trim().startsWith('// TODO')).toBe(true); + expect(result).toContain('const mySpy = jasmine.createSpy()'); + }); +});