1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-13 15:03:26 +00:00

Fix tests

This commit is contained in:
Hinton
2025-07-28 11:04:26 +02:00
parent 9d76924a6d
commit 02505516ef
4 changed files with 151 additions and 339 deletions

View File

@@ -44,6 +44,8 @@ module.exports = {
"<rootDir>/libs/vault/jest.config.js",
"<rootDir>/libs/key-management/jest.config.js",
"<rootDir>/libs/key-management-ui/jest.config.js",
"<rootDir>/scripts/migration/i18n/jest.config.js",
],
// Workaround for a memory leak that crashes tests in CI:

View File

@@ -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
*/

View File

@@ -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 }} -> <span i18n="@@key">key</span>
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 }} -> <span i18n="@@key">key</span>
* 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 = `<span i18n="@@${i18nId}">${key}</span>`;
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 }} -> <span i18n="@@key">key</span>
return `<span i18n="@@${i18nId}">${usage.key}</span>`;
} 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 = `<span i18n="@@${i18nId}">{{${variable}}}</span>`;
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("@@");
});
}
}

View File

@@ -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());
});
});
});