From a771c4236dca0a4753ca6a513b8ebc5c5e2fc0f3 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 4 Nov 2025 14:36:24 -0500 Subject: [PATCH] refactor(@schematics/angular): improve code quality in jasmine-vitest transformer This commit introduces several code quality improvements to the `test-file-transformer.ts` file within the Jasmine to Vitest schematic. These changes enhance readability, maintainability, and performance without altering the existing behavior. Key improvements include: - **Module-level transformer arrays:** The `callExpressionTransformers`, `propertyAccessExpressionTransformers`, and `expressionStatementTransformers` arrays have been refactored to be module-level constants. This ensures they are initialized only once when the module is loaded, improving performance, and clearly separates static configuration from dynamic execution logic. - **Comprehensive JSDoc comments:** Extensive JSDoc comments have been added or updated throughout the file, including a file-level overview, detailed explanations for module-level constants, and updated documentation for key functions. This significantly improves code clarity and maintainability for future development. --- .../jasmine-vitest/test-file-transformer.ts | 134 ++++++++++++------ .../test-file-transformer_add-imports_spec.ts | 29 ++++ 2 files changed, 116 insertions(+), 47 deletions(-) diff --git a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.ts b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.ts index da9bf28ba420..eb8afdfe0449 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer.ts @@ -6,6 +6,13 @@ * found in the LICENSE file at https://angular.dev/license */ +/** + * @fileoverview This is the main entry point for the Jasmine to Vitest transformation. + * It orchestrates the application of various AST transformers to convert Jasmine test + * syntax and APIs to their Vitest equivalents. It also handles import management, + * blank line preservation, and reporting of transformation details. + */ + import ts from '../../third_party/github.com/Microsoft/TypeScript/lib/typescript'; import { transformDoneCallback, @@ -43,8 +50,18 @@ import { addVitestValueImport, getVitestAutoImports } from './utils/ast-helpers' import { RefactorContext } from './utils/refactor-context'; import { RefactorReporter } from './utils/refactor-reporter'; +/** + * A placeholder used to temporarily replace blank lines in the source code. + * This is necessary because TypeScript's printer removes blank lines by default. + */ const BLANK_LINE_PLACEHOLDER = '// __PRESERVE_BLANK_LINE__'; +/** + * Replaces blank lines in the content with a placeholder to prevent TypeScript's printer + * from removing them. This ensures that the original formatting of blank lines is preserved. + * @param content The source code content. + * @returns The content with blank lines replaced by placeholders. + */ function preserveBlankLines(content: string): string { return content .split('\n') @@ -52,19 +69,83 @@ function preserveBlankLines(content: string): string { .join('\n'); } +/** + * Restores blank lines in the content by replacing the placeholder with actual blank lines. + * This is called after TypeScript's printer has processed the file. + * @param content The content with blank line placeholders. + * @returns The content with blank lines restored. + */ function restoreBlankLines(content: string): string { - const regex = new RegExp(`^\\s*${BLANK_LINE_PLACEHOLDER.replace(/\//g, '\\/')}\\s*$`, 'gm'); + const regex = /^\s*\/\/ __PRESERVE_BLANK_LINE__\s*$/gm; return content.replace(regex, ''); } +/** + * A collection of transformers that operate on `ts.CallExpression` nodes. + * These are applied in stages to ensure correct order of operations: + * 1. High-Level & Context-Sensitive: Transformations that fundamentally change the call. + * 2. Core Matcher & Spy: Bulk conversions for `expect(...)` and `spyOn(...)`. + * 3. Global Functions & Cleanup: Handles global Jasmine functions and unsupported APIs. + */ +const callExpressionTransformers = [ + // **Stage 1: High-Level & Context-Sensitive Transformations** + // These transformers often wrap or fundamentally change the nature of the call, + // so they need to run before more specific matchers. + transformWithContext, + transformExpectAsync, + transformFocusedAndSkippedTests, + transformPending, + transformDoneCallback, + + // **Stage 2: Core Matcher & Spy Transformations** + // This is the bulk of the `expect(...)` and `spyOn(...)` conversions. + transformSyntacticSugarMatchers, + transformComplexMatchers, + transformSpies, + transformCreateSpyObj, + transformSpyReset, + transformSpyCallInspection, + transformtoHaveBeenCalledBefore, + transformToHaveClass, + + // **Stage 3: Global Functions & Cleanup** + // These handle global Jasmine functions and catch-alls for unsupported APIs. + transformTimerMocks, + transformGlobalFunctions, + transformUnsupportedJasmineCalls, +]; + +/** + * A collection of transformers that operate on `ts.PropertyAccessExpression` nodes. + * These primarily handle `jasmine.any()` and other `jasmine.*` properties. + */ +const propertyAccessExpressionTransformers = [ + // These transformers handle `jasmine.any()` and other `jasmine.*` properties. + transformAsymmetricMatchers, + transformSpyCallInspection, + transformUnknownJasmineProperties, +]; + +/** + * A collection of transformers that operate on `ts.ExpressionStatement` nodes. + * These are mutually exclusive; the first one that matches will be applied. + */ +const expressionStatementTransformers = [ + transformCalledOnceWith, + transformArrayWithExactContents, + transformExpectNothing, + transformFail, + transformDefaultTimeoutInterval, +]; + /** * Transforms a string of Jasmine test code to Vitest test code. * This is the main entry point for the transformation. * @param filePath The path to the file being transformed. * @param content The source code to transform. * @param reporter The reporter to track TODOs. - * @param options Transformation options. + * @param options Transformation options, including whether to add Vitest API imports. * @returns The transformed code. */ export function transformJasmineToVitest( @@ -85,6 +166,7 @@ export function transformJasmineToVitest( const pendingVitestValueImports = new Set(); const pendingVitestTypeImports = new Set(); + const transformer: ts.TransformerFactory = (context) => { const refactorCtx: RefactorContext = { sourceFile, @@ -106,59 +188,17 @@ export function transformJasmineToVitest( } } - const transformations = [ - // **Stage 1: High-Level & Context-Sensitive Transformations** - // These transformers often wrap or fundamentally change the nature of the call, - // so they need to run before more specific matchers. - transformWithContext, - transformExpectAsync, - transformFocusedAndSkippedTests, - transformPending, - transformDoneCallback, - - // **Stage 2: Core Matcher & Spy Transformations** - // This is the bulk of the `expect(...)` and `spyOn(...)` conversions. - transformSyntacticSugarMatchers, - transformComplexMatchers, - transformSpies, - transformCreateSpyObj, - transformSpyReset, - transformSpyCallInspection, - transformtoHaveBeenCalledBefore, - transformToHaveClass, - - // **Stage 3: Global Functions & Cleanup** - // These handle global Jasmine functions and catch-alls for unsupported APIs. - transformTimerMocks, - transformGlobalFunctions, - transformUnsupportedJasmineCalls, - ]; - - for (const transformer of transformations) { + for (const transformer of callExpressionTransformers) { transformedNode = transformer(transformedNode, refactorCtx); } } else if (ts.isPropertyAccessExpression(transformedNode)) { - const transformations = [ - // These transformers handle `jasmine.any()` and other `jasmine.*` properties. - transformAsymmetricMatchers, - transformSpyCallInspection, - transformUnknownJasmineProperties, - ]; - for (const transformer of transformations) { + for (const transformer of propertyAccessExpressionTransformers) { transformedNode = transformer(transformedNode, refactorCtx); } } else if (ts.isExpressionStatement(transformedNode)) { // Statement-level transformers are mutually exclusive. The first one that // matches will be applied, and then the visitor will stop for this node. - const statementTransformers = [ - transformCalledOnceWith, - transformArrayWithExactContents, - transformExpectNothing, - transformFail, - transformDefaultTimeoutInterval, - ]; - - for (const transformer of statementTransformers) { + for (const transformer of expressionStatementTransformers) { const result = transformer(transformedNode, refactorCtx); if (result !== transformedNode) { transformedNode = result; diff --git a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts index 4f85efc48807..fac88bb9ed02 100644 --- a/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts +++ b/packages/schematics/angular/refactor/jasmine-vitest/test-file-transformer_add-imports_spec.ts @@ -52,5 +52,34 @@ describe('Jasmine to Vitest Transformer', () => { const expected = `const a = 1;`; await expectTransformation(input, expected, true); }); + + it('should add imports for top-level describe and it when addImports is true', async () => { + const input = ` + describe('My Suite', () => { + it('should do something', () => { + // test content + }); + }); + `; + const expected = ` + import { describe, it } from 'vitest'; + + describe('My Suite', () => { + it('should do something', () => { + // test content + }); + }); + `; + await expectTransformation(input, expected, true); + }); + + it('should add imports for top-level expect when addImports is true', async () => { + const input = `expect(true).toBe(true);`; + const expected = ` + import { expect } from 'vitest'; + expect(true).toBe(true); + `; + await expectTransformation(input, expected, true); + }); }); });