From 4c869d2a37d614dd58d17da06ebd4d9fcaba04f0 Mon Sep 17 00:00:00 2001 From: "Calum H." Date: Tue, 23 Jun 2026 15:43:12 +0100 Subject: [PATCH] fix: broken icu script (#6480) --- scripts/i18n-icu-contract.test.ts | 69 +++++++++++++++++++- scripts/i18n-icu-contract.ts | 101 +++++++++++++++++++++++++----- 2 files changed, 153 insertions(+), 17 deletions(-) diff --git a/scripts/i18n-icu-contract.test.ts b/scripts/i18n-icu-contract.test.ts index b6baba69d..5afd7960c 100644 --- a/scripts/i18n-icu-contract.test.ts +++ b/scripts/i18n-icu-contract.test.ts @@ -1,7 +1,12 @@ import assert from 'node:assert/strict' import { test } from 'node:test' -import { contractFromMessage, contractsEqual, sourceContractChanged } from './i18n-icu-contract' +import { + contractFromMessage, + contractsEqual, + sourceContractChanged, + translationCompatibleWithSource, +} from './i18n-icu-contract' test('same plain text contract is equal', () => { assert.equal( @@ -57,6 +62,68 @@ test('select branch changes contract', () => { ) }) +test('translation can move phrase inside ICU branches', () => { + const source = contractFromMessage( + 'In the last {amount} {unit, select, hours {{amount, plural, one {hour} other {hours}}} days {{amount, plural, one {day} other {days}}} other {days}}', + 'source', + ) + const translation = contractFromMessage( + "{unit, select, hours {{amount, plural, one {Nell'ultima ora} other {Nelle ultime # ore}}} days {{amount, plural, one {Nell'ultimo giorno} other {Negli ultimi # giorni}}} other {Negli ultimi {amount} giorni}}", + 'translation', + ) + + assert.equal(translationCompatibleWithSource(source, translation), true) +}) + +test('translation can use locale-specific plural categories', () => { + const source = contractFromMessage('{count, plural, one {# file} other {# files}}', 'source') + const translation = contractFromMessage( + '{count, plural, one {# файл} few {# файла} many {# файлов} other {# файла}}', + 'translation', + ) + + assert.equal(translationCompatibleWithSource(source, translation), true) +}) + +test('translation can simplify plural when wording does not vary', () => { + const source = contractFromMessage('{count, plural, one {# server} other {# servers}}', 'source') + const translation = contractFromMessage('{count} Server', 'translation') + + assert.equal(translationCompatibleWithSource(source, translation), true) +}) + +test('translation cannot invent app select values', () => { + const source = contractFromMessage( + '{unit, select, hours {hours} days {days} other {days}}', + 'source', + ) + const translation = contractFromMessage( + '{unit, select, years {years} other {days}}', + 'translation', + ) + + assert.equal(translationCompatibleWithSource(source, translation), false) +}) + +test('translation cannot invent variables', () => { + const source = contractFromMessage('Created by {user}', 'source') + const translation = contractFromMessage('Created by {username}', 'translation') + + assert.equal(translationCompatibleWithSource(source, translation), false) +}) + +test('source shape rewrite with same runtime interface is not a contract change', () => { + assert.equal( + sourceContractChanged( + 'In the last {amount} {unit, select, hours {{amount, plural, one {hour} other {hours}}} days {{amount, plural, one {day} other {days}}} other {days}}', + '{unit, select, hours {{amount, plural, one {In the last hour} other {In the last # hours}}} days {{amount, plural, one {In the last day} other {In the last # days}}} other {In the last {amount} days}}', + 'previous', + 'current', + ), + false, + ) +}) + test('invalid previous source message is treated as changed', () => { assert.equal( sourceContractChanged( diff --git a/scripts/i18n-icu-contract.ts b/scripts/i18n-icu-contract.ts index 661891b70..743ecfd46 100644 --- a/scripts/i18n-icu-contract.ts +++ b/scripts/i18n-icu-contract.ts @@ -10,7 +10,8 @@ import { parse as parseYaml } from 'yaml' type MessageEntry = string | { message?: string; defaultMessage?: string } type MessageFile = Record type CrowdinFileEntry = { source: string; dest?: string; translation: string } -type Contract = { args: string[]; tags: string[]; branches: string[] } +type ArgUse = 'argument' | 'number' | 'date' | 'time' | 'plural' | 'select' +type Contract = { args: Record; tags: string[]; selectBranches: Record } type Issue = { file: string; key: string; reason: string } type CrowdinListResponse = { data: Array<{ data: T }> @@ -35,42 +36,61 @@ function textOf(entry: MessageEntry | undefined): string | undefined { return entry?.message ?? entry?.defaultMessage } -function stable(items: Set) { +function stable(items: Set) { return [...items].sort() } +function stableRecord(items: Map>) { + return Object.fromEntries( + [...items.entries()] + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, values]) => [key, stable(values)]), + ) +} + export function contractFromMessage(message: string, label: string): Contract { - const args = new Set() + const args = new Map>() const tags = new Set() - const branches = new Set() + const selectBranches = new Map>() + + function addArg(name: string, use: ArgUse) { + const uses = args.get(name) ?? new Set() + uses.add(use) + args.set(name, uses) + } + + function addSelectBranch(name: string, selector: string) { + const branches = selectBranches.get(name) ?? new Set() + branches.add(selector) + selectBranches.set(name, branches) + } function visit(elements: ReturnType) { for (const element of elements) { switch (element.type) { case TYPE.argument: - args.add(`${element.value}:argument`) + addArg(element.value, 'argument') break case TYPE.number: - args.add(`${element.value}:number`) + addArg(element.value, 'number') break case TYPE.date: - args.add(`${element.value}:date`) + addArg(element.value, 'date') break case TYPE.time: - args.add(`${element.value}:time`) + addArg(element.value, 'time') break case TYPE.select: { - args.add(`${element.value}:select`) + addArg(element.value, 'select') for (const [selector, option] of Object.entries(element.options)) { - branches.add(`${element.value}:select:${selector}`) + addSelectBranch(element.value, selector) visit(option.value) } break } case TYPE.plural: { - args.add(`${element.value}:plural:${element.pluralType}`) + addArg(element.value, 'plural') for (const [selector, option] of Object.entries(element.options)) { - branches.add(`${element.value}:plural:${selector}`) visit(option.value) } break @@ -93,13 +113,58 @@ export function contractFromMessage(message: string, label: string): Contract { } } - return { args: stable(args), tags: stable(tags), branches: stable(branches) } + return { args: stableRecord(args), tags: stable(tags), selectBranches: stableRecord(selectBranches) } } export function contractsEqual(a: Contract, b: Contract) { return JSON.stringify(a) === JSON.stringify(b) } +export function translationCompatibleWithSource(source: Contract, translation: Contract) { + const sourceArgs = new Map( + Object.entries(source.args).map(([key, value]) => [key, new Set(value)]), + ) + const sourceTags = new Set(source.tags) + const sourceSelectBranches = new Map( + Object.entries(source.selectBranches).map(([key, value]) => [key, new Set(value)]), + ) + + for (const tag of translation.tags) { + if (!sourceTags.has(tag)) return false + } + + for (const tag of source.tags) { + if (!translation.tags.includes(tag)) return false + } + + for (const [arg, uses] of Object.entries(translation.args)) { + const allowedUses = sourceArgs.get(arg) + if (!allowedUses) return false + + for (const use of uses) { + if (use === 'argument') continue + + if (use === 'number' || use === 'plural') { + if (!allowedUses.has('number') && !allowedUses.has('plural')) return false + continue + } + + if (!allowedUses.has(use)) return false + } + } + + for (const [arg, branches] of Object.entries(translation.selectBranches)) { + const allowedBranches = sourceSelectBranches.get(arg) + if (!allowedBranches) return false + + for (const branch of branches) { + if (branch !== 'other' && !allowedBranches.has(branch)) return false + } + } + + return true +} + export function sourceContractChanged( previousText: string, currentText: string, @@ -110,7 +175,7 @@ export function sourceContractChanged( try { const before = contractFromMessage(previousText, previousLabel) - return !contractsEqual(before, after) + return !translationCompatibleWithSource(after, before) } catch { return true } @@ -206,10 +271,14 @@ export async function pruneLocalTranslations(options: { check: boolean; scope?: try { const translationContract = contractFromMessage(translationText, `${translationFile}:${key}`) - if (!contractsEqual(sourceContract, translationContract)) { + if (!translationCompatibleWithSource(sourceContract, translationContract)) { delete translations[key] changed = true - issues.push({ file: translationFile, key, reason: 'ICU contract differs from en-US' }) + issues.push({ + file: translationFile, + key, + reason: 'translation uses unsupported ICU variables, tags, or select branches', + }) } } catch { delete translations[key]