diff --git a/jest.config.js b/jest.config.js index d3a7fa6ae57..1da0d7cc5fb 100644 --- a/jest.config.js +++ b/jest.config.js @@ -44,6 +44,8 @@ module.exports = { "/libs/vault/jest.config.js", "/libs/key-management/jest.config.js", "/libs/key-management-ui/jest.config.js", + + "/scripts/migration/i18n/jest.config.js", ], // Workaround for a memory leak that crashes tests in CI: diff --git a/scripts/migration/i18n/templates/template-parser.ts b/scripts/migration/i18n/templates/template-parser.ts index a5a13d3f74d..727863c32bc 100644 --- a/scripts/migration/i18n/templates/template-parser.ts +++ b/scripts/migration/i18n/templates/template-parser.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-console */ import { parseTemplate, TmplAstNode, TmplAstElement, TmplAstBoundText } from "@angular/compiler"; import { I18nUsage } from "../shared/types"; @@ -13,22 +12,11 @@ export class TemplateParser { findI18nPipeUsage(templateContent: string, filePath: string): I18nUsage[] { const usages: I18nUsage[] = []; - try { - // Parse template using Angular compiler - const parseResult = parseTemplate(templateContent, filePath); + // Parse template using Angular compiler + const parseResult = parseTemplate(templateContent, filePath); - if (parseResult.nodes) { - this.traverseNodes(parseResult.nodes, usages, filePath); - } - - // Also use regex as fallback for edge cases - this.findWithRegex(templateContent, filePath, usages); - } catch (error) { - const errorMessage = error instanceof Error ? error.message : String(error); - console.warn(`Warning: Could not parse template ${filePath}:`, errorMessage); - - // Fallback to regex parsing - this.findWithRegex(templateContent, filePath, usages); + if (parseResult.nodes) { + this.traverseNodes(parseResult.nodes, usages, filePath); } return usages; @@ -55,7 +43,8 @@ export class TemplateParser { // Handle bound text nodes (interpolations) if (this.isBoundText(node)) { const expression = node.value; - if (expression && "source" in expression) { + + if (expression && typeof expression == "object" && "source" in expression) { const expressionText = (expression.source as string) || ""; if (this.containsI18nPipe(expressionText)) { @@ -77,7 +66,7 @@ export class TemplateParser { // Handle element nodes with attributes if (this.isElement(node)) { - // Check bound attributes (property bindinxgs) + // Check bound attributes (property bindings) for (const input of node.inputs || []) { if (input.value && "source" in input.value) { const inputValue = (input.value.source as string) || ""; @@ -118,170 +107,6 @@ export class TemplateParser { } } - /** - * Fallback regex-based parsing for edge cases - */ - private findWithRegex(templateContent: string, filePath: string, usages: I18nUsage[]): void { - // Find interpolation usage: {{ 'key' | i18n }} - this.findInterpolationUsage(templateContent, filePath, usages); - - // Find attribute usage: [attr]="'key' | i18n" - this.findAttributeUsage(templateContent, filePath, usages); - } - - /** - * Find i18n pipe usage in interpolations {{ }} - */ - private findInterpolationUsage( - templateContent: string, - filePath: string, - usages: I18nUsage[], - ): void { - // Pattern to match {{ 'key' | i18n }} or {{ "key" | i18n }} with optional parameters - const interpolationPattern = /\{\{\s*['"`]([^'"`]+)['"`]\s*\|\s*i18n(?::([^}]+))?\s*\}\}/g; - - let match; - while ((match = interpolationPattern.exec(templateContent)) !== null) { - const key = match[1]; - const paramString = match[2]; - const parameters = paramString - ? paramString - .split(":") - .map((p) => p.trim()) - .filter((p) => p) - : undefined; - - // Check if we already found this usage via AST parsing - const position = this.getPositionInfo(templateContent, match.index); - const alreadyFound = usages.some( - (usage) => - usage.line === position.line && usage.column === position.column && usage.key === key, - ); - - if (!alreadyFound) { - usages.push({ - filePath, - line: position.line, - column: position.column, - method: "pipe", - key, - parameters, - context: match[0], - }); - } - } - - // Also handle variable interpolations: {{ variable | i18n }} - const variableInterpolationPattern = - /\{\{\s*([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\s*\|\s*i18n(?::([^}]+))?\s*\}\}/g; - - while ((match = variableInterpolationPattern.exec(templateContent)) !== null) { - const key = match[1]; - const paramString = match[2]; - const parameters = paramString - ? paramString - .split(":") - .map((p) => p.trim()) - .filter((p) => p) - : undefined; - - const position = this.getPositionInfo(templateContent, match.index); - const alreadyFound = usages.some( - (usage) => - usage.line === position.line && usage.column === position.column && usage.key === key, - ); - - if (!alreadyFound) { - usages.push({ - filePath, - line: position.line, - column: position.column, - method: "pipe", - key, - parameters, - context: match[0], - }); - } - } - } - - /** - * Find i18n pipe usage in attributes - */ - private findAttributeUsage(templateContent: string, filePath: string, usages: I18nUsage[]): void { - // Pattern to match [attr]="'key' | i18n" or attr="{{ 'key' | i18n }}" - const attributePattern = /(\[?[\w-]+\]?)\s*=\s*["']([^"']*\|\s*i18n[^"']*)["']/g; - - let match; - while ((match = attributePattern.exec(templateContent)) !== null) { - const attrValue = match[2]; - - // Extract the key from the pipe expression - const keyMatch = attrValue.match(/['"`]([^'"`]+)['"`]\s*\|\s*i18n(?::([^"'|]+))?/); - if (keyMatch) { - const key = keyMatch[1]; - const paramString = keyMatch[2]; - const parameters = paramString - ? paramString - .split(":") - .map((p) => p.trim()) - .filter((p) => p) - : undefined; - - const position = this.getPositionInfo(templateContent, match.index); - const alreadyFound = usages.some( - (usage) => - usage.line === position.line && usage.column === position.column && usage.key === key, - ); - - if (!alreadyFound) { - usages.push({ - filePath, - line: position.line, - column: position.column, - method: "pipe", - key, - parameters, - context: match[0], - }); - } - } - - // Also handle variable attributes - const variableMatch = attrValue.match( - /([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\s*\|\s*i18n(?::([^"'|]+))?/, - ); - if (variableMatch && !keyMatch) { - const key = variableMatch[1]; - const paramString = variableMatch[2]; - const parameters = paramString - ? paramString - .split(":") - .map((p) => p.trim()) - .filter((p) => p) - : undefined; - - const position = this.getPositionInfo(templateContent, match.index); - const alreadyFound = usages.some( - (usage) => - usage.line === position.line && usage.column === position.column && usage.key === key, - ); - - if (!alreadyFound) { - usages.push({ - filePath, - line: position.line, - column: position.column, - method: "pipe", - key, - parameters, - context: match[0], - }); - } - } - } - } - /** * Check if a node is a bound text node */ diff --git a/scripts/migration/i18n/templates/template-transformer.ts b/scripts/migration/i18n/templates/template-transformer.ts index 1b82961a601..86dbc442f32 100644 --- a/scripts/migration/i18n/templates/template-transformer.ts +++ b/scripts/migration/i18n/templates/template-transformer.ts @@ -27,13 +27,28 @@ export class TemplateTransformer { const errors: string[] = []; try { + // Use the parser to find all i18n pipe usages via AST + const usages = this.parser.findI18nPipeUsage(templateContent, filePath); + let transformedContent = templateContent; - // Transform interpolations: {{ 'key' | i18n }} -> key - transformedContent = this.transformInterpolations(transformedContent, changes); + // Process each usage found by the AST parser (reverse order to handle replacements from end to start) + for (const usage of usages.reverse()) { + if (!usage.context) { + continue; // Skip usages without context + } - // Transform attributes: [title]="'key' | i18n" -> [title]="'key'" i18n-title="@@key" - transformedContent = this.transformAttributes(transformedContent, changes); + const replacement = this.generateReplacement(usage); + transformedContent = this.replaceAtPosition(transformedContent, usage, replacement); + + changes.push({ + type: "replace", + location: { line: usage.line, column: usage.column }, + original: usage.context, + replacement, + description: `Transformed ${usage.method} usage '${usage.key}' to i18n attribute`, + }); + } return { success: true, @@ -54,124 +69,42 @@ export class TemplateTransformer { } /** - * Transform interpolation usage: {{ 'key' | i18n }} -> key + * Generate replacement text for a given i18n usage */ - private transformInterpolations( - templateContent: string, - changes: TransformationChange[], - ): string { - let transformedContent = templateContent; + private generateReplacement(usage: I18nUsage): string { + const i18nId = this.generateI18nId(usage.key); + const context = usage.context || ""; - // Pattern for string literal interpolations - const stringInterpolationPattern = - /\{\{\s*['"`]([^'"`]+)['"`]\s*\|\s*i18n(?::([^}]+))?\s*\}\}/g; - - let match; - while ((match = stringInterpolationPattern.exec(templateContent)) !== null) { - const original = match[0]; - const key = match[1]; - const i18nId = this.generateI18nId(key); - const replacement = `${key}`; - - transformedContent = transformedContent.replace(original, replacement); - - const position = this.getPositionInfo(templateContent, match.index); - changes.push({ - type: "replace", - location: position, - original, - replacement, - description: `Transformed interpolation '${key}' to i18n attribute`, - }); + if (context.startsWith("{{") && context.endsWith("}}")) { + // Interpolation: {{ 'key' | i18n }} -> key + return `${usage.key}`; + } else if (context.includes("[") && context.includes("]")) { + // Attribute binding: [title]="'key' | i18n" -> [title]="'key'" i18n-title="@@key" + const attrMatch = context.match(/\[([^\]]+)\]/); + if (attrMatch) { + const attrName = attrMatch[1]; + return `[${attrName}]="${usage.key}" i18n-${attrName}="@@${i18nId}"`; + } } - // Pattern for variable interpolations - const variableInterpolationPattern = - /\{\{\s*([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\s*\|\s*i18n(?::([^}]+))?\s*\}\}/g; - - while ((match = variableInterpolationPattern.exec(templateContent)) !== null) { - const original = match[0]; - const variable = match[1]; - const i18nId = this.generateI18nId(variable); - const replacement = `{{${variable}}}`; - - transformedContent = transformedContent.replace(original, replacement); - - const position = this.getPositionInfo(templateContent, match.index); - changes.push({ - type: "replace", - location: position, - original, - replacement, - description: `Transformed variable interpolation '${variable}' to i18n attribute`, - }); - } - - return transformedContent; + return context; // fallback } /** - * Transform attribute usage: [attr]="'key' | i18n" -> [attr]="'key'" i18n-attr="@@key" + * Replace usage at specific position in template content */ - private transformAttributes(templateContent: string, changes: TransformationChange[]): string { - let transformedContent = templateContent; - - // Pattern for attributes with i18n pipe - const attributePattern = /(\[?[\w-]+\]?)\s*=\s*["']([^"']*\|\s*i18n[^"']*)["']/g; - - let match; - while ((match = attributePattern.exec(templateContent)) !== null) { - const original = match[0]; - const attrName = match[1]; - const attrValue = match[2]; - - // Extract the key from the pipe expression - const keyMatch = attrValue.match(/['"`]([^'"`]+)['"`]\s*\|\s*i18n(?::([^"'|]+))?/); - if (keyMatch) { - const key = keyMatch[1]; - const i18nId = this.generateI18nId(key); - - // Remove brackets if present for i18n attribute - const baseAttrName = attrName.replace(/[\[\]]/g, ""); - const replacement = `${attrName}="${key}" i18n-${baseAttrName}="@@${i18nId}"`; - - transformedContent = transformedContent.replace(original, replacement); - - const position = this.getPositionInfo(templateContent, match.index); - changes.push({ - type: "replace", - location: position, - original, - replacement, - description: `Transformed attribute '${attrName}' with key '${key}' to i18n attribute`, - }); - } - - // Handle variable attributes - const variableMatch = attrValue.match( - /([a-zA-Z_$][a-zA-Z0-9_$]*(?:\.[a-zA-Z_$][a-zA-Z0-9_$]*)*)\s*\|\s*i18n(?::([^"'|]+))?/, + private replaceAtPosition(content: string, usage: I18nUsage, replacement: string): string { + // Find the exact position of the usage.context in the content and replace it + const context = usage.context || ""; + const contextIndex = content.indexOf(context); + if (contextIndex !== -1) { + return ( + content.substring(0, contextIndex) + + replacement + + content.substring(contextIndex + context.length) ); - if (variableMatch && !keyMatch) { - const variable = variableMatch[1]; - const i18nId = this.generateI18nId(variable); - - const baseAttrName = attrName.replace(/[\[\]]/g, ""); - const replacement = `${attrName}="${variable}" i18n-${baseAttrName}="@@${i18nId}"`; - - transformedContent = transformedContent.replace(original, replacement); - - const position = this.getPositionInfo(templateContent, match.index); - changes.push({ - type: "replace", - location: position, - original, - replacement, - description: `Transformed variable attribute '${attrName}' with variable '${variable}' to i18n attribute`, - }); - } } - - return transformedContent; + return content; } /** @@ -211,7 +144,7 @@ export class TemplateTransformer { const hasNoRemainingPipes = !this.parser.hasI18nPipeUsage(transformed); return hasMatchingBrackets && hasValidI18nAttributes && hasNoRemainingPipes; - } catch (error) { + } catch { return false; } } @@ -229,7 +162,10 @@ export class TemplateTransformer { * Validate that i18n attributes are properly formatted */ private validateI18nAttributes(content: string): boolean { - const i18nAttrs = content.match(/i18n(-[\w-]+)?="@@[\w-]+"/g) || []; - return i18nAttrs.every((attr) => attr.includes("@@")); + const i18nAttrs = content.match(/i18n(-[\w-]+)?="[^"]*"/g) || []; + return i18nAttrs.every((attr) => { + const valueMatch = attr.match(/="([^"]*)"/); + return valueMatch && valueMatch[1].startsWith("@@"); + }); } } diff --git a/scripts/migration/i18n/tests/typescript-migrator.test.ts b/scripts/migration/i18n/tests/typescript-migrator.test.ts index c7b99b45457..e04e25a82c2 100644 --- a/scripts/migration/i18n/tests/typescript-migrator.test.ts +++ b/scripts/migration/i18n/tests/typescript-migrator.test.ts @@ -1,6 +1,6 @@ import { Project, SourceFile } from "ts-morph"; + import { ASTTransformer } from "../typescript/ast-transformer"; -import { MigrationConfig } from "../shared/types"; describe("TypeScript Migration Tools", () => { let project: Project; @@ -56,14 +56,22 @@ describe("TypeScript Migration Tools", () => { } `; - sourceFile = project.createSourceFile("test.ts", code); - const result = transformer.transformI18nServiceCalls(sourceFile); + const expected = ` + import { I18nService } from '@bitwarden/common/platform/services/i18n.service'; - expect(result.success).toBe(true); - expect(result.changes).toHaveLength(1); // Only transformation, import kept due to constructor usage - expect(result.changes[0].replacement).toBe("$localize`loginWithDevice`"); - expect(sourceFile.getFullText()).toContain("$localize`loginWithDevice`"); - expect(sourceFile.getFullText()).toContain("I18nService"); // Import should still be there + class TestComponent { + constructor(private i18nService: I18nService) {} + + test() { + const message = $localize\`loginWithDevice\`; + } + } + `; + + sourceFile = project.createSourceFile("test.ts", code); + transformer.transformI18nServiceCalls(sourceFile); + + expect(sourceFile.getFullText().trim()).toBe(expected.trim()); }); it("should handle parameters in I18nService.t() calls", () => { @@ -75,13 +83,18 @@ describe("TypeScript Migration Tools", () => { } `; - sourceFile = project.createSourceFile("test.ts", code); - const result = transformer.transformI18nServiceCalls(sourceFile); + const expected = ` + class TestComponent { + test() { + const message = $localize\`itemsCount\${count.toString()}:param0:\`; + } + } + `; - expect(result.success).toBe(true); - expect(result.changes[0].replacement).toBe( - "$localize`itemsCount\${count.toString()}:param0:`", - ); + sourceFile = project.createSourceFile("test.ts", code); + transformer.transformI18nServiceCalls(sourceFile); + + expect(sourceFile.getFullText().trim()).toBe(expected.trim()); }); it("should handle files without I18nService usage", () => { @@ -96,11 +109,21 @@ describe("TypeScript Migration Tools", () => { } `; - sourceFile = project.createSourceFile("test.ts", code); - const result = transformer.transformI18nServiceCalls(sourceFile); + const expected = ` + import { Component } from '@angular/core'; - expect(result.success).toBe(true); - expect(result.changes).toHaveLength(0); + @Component({}) + class TestComponent { + test() { + console.log('no i18n here'); + } + } + `; + + sourceFile = project.createSourceFile("test.ts", code); + transformer.transformI18nServiceCalls(sourceFile); + + expect(sourceFile.getFullText().trim()).toBe(expected.trim()); }); it("should remove I18nService import when no longer used", () => { @@ -116,12 +139,21 @@ describe("TypeScript Migration Tools", () => { } `; - sourceFile = project.createSourceFile("test.ts", code); - const result = transformer.transformI18nServiceCalls(sourceFile); + const expected = ` + import { Component } from '@angular/core'; - expect(result.success).toBe(true); - expect(result.changes).toHaveLength(2); // One for transformation, one for import removal - expect(sourceFile.getFullText()).not.toContain("I18nService"); + @Component({}) + class TestComponent { + test() { + const message = $localize\`loginWithDevice\`; + } + } + `; + + sourceFile = project.createSourceFile("test.ts", code); + transformer.transformI18nServiceCalls(sourceFile); + + expect(sourceFile.getFullText().trim()).toBe(expected.trim()); }); }); @@ -152,20 +184,34 @@ describe("TypeScript Migration Tools", () => { } `; + const expected = ` + import { I18nService } from '@bitwarden/common/platform/services/i18n.service'; + import { Component } from '@angular/core'; + + @Component({}) + class TestComponent { + constructor(private i18nService: I18nService) {} + + getMessage() { + return $localize\`simpleMessage\`; + } + + getParameterizedMessage(count: number) { + return $localize\`itemCount\${count.toString()}:param0:\`; + } + + getMultipleMessages() { + const msg1 = $localize\`message1\`; + const msg2 = $localize\`message2\${'param'}:param0:\`; + return [msg1, msg2]; + } + } + `; + const sourceFile = project.createSourceFile("complex-test.ts", code); - const result = transformer.transformI18nServiceCalls(sourceFile); + transformer.transformI18nServiceCalls(sourceFile); - expect(result.success).toBe(true); - expect(result.changes.length).toBe(4); // 4 transformations, no import removal due to constructor - - const transformedCode = sourceFile.getFullText(); - expect(transformedCode).toContain("$localize`simpleMessage`"); - expect(transformedCode).toContain("$localize`itemCount\${count.toString()}:param0:`"); - expect(transformedCode).toContain("$localize`message1`"); - expect(transformedCode).toContain("$localize`message2\${'param'}:param0:`"); - - // Should keep the I18nService import due to constructor usage - expect(transformedCode).toContain("I18nService"); + expect(sourceFile.getFullText().trim()).toBe(expected.trim()); }); it("should remove import when only method calls are used (no constructor)", () => { @@ -180,15 +226,18 @@ describe("TypeScript Migration Tools", () => { } `; + const expected = ` + class TestComponent { + test() { + const message = $localize\`testMessage\`; + } + } + `; + const sourceFile = project.createSourceFile("no-constructor-test.ts", code); - const result = transformer.transformI18nServiceCalls(sourceFile); + transformer.transformI18nServiceCalls(sourceFile); - expect(result.success).toBe(true); - expect(result.changes.length).toBe(2); // 1 transformation + 1 import removal - - const transformedCode = sourceFile.getFullText(); - expect(transformedCode).toContain("$localize`testMessage`"); - expect(transformedCode).not.toContain("I18nService"); + expect(sourceFile.getFullText().trim()).toBe(expected.trim()); }); }); });