diff --git a/.github/workflows/i18n-ci-pr.yml b/.github/workflows/i18n-ci-pr.yml index b896a8eaebf..c03dba327f8 100644 --- a/.github/workflows/i18n-ci-pr.yml +++ b/.github/workflows/i18n-ci-pr.yml @@ -4,10 +4,11 @@ on: paths: - "server/i18n/**" - "webapp/channels/src/i18n/**" - # The checkers themselves, so a change to one is verified by it. + - "i18n/**" - "webapp/channels/scripts/check_icu.mjs" - "webapp/channels/scripts/icu_rules.mjs" - "tools/mmgotool/**" + - "server/Makefile" - ".github/workflows/i18n-ci-pr.yml" - ".github/workflows/i18n-ci-template.yml" concurrency: diff --git a/.github/workflows/i18n-ci-template.yml b/.github/workflows/i18n-ci-template.yml index e5e0f4de04f..e904075ad99 100644 --- a/.github/workflows/i18n-ci-template.yml +++ b/.github/workflows/i18n-ci-template.yml @@ -79,30 +79,8 @@ jobs: run: | echo "::warning title=Non-English i18n files changed::This PR modifies non-English translation files directly. Ensure the changes are intentional and will not be overwritten by Weblate." - verify-webapp: - name: Verify webapp translation files - runs-on: ubuntu-24.04 - defaults: - run: - working-directory: webapp - steps: - - name: ci/checkout-repo - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - with: - persist-credentials: false - - - name: ci/setup - uses: ./.github/actions/webapp-setup - with: - read-only: "true" - - - name: ci/i18n-verify-translations - working-directory: webapp/channels - run: | - npm run i18n-verify-translations -- --warn-missing-keys - - verify-server: - name: Verify server translation files + verify: + name: Verify translations needs: buildenv runs-on: ubuntu-24.04 steps: @@ -111,11 +89,16 @@ jobs: with: persist-credentials: false + - name: ci/setup-webapp + uses: ./.github/actions/webapp-setup + with: + read-only: "true" + - name: ci/restore-go-cache uses: ./.github/actions/restore-go-cache - - name: ci/i18n-verify + - name: ci/i18n-verify-all uses: ./.github/actions/run-in-buildenv with: build-server-tag: ${{ needs.buildenv.outputs.build-server-tag }} - run: make i18n-verify + run: make i18n-verify-all diff --git a/i18n/glossary/README.md b/i18n/glossary/README.md index e3db9a5f3b3..6c32279470e 100644 --- a/i18n/glossary/README.md +++ b/i18n/glossary/README.md @@ -2,11 +2,13 @@ Key terminology for AI-assisted translation: core product terminology shared by webapp and server (mobile and desktop reuse it implicitly). -- `en.json` — term inventory: `term -> {definition, partOfSpeech, doNotTranslate, aliases?}`. -- `.json` — one file per officially supported locale: `term -> {target, note?}`. Key set matches `en.json` exactly. `target` is the term to propagate and holds nothing but the translated term itself. `note` carries inflection/usage guidance and records known inconsistencies or mistranslations in existing locale files that future passes must not propagate. +- `en.json` — term inventory: `term -> {definition, partOfSpeech, doNotTranslate, aliases?, commonWord?}`. +- `.json` — one file per officially supported locale: `term -> {target, note?}`. Key set and order match `en.json` exactly. `target` is the term to propagate and holds nothing but the translated term itself. `note` carries inflection/usage guidance and records known inconsistencies or mistranslations in existing locale files that future passes must not propagate. -Targets were derived from the majority rendering in existing locale files and the community translation rules for German, French, and Dutch. Where the majority rendering is wrong, `target` carries the correct term and `note` records what the catalogs ship today — the glossary is a correction, not a mirror. `doNotTranslate` terms keep their English form in every context, including fully translated sentences. +Targets were derived from the majority rendering in existing locale files and the community translation rules for German, French, and Dutch. Where the majority rendering is wrong, `target` carries the correct term and `note` records what the catalogs ship today — the glossary is a correction, not a mirror. `doNotTranslate` terms keep their English form in every context, including fully translated sentences, and their `target` is the term itself. + +`commonWord` marks a `doNotTranslate` name that is also an ordinary English word, such as `Channels` or `Calls`. The English reads the same whether a string names the product or just starts a sentence with the word, so the translation rules do not flag a translation that leaves such a name out. The translator decides from the description. This file set is the authority for these terms. Notes state guidance directly rather than deferring to an external source, so a translation prompt needs nothing beyond the glossary itself. -Generated as part of the AI i18n overhaul. Reference material for translation prompts — not wired into any build or runtime. +The translation rules in [`i18n/scripts/`](../scripts/) use it: a translation that leaves out a `doNotTranslate` name lands only with a stated reason. `node i18n/scripts/check_glossary.mjs` checks these files in CI. diff --git a/i18n/glossary/en.json b/i18n/glossary/en.json index e10256785d1..5fe4ce60c63 100644 --- a/i18n/glossary/en.json +++ b/i18n/glossary/en.json @@ -25,22 +25,26 @@ "Channels": { "definition": "Product name of the Mattermost messaging product; as a product name it stays in English, unlike the common noun 'channel'.", "partOfSpeech": "noun", - "doNotTranslate": true + "doNotTranslate": true, + "commonWord": true }, "Playbooks": { "definition": "Product name of the Mattermost workflow product; as a product name it stays in English.", "partOfSpeech": "noun", - "doNotTranslate": true + "doNotTranslate": true, + "commonWord": true }, "Boards": { "definition": "Product name of the Mattermost project/kanban board product; as a product name it stays in English.", "partOfSpeech": "noun", - "doNotTranslate": true + "doNotTranslate": true, + "commonWord": true }, "Calls": { "definition": "Product name of the Mattermost voice/video calling feature; as a product name it stays in English.", "partOfSpeech": "noun", - "doNotTranslate": true + "doNotTranslate": true, + "commonWord": true }, "Markdown": { "definition": "Name of the text formatting syntax used in Mattermost messages.", diff --git a/i18n/scripts/check_glossary.mjs b/i18n/scripts/check_glossary.mjs new file mode 100644 index 00000000000..06b1fbea455 --- /dev/null +++ b/i18n/scripts/check_glossary.mjs @@ -0,0 +1,205 @@ +// Check i18n/glossary/ and i18n/style.json. The translation rules read both, so +// a malformed entry would quietly change what a translation is held to. +// +// node i18n/scripts/check_glossary.mjs +// +// Run from the repository root. Exits 1 on any problem: +// +// - every file is canonical JSON: 2-space indent, trailing newline +// - en.json entries hold definition, partOfSpeech and doNotTranslate, and +// optionally aliases and commonWord, and nothing else +// - partOfSpeech is noun, verb, adjective or noun/verb +// - commonWord appears only on a doNotTranslate term +// - no form, term or alias, belongs to two terms +// - there is a .json for exactly the locales with catalogs, with the +// same keys as en.json in the same order +// - each holds a non-empty target, and optionally a note; a doNotTranslate +// term's target is the term itself +// - style.json entries are for locales with catalogs, and hold address +// (formal or informal), use and avoid, and optionally matchCase and +// skipSentenceStart; no word is in both use and avoid +import {GLOSSARY_DIR, localesOf, STYLE, SURFACES} from './lib/catalogs.mjs'; +import {workingTree} from './lib/repo.mjs'; + +const EN_FIELDS = new Set(['definition', 'partOfSpeech', 'doNotTranslate', 'aliases', 'commonWord']); +const EN_REQUIRED = ['definition', 'partOfSpeech', 'doNotTranslate']; +const PARTS_OF_SPEECH = new Set(['noun', 'verb', 'adjective', 'noun/verb']); +const LOCALE_FIELDS = new Set(['target', 'note']); +const STYLE_FIELDS = new Set(['address', 'use', 'avoid', 'matchCase', 'skipSentenceStart']); + +const tree = workingTree(process.cwd()); +const problems = []; +const problem = (file, message) => problems.push(`${file}: ${message}`); +const isText = (v) => typeof v === 'string' && v.trim() !== ''; +const isTextList = (v) => Array.isArray(v) && v.length > 0 && v.every(isText); + +function load(file) { + const text = tree.read(file); + if (text === null) { + return null; + } + let body; + try { + body = JSON.parse(text); + } catch (e) { + problem(file, `not JSON: ${e.message}`); + return null; + } + if (!body || typeof body !== 'object' || Array.isArray(body)) { + problem(file, 'not a JSON object'); + return null; + } + if (JSON.stringify(body, null, 2) + '\n' !== text) { + problem(file, 'not in canonical form; rewrite it as JSON.stringify(body, null, 2) plus a newline'); + } + return body; +} + +const enFile = `${GLOSSARY_DIR}/en.json`; +const en = load(enFile); +if (!en) { + console.error(`${enFile} is missing or unreadable`); + process.exit(1); +} + +const owner = new Map(); +for (const [term, entry] of Object.entries(en)) { + const where = `${enFile}: "${term}"`; + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { + problems.push(`${where}: not an object`); + continue; + } + for (const field of Object.keys(entry)) { + if (!EN_FIELDS.has(field)) { + problems.push(`${where}: unknown field "${field}"`); + } + } + for (const field of EN_REQUIRED) { + if (!(field in entry)) { + problems.push(`${where}: no ${field}`); + } + } + if ('definition' in entry && !isText(entry.definition)) { + problems.push(`${where}: definition is empty`); + } + if ('partOfSpeech' in entry && !PARTS_OF_SPEECH.has(entry.partOfSpeech)) { + problems.push(`${where}: partOfSpeech is ${JSON.stringify(entry.partOfSpeech)}, not one of ${[...PARTS_OF_SPEECH].join(', ')}`); + } + if ('doNotTranslate' in entry && typeof entry.doNotTranslate !== 'boolean') { + problems.push(`${where}: doNotTranslate is not true or false`); + } + if ('commonWord' in entry && entry.commonWord !== true) { + problems.push(`${where}: commonWord is only ever true; leave it out otherwise`); + } + if (entry.commonWord && !entry.doNotTranslate) { + problems.push(`${where}: commonWord is for doNotTranslate terms only`); + } + if ('aliases' in entry && !isTextList(entry.aliases)) { + problems.push(`${where}: aliases is not a non-empty list of non-empty strings`); + } + for (const form of [term, ...(isTextList(entry.aliases) ? entry.aliases : [])]) { + if (owner.has(form) && owner.get(form) !== term) { + problems.push(`${where}: "${form}" is also "${owner.get(form)}"`); + } else if (owner.has(form)) { + problems.push(`${where}: "${form}" is listed twice`); + } + owner.set(form, term); + } +} + +const catalogLocales = new Set(Object.keys(SURFACES).flatMap((surface) => localesOf(tree, surface))); +const glossaryLocales = new Set(tree.list(GLOSSARY_DIR).filter((f) => f.endsWith('.json') && f !== 'en.json').map((f) => f.slice(0, -'.json'.length))); +for (const locale of catalogLocales) { + if (!glossaryLocales.has(locale)) { + problem(`${GLOSSARY_DIR}/${locale}.json`, 'missing; every locale with a catalog has one'); + } +} + +const terms = Object.keys(en); +for (const locale of [...glossaryLocales].sort()) { + const file = `${GLOSSARY_DIR}/${locale}.json`; + if (!catalogLocales.has(locale)) { + problem(file, 'no catalog has this locale'); + } + const body = load(file); + if (!body) { + continue; + } + const keys = Object.keys(body); + const missing = terms.filter((t) => !(t in body)); + const extra = keys.filter((t) => !(t in en)); + if (missing.length) { + problem(file, `missing ${missing.map((t) => `"${t}"`).join(', ')}`); + } + if (extra.length) { + problem(file, `not in en.json: ${extra.map((t) => `"${t}"`).join(', ')}`); + } + if (!missing.length && !extra.length && keys.join('\n') !== terms.join('\n')) { + problem(file, 'terms are not in en.json order'); + } + for (const [term, entry] of Object.entries(body)) { + const where = `${file}: "${term}"`; + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { + problems.push(`${where}: not an object`); + continue; + } + for (const field of Object.keys(entry)) { + if (!LOCALE_FIELDS.has(field)) { + problems.push(`${where}: unknown field "${field}"`); + } + } + if (!isText(entry.target)) { + problems.push(`${where}: no target`); + } else if (en[term]?.doNotTranslate && entry.target !== term) { + problems.push(`${where}: a doNotTranslate term's target is the term itself, not "${entry.target}"`); + } + if ('note' in entry && !isText(entry.note)) { + problems.push(`${where}: note is empty; leave it out instead`); + } + } +} + +const style = load(STYLE) || {}; +for (const [locale, entry] of Object.entries(style)) { + const where = `${STYLE}: ${locale}`; + if (!catalogLocales.has(locale)) { + problems.push(`${where}: no catalog has this locale`); + } + if (!entry || typeof entry !== 'object' || Array.isArray(entry)) { + problems.push(`${where}: not an object`); + continue; + } + for (const field of Object.keys(entry)) { + if (!STYLE_FIELDS.has(field)) { + problems.push(`${where}: unknown field "${field}"`); + } + } + if (entry.address !== 'formal' && entry.address !== 'informal') { + problems.push(`${where}: address is formal or informal`); + } + for (const field of ['use', 'avoid']) { + if (!isTextList(entry[field])) { + problems.push(`${where}: ${field} is not a non-empty list of words`); + } + } + for (const field of ['matchCase', 'skipSentenceStart']) { + if (field in entry && entry[field] !== true) { + problems.push(`${where}: ${field} is only ever true; leave it out otherwise`); + } + } + if (isTextList(entry.use) && isTextList(entry.avoid)) { + const fold = (w) => (entry.matchCase ? w : w.toLowerCase()); + const use = new Set(entry.use.map(fold)); + const both = entry.avoid.filter((w) => use.has(fold(w))); + if (both.length) { + problems.push(`${where}: ${both.join(', ')} in both use and avoid`); + } + } +} + +if (problems.length) { + console.error(problems.join('\n')); + console.error(`\n${problems.length} problem(s)`); + process.exit(1); +} +console.log(`glossary: ${terms.length} terms, ${glossaryLocales.size} locales; style: ${Object.keys(style).length} locales`); diff --git a/i18n/scripts/check_glossary.test.mjs b/i18n/scripts/check_glossary.test.mjs new file mode 100644 index 00000000000..294f5627e9e --- /dev/null +++ b/i18n/scripts/check_glossary.test.mjs @@ -0,0 +1,79 @@ +import assert from 'node:assert/strict'; +import {spawnSync} from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import {afterEach, beforeEach, describe, test} from 'node:test'; +import {fileURLToPath} from 'node:url'; + +const script = path.join(path.dirname(fileURLToPath(import.meta.url)), 'check_glossary.mjs'); +const json = (body) => JSON.stringify(body, null, 2) + '\n'; + +describe('check_glossary.mjs', () => { + let root; + const write = (rel, text) => { + fs.mkdirSync(path.dirname(path.join(root, rel)), {recursive: true}); + fs.writeFileSync(path.join(root, rel), text); + }; + const run = () => spawnSync(process.execPath, [script], {cwd: root, encoding: 'utf8'}); + + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'check-glossary-')); + for (const locale of ['en', 'de', 'ru']) { + write(`webapp/channels/src/i18n/${locale}.json`, json({})); + } + write('server/i18n/en.json', json([])); + write('server/i18n/ru.json', json([])); + write('i18n/glossary/en.json', json({ + Mattermost: {definition: 'The product.', partOfSpeech: 'noun', doNotTranslate: true}, + channel: {definition: 'A place to talk.', partOfSpeech: 'noun', doNotTranslate: false}, + })); + write('i18n/glossary/de.json', json({Mattermost: {target: 'Mattermost'}, channel: {target: 'Kanal'}})); + write('i18n/glossary/ru.json', json({Mattermost: {target: 'Mattermost'}, channel: {target: 'канал', note: 'Masculine.'}})); + write('i18n/style.json', json({de: {address: 'informal', use: ['du'], avoid: ['Sie'], matchCase: true}})); + }); + afterEach(() => fs.rmSync(root, {recursive: true, force: true})); + + test('passes consistent files', () => { + const r = run(); + assert.equal(r.status, 0, r.stderr); + assert.match(r.stdout, /glossary: 2 terms, 2 locales; style: 1 locales/); + }); + + test('reports each kind of problem', () => { + write('i18n/glossary/en.json', json({ + Mattermost: {definition: 'The product.', partOfSpeech: 'noun', doNotTranslate: true}, + Boards: {definition: 'A product.', partOfSpeech: 'thing', doNotTranslate: false, commonWord: true, aliases: ['Mattermost']}, + })); + write('i18n/glossary/ru.json', json({Mattermost: {target: 'Маттермост'}})); + write('i18n/glossary/de.json', '{"Boards": {"target": "Boards"}, "Mattermost": {"target": "Mattermost", "extra": 1}}'); + write('i18n/glossary/fr.json', json({})); + write('i18n/style.json', json({ru: {address: 'polite', use: ['вы'], avoid: ['вы']}})); + + const r = run(); + assert.equal(r.status, 1); + for (const expected of [ + /partOfSpeech is "thing"/, + /commonWord is for doNotTranslate terms only/, + /"Boards": "Mattermost" is also "Mattermost"/, + /ru\.json: missing "Boards"/, + /ru\.json: "Mattermost": a doNotTranslate term's target is the term itself/, + /de\.json: not in canonical form/, + /de\.json: terms are not in en\.json order/, + /de\.json: "Mattermost": unknown field "extra"/, + /fr\.json: no catalog has this locale/, + /style\.json: ru: address is formal or informal/, + /style\.json: ru: вы in both use and avoid/, + ]) { + assert.match(r.stderr, expected); + } + }); + + test('requires a glossary file for every locale with a catalog', () => { + fs.rmSync(path.join(root, 'i18n/glossary/de.json')); + + const r = run(); + assert.equal(r.status, 1); + assert.match(r.stderr, /i18n\/glossary\/de\.json: missing; every locale with a catalog has one/); + }); +}); diff --git a/i18n/scripts/lib/catalogs.mjs b/i18n/scripts/lib/catalogs.mjs new file mode 100644 index 00000000000..0ce6dd470d9 --- /dev/null +++ b/i18n/scripts/lib/catalogs.mjs @@ -0,0 +1,26 @@ +// The two catalog surfaces. +// +// webapp webapp/channels/src/i18n/.json {"": ""}, keys +// in code-point order. en.json is {"": {defaultMessage, description}}. +// server server/i18n/.json [{"id", "translation"}], in no particular +// order; a translation is a string or a map of plural forms. en.json +// entries also carry a description. +export const SURFACES = { + webapp: {dir: 'webapp/channels/src/i18n'}, + server: {dir: 'server/i18n'}, +}; + +export const SOURCE = 'en.json'; +export const GLOSSARY_DIR = 'i18n/glossary'; +export const STYLE = 'i18n/style.json'; + +export function localesOf(tree, surface) { + return tree.list(SURFACES[surface].dir). + filter((f) => f.endsWith('.json') && f !== SOURCE). + map((f) => f.slice(0, -'.json'.length)). + sort(); +} + +export function flatten(value) { + return value && typeof value === 'object' ? Object.values(value).join('\n') : String(value ?? ''); +} diff --git a/i18n/scripts/lib/glossary.mjs b/i18n/scripts/lib/glossary.mjs new file mode 100644 index 00000000000..b795e2527bc --- /dev/null +++ b/i18n/scripts/lib/glossary.mjs @@ -0,0 +1,90 @@ +// i18n/glossary/: en.json is the term inventory, and .json holds each +// term's target in that locale. See i18n/glossary/README.md. +// +// i18n/style.json: how each locale's catalogs address the user, formally or +// informally (address), the pronouns that say so (use), and the pronouns that +// would mean the other (avoid). matchCase matches avoid only as written, for a +// formal pronoun that is an ordinary one capitalized: German "Sie" is "you", +// "sie" is "she" or "they". +import {flatten, GLOSSARY_DIR, STYLE} from './catalogs.mjs'; + +const escape = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +const wordPattern = (text, flags) => new RegExp(`(? { + const file = `${GLOSSARY_DIR}/${name}.json`; + const text = tree.read(file); + return text === null ? {} : parseObject(text, file); + }; + const terms = read('en'); + const targets = read(locale); + return Object.entries(terms).map(([term, entry]) => ({ + term, + ...entry, + aliases: entry.aliases || [], + target: targets[term]?.target, + note: targets[term]?.note, + + // A doNotTranslate name is matched exactly as written: "Mattermost", not + // "mattermost.com". Everything else is ordinary vocabulary, which English + // capitalizes at the start of a sentence and in title case. + patterns: [term, ...(entry.aliases || [])].map((form) => ({form, re: wordPattern(form, entry.doNotTranslate ? '' : 'i')})), + })); +} + +/** + * A locale's style.json entry, with avoid compiled into one pattern, or null. + * skipSentenceStart is for languages whose formal pronoun is also an ordinary + * one at the start of a sentence: German "Sie" is "you" mid-sentence, but can be + * "they" first. + */ +export function readStyle(tree, locale) { + const text = tree.read(STYLE); + const entry = text === null ? null : parseObject(text, STYLE)[locale]; + if (!entry) { + return null; + } + const start = entry.skipSentenceStart ? '(? entry.isFile()). + map((entry) => entry.name). + sort(); + } catch (e) { + if (e.code === 'ENOENT') { + return []; + } + throw e; + } + }, + }; +} diff --git a/i18n/scripts/lib/rules.mjs b/i18n/scripts/lib/rules.mjs new file mode 100644 index 00000000000..3d922b1cc44 --- /dev/null +++ b/i18n/scripts/lib/rules.mjs @@ -0,0 +1,239 @@ +// The rules a new or changed translation has to pass before it lands. +// +// Webapp messages go through webapp/channels/scripts/icu_rules.mjs, the same code +// check_icu.mjs runs in CI. Server messages get the checks mmgotool i18n verify +// makes per string, so that a translation fails here, key by key, before its +// catalog is written. +// +// New work is held to more than the catalogs as they stand. Every finding has a +// severity: +// +// error never lands +// soft lands only with a stated reason, because it is usually a mistake but +// sometimes exactly right: a product name that stays in English, a +// sentence that leaves a brand name implicit +import fs from 'fs'; +import path from 'path'; +import {fileURLToPath, pathToFileURL} from 'url'; + +import {flatten} from './catalogs.mjs'; +import {droppedNames} from './glossary.mjs'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const icu = await import(pathToFileURL(path.join(here, '../../../webapp/channels/scripts/icu_rules.mjs')).href); +const SERVER_PLURALS = JSON.parse(fs.readFileSync(path.join(here, 'server_plurals.json'), 'utf8')); + +const {analyzeSource, checkMessage, isRightToLeft, HORIZONTAL_GLYPH} = icu; + +// What CI reports as a warning, new work must not do at all, except an arrow in a +// right-to-left locale, which can be the clearest way to write a sequence. +const WEBAPP_SEVERITY = { + 'plural-category': 'error', + 'plural-unused-category': 'error', + 'rtl-arrow': 'soft', +}; + +const finding = (rule, severity, message) => ({rule, severity, message}); + +function literalText(ast, out = []) { + for (const el of ast) { + if (el.type === 0) { + out.push(el.value); + } + if (el.options) { + for (const option of Object.values(el.options)) { + literalText(option.value, out); + } + } + if (el.children) { + literalText(el.children, out); + } + } + return out; +} + +function proseOf(value, surface) { + let text; + if (surface === 'webapp') { + try { + text = literalText(icu.parse(value, icu.PARSE_OPTIONS)).join(' '); + } catch { + text = value; + } + } else { + text = flatten(value).replace(/\{\{[^}]*\}\}/g, ' '); + } + return text.replace(/[a-z][a-z0-9+.-]*:\/\/\S*|\S+@\S+\.\S+/gi, ' '); +} + +const WORD = /(? w.replace(/[._/'’-]+$/u, '')); + return tokens.filter((w) => ( + w.length > 1 && + !CODE_LITERALS.has(w) && + !/\d/.test(w) && + !/[._/]/.test(w) && + !/^\p{Lu}{2,}s?$/u.test(w) && + !/\p{Ll}\p{Lu}/u.test(w) + )); +} + +// A Latin-script language can share a word or two with English, "Status" or +// "Server", and be right. A language in any other script almost never can. +const isLatinScript = (locale) => new Intl.Locale(locale).maximize().script === 'Latn'; +const isEnglish = (locale) => new Intl.Locale(locale).language === 'en'; +const unquoted = (value) => (typeof value === 'string' ? value.replace(QUOTED, ' ') : Object.fromEntries(Object.entries(value).map(([k, v]) => [k, v.replace(QUOTED, ' ')]))); + +function addressFindings(value, locale, surface, style) { + if (!style) { + return []; + } + const found = [...new Set([...proseOf(unquoted(value), surface).matchAll(style.avoidRe)].map((m) => m[1]))]; + return found.length ? [finding('address', 'soft', `${found.map((w) => `"${w}"`).join(', ')}: ${locale} addresses the user ${style.address}ly (${style.use.join(', ')})`)] : []; +} + +function softChecks(item, value, locale, surface, glossary, style) { + const out = []; + if (!isEnglish(locale) && flatten(value) === flatten(item.english)) { + // A name is capitalised; three ordinary words are a sentence nobody translated. + const words = translatableWords(item.english, surface, glossary); + const severity = words.filter((w) => w === w.toLowerCase()).length >= 3 ? 'error' : 'soft'; + if (words.length >= (isLatinScript(locale) ? 2 : 1)) { + out.push(finding('identical-to-english', severity, `identical to the English, which has words to translate (${words.slice(0, 4).join(' ')})`)); + } + } else if (!isEnglish(locale) && !isLatinScript(locale)) { + // Lowercase words only: a capitalised one is usually a name, and a quoted + // one a value the user types. + const source = new Set(translatableWords(unquoted(item.english), surface, glossary).filter((w) => w === w.toLowerCase())); + const kept = [...new Set(translatableWords(unquoted(value), surface, glossary).filter((w) => source.has(w)))]; + if (kept.length) { + out.push(finding('english-words', 'soft', `keeps English words from the source: ${kept.slice(0, 6).join(', ')}`)); + } + } + out.push(...addressFindings(value, locale, surface, style)); + const dropped = droppedNames(glossary, item.english, value); + if (dropped.length) { + out.push(finding('do-not-translate', 'soft', `leaves out ${dropped.map((d) => `"${d}"`).join(', ')}, which the glossary keeps in English`)); + } + return out; +} + +const TOKEN = /\{\{\s*\.([A-Za-z0-9_]+)\s*\}\}/g; +const ACTION = /\{\{(.*?)\}\}/g; +const tokensOf = (text) => new Set([...text.matchAll(TOKEN)].map((m) => m[1])); +const actionsOf = (text) => [...text.matchAll(ACTION)]. + map((m) => m[1].trim().replace(/\s+/g, ' ')). + filter((a) => !/^\.[A-Za-z0-9_]+$/.test(a)). + sort(); + +function checkServer(item, value, locale) { + const out = []; + const forms = typeof value === 'string' ? [value] : Object.values(value); + const sourceForms = typeof item.english === 'string' ? [item.english] : Object.values(item.english); + + for (const form of forms) { + if ((form.match(/\{\{/g) || []).length !== (form.match(/\}\}/g) || []).length) { + out.push(finding('template-syntax', 'error', 'unbalanced {{ and }}')); + break; + } + } + + const sourceTokens = new Set(sourceForms.flatMap((f) => [...tokensOf(f)])); + const tokens = new Set(forms.flatMap((f) => [...tokensOf(f)])); + for (const t of tokens) { + if (!sourceTokens.has(t)) { + out.push(finding('unknown-token', 'error', `unknown template token {{.${t}}} not present in source`)); + } + } + for (const t of sourceTokens) { + if (!tokens.has(t)) { + out.push(finding('missing-token', 'error', `source template token {{.${t}}} is missing from the translation`)); + } + } + + // {{if}}, {{else}} and {{end}} have to survive translation as they are, in + // every form, or the template either fails to parse or shows the wrong branch. + const sourceActions = actionsOf(sourceForms[0]).join('\n'); + if (forms.some((f) => actionsOf(f).join('\n') !== sourceActions)) { + out.push(finding('template-action', 'error', `template actions differ from the source's: ${sourceActions ? sourceActions.split('\n').map((a) => `{{${a}}}`).join(' ') : 'none'}`)); + } + + if (typeof value === 'object') { + const want = SERVER_PLURALS[locale]; + const have = Object.keys(value).sort(); + if (!want) { + out.push(finding('plural-category', 'error', `no go-i18n plural categories recorded for ${locale} in i18n/scripts/lib/server_plurals.json`)); + } else if (have.join('/') !== want.join('/')) { + out.push(finding('plural-category', 'error', `plural forms are ${have.join('/')}, but go-i18n uses exactly ${want.join('/')} for ${locale}`)); + } + } else if (typeof item.english === 'object') { + out.push(finding('plural-demoted', 'error', 'en.json pluralises this id but the translation is a single string, which silently stops pluralising')); + } + + if (isRightToLeft(locale) && forms.some((f) => HORIZONTAL_GLYPH.test(f))) { + out.push(finding('rtl-arrow', 'soft', 'horizontal arrow in a right-to-left locale points the opposite way to the reading direction')); + } + return out; +} + +const analyzed = new Map(); +function sourceFor(english) { + if (!analyzed.has(english)) { + analyzed.set(english, analyzeSource(english)); + } + return analyzed.get(english); +} + +/** + * Check one translation of item, {english, description}, where english is the + * en.json message, against the locale's glossary and style from readGlossary + * and readStyle. Returns findings of {rule, severity, message}. + */ +export function checkTranslation({item, value, locale, surface, glossary, style = null}) { + const isPluralMap = surface === 'server' && value && typeof value === 'object' && !Array.isArray(value) && + Object.values(value).every((form) => typeof form === 'string'); + if (typeof value !== 'string' && !isPluralMap) { + return [finding('type', 'error', surface === 'server' ? 'not a string or a map of plural forms' : 'not a string')]; + } + if (flatten(value).trim() === '' || (isPluralMap && Object.values(value).some((f) => f.trim() === ''))) { + return [finding('empty', 'error', 'empty')]; + } + + const out = []; + if (/[\uFFFD\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/.test(flatten(value))) { + out.push(finding('encoding', 'error', 'contains a replacement character (U+FFFD) or a control character, the mark of text that went through the wrong encoding')); + } + if (surface === 'webapp') { + let source; + try { + source = sourceFor(item.english); + } catch (e) { + return [finding('source', 'error', `the English does not parse, so fix en.json first: ${e.message}`)]; + } + for (const f of checkMessage(source, value, locale)) { + out.push({...f, severity: WEBAPP_SEVERITY[f.rule] || f.severity}); + } + } else { + out.push(...checkServer(item, value, locale)); + } + out.push(...softChecks(item, value, locale, surface, glossary, style)); + return out; +} diff --git a/i18n/scripts/lib/server_plurals.json b/i18n/scripts/lib/server_plurals.json new file mode 100644 index 00000000000..94e996628e2 --- /dev/null +++ b/i18n/scripts/lib/server_plurals.json @@ -0,0 +1,30 @@ +{ + "bg": ["one", "other"], + "cs": ["few", "many", "one", "other"], + "da": ["one", "other"], + "de": ["one", "other"], + "en-AU": ["one", "other"], + "es": ["one", "other"], + "fa": ["one", "other"], + "fi": ["one", "other"], + "fr": ["one", "other"], + "hr": ["few", "one", "other"], + "hu": ["one", "other"], + "it": ["one", "other"], + "ja": ["other"], + "ko": ["other"], + "lt": ["few", "many", "one", "other"], + "nb-NO": ["one", "other"], + "nl": ["one", "other"], + "pl": ["few", "many", "one", "other"], + "pt": ["one", "other"], + "pt-BR": ["one", "other"], + "ro": ["few", "one", "other"], + "ru": ["few", "many", "one", "other"], + "sv": ["one", "other"], + "tr": ["one", "other"], + "uk": ["few", "many", "one", "other"], + "vi": ["other"], + "zh-CN": ["other"], + "zh-TW": ["other"] +} diff --git a/i18n/scripts/rules.test.mjs b/i18n/scripts/rules.test.mjs new file mode 100644 index 00000000000..90666735127 --- /dev/null +++ b/i18n/scripts/rules.test.mjs @@ -0,0 +1,136 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import {describe, test} from 'node:test'; +import {fileURLToPath} from 'node:url'; + +import {readGlossary, readStyle} from './lib/glossary.mjs'; +import {checkTranslation} from './lib/rules.mjs'; + +const root = path.join(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const files = (contents) => ({read: (rel) => (rel in contents ? JSON.stringify(contents[rel]) : null)}); +const repoTree = {read: (rel) => (fs.existsSync(path.join(root, rel)) ? fs.readFileSync(path.join(root, rel), 'utf8') : null)}; + +const fixtureGlossary = (locale) => readGlossary(files({ + 'i18n/glossary/en.json': { + Mattermost: {definition: 'The product.', partOfSpeech: 'noun', doNotTranslate: true, aliases: ['MM']}, + Boards: {definition: 'A product.', partOfSpeech: 'noun', doNotTranslate: true, commonWord: true}, + channel: {definition: 'A place to talk.', partOfSpeech: 'noun', doNotTranslate: false}, + }, +}), locale); + +const check = (locale, english, value, surface = 'webapp', glossary = fixtureGlossary(locale)) => checkTranslation({ + item: {english, description: ''}, + value, + locale, + surface, + glossary, + style: readStyle(repoTree, locale), +}).map((f) => `${f.severity} ${f.rule}`); + +describe('rules', () => { + test('a translation is a non-empty string, or on the server a map of plural forms', () => { + assert.deepEqual(check('de', 'Save', 5), ['error type']); + assert.deepEqual(check('de', 'Save', {one: 'x'}), ['error type']); + assert.deepEqual(check('de', 'Save', {one: 'x', other: 1}, 'server'), ['error type']); + assert.deepEqual(check('de', 'Save', ' '), ['error empty']); + assert.deepEqual(check('de', {one: '{{.Count}} item', other: '{{.Count}} items'}, {one: '{{.Count}} Element', other: ''}, 'server'), ['error empty']); + assert.deepEqual(check('de', 'Save', 'Speichern'), []); + }); + + test('broken encoding is an error', () => { + assert.deepEqual(check('de', 'Settings', 'Einstellun\uFFFDen'), ['error encoding']); + assert.deepEqual(check('de', 'Settings', 'Einstellungen\u0007', 'server'), ['error encoding']); + }); + + test('webapp messages get the ICU rules, with what CI tolerates as a warning an error', () => { + assert.deepEqual(check('de', 'Hello {name}', 'Hallo'), ['error missing-argument']); + assert.deepEqual(check('ru', '{count} items', '{count, plural, one {# элемент} other {# элемента}}'), ['error plural-category']); + assert.deepEqual(check('ja', '{count, plural, one {# item} other {# items}}', '{count, plural, one {# 件} other {# 件}}'), ['error plural-unused-category']); + assert.deepEqual(check('ja', '{count, plural, one {# item} other {# items}}', '{count, plural, other {# 件}}'), []); + }); + + test('an arrow in a right-to-left locale is soft, since it can be the clearest way to write a sequence', () => { + assert.deepEqual(check('fa', 'Next →', 'بعدی →'), ['soft rtl-arrow']); + assert.deepEqual(check('fa', 'Next → {{.Name}}', 'بعدی → {{.Name}}', 'server'), ['soft rtl-arrow']); + }); + + test('an English message that does not parse is reported against en.json', () => { + assert.deepEqual(check('de', 'Hello {name', 'Hallo {name}'), ['error source']); + }); + + test('server template tokens and actions survive translation', () => { + assert.deepEqual(check('de', 'Hello {{.Name}}', 'Hallo {{.Nmae}}', 'server'), ['error unknown-token', 'error missing-token']); + assert.deepEqual(check('de', 'Hello {{.Name}}', 'Hallo {{.Name}', 'server'), ['error template-syntax', 'error missing-token']); + assert.deepEqual(check('de', '{{if .On}}On{{else}}Off{{end}}', '{{if .On}}An{{end}}', 'server'), ['error template-action']); + assert.deepEqual(check('de', '{{if .On}}On{{else}}Off{{end}}', '{{if .On}}An{{else}}Aus{{end}}', 'server'), []); + }); + + test('server plural forms follow go-i18n, not CLDR', () => { + const english = {one: '{{.Count}} item', other: '{{.Count}} items'}; + assert.deepEqual(check('es', english, {one: '{{.Count}} elemento', other: '{{.Count}} elementos'}, 'server'), []); + assert.deepEqual(check('es', english, {one: '{{.Count}} elemento', many: '{{.Count}} elementos', other: '{{.Count}} elementos'}, 'server'), ['error plural-category']); + assert.deepEqual(check('xx', english, {one: '{{.Count}} x', other: '{{.Count}} x'}, 'server'), ['error plural-category']); + assert.deepEqual(check('ru', english, '{{.Count}} элементов', 'server'), ['error plural-demoted']); + }); + + test('English left as it is', () => { + assert.deepEqual(check('ru', 'With CEL you can define conditions', 'With CEL you can define conditions'), ['error identical-to-english']); + assert.deepEqual(check('pl', 'Confirm sending notifications to groups', 'Confirm sending notifications to groups'), ['error identical-to-english']); + assert.deepEqual(check('ru', 'Azure Blob Storage', 'Azure Blob Storage'), ['soft identical-to-english']); + assert.deepEqual(check('de', 'Save changes', 'Save changes'), ['soft identical-to-english']); + assert.deepEqual(check('de', 'Status', 'Status'), []); + assert.deepEqual(check('fr', '[on|off|status] [@username]', '[on|off|status] [@username]', 'server'), []); + assert.deepEqual(check('en-AU', 'Save your changes now', 'Save your changes now'), []); + }); + + test('a glossary name is not English left untranslated', () => { + assert.deepEqual(check('ru', 'Mattermost', 'Mattermost'), []); + assert.deepEqual(check('de', 'Mattermost Boards', 'Mattermost Boards'), []); + assert.deepEqual(check('ja', 'Open Mattermost', 'Open Mattermost'), ['soft identical-to-english']); + }); + + test('English words left in a non-Latin script', () => { + assert.deepEqual(check('ru', '{count, plural, one {team} other {teams}}', '{count, plural, one {team} few {teams} many {teams} other {teams}}'), ['soft english-words']); + assert.deepEqual(check('ru', 'Open the {name} webhook settings', 'Откройте настройки вебхука {name}'), []); + assert.deepEqual(check('ru', 'Upload a .png file from https://example.com', 'Загрузите файл .png с https://example.com'), []); + assert.deepEqual(check('ru', 'Type /header to set it', 'Введите /header, чтобы задать его'), []); + assert.deepEqual(check('ru', 'Type "yes" to confirm', 'Введите "yes" для подтверждения'), []); + }); + + test('form of address', () => { + assert.deepEqual(check('de', 'Enter a number', 'Geben Sie eine Zahl ein'), ['soft address']); + assert.deepEqual(check('de', 'They agreed', 'Sie stimmten zu'), []); + assert.deepEqual(check('de', 'Enter a number', 'Gib eine Zahl ein'), []); + assert.deepEqual(check('ru', 'Your license', 'Твоя лицензия', 'server'), ['soft address']); + assert.deepEqual(check('fr', 'You can do it', 'Tu peux le faire'), ['soft address']); + assert.deepEqual(check('fr', 'Type "tu" to continue', 'Saisissez "tu" pour continuer'), []); + assert.deepEqual(check('ja', 'You can do it', 'できます'), []); + }); + + test('a glossary or style file that is not a JSON object is an error, not an empty one', () => { + for (const text of ['[]', '5', 'null', '{"Mattermost":']) { + const tree = {read: () => text}; + assert.throws(() => readGlossary(tree, 'de'), /^Error: i18n\/glossary\/en\.json: /); + assert.throws(() => readStyle(tree, 'de'), /^Error: i18n\/style\.json: /); + } + assert.deepEqual(readGlossary(files({}), 'de'), []); + assert.equal(readStyle(files({}), 'de'), null); + }); + + test('leaving out a doNotTranslate name is soft', () => { + assert.deepEqual(check('de', 'Open Mattermost', 'Öffne die App'), ['soft do-not-translate']); + assert.deepEqual(check('de', 'Open MM', 'Öffne die App', 'server'), ['soft do-not-translate']); + assert.deepEqual(check('de', 'Open Mattermost', 'Mattermost öffnen'), []); + assert.deepEqual(check('de', 'Visit mattermost.com', 'Besuche unsere Website'), []); + assert.deepEqual(check('de', 'Open the channel', 'Öffne den Kanal'), []); + }); + + test('a doNotTranslate name that is also a common word may be left out', () => { + assert.deepEqual(check('de', 'Boards are shared', 'Tafeln werden geteilt'), []); + + const real = readGlossary(repoTree, 'de'); + assert.deepEqual(check('de', 'Channels are listed here', 'Kanäle werden hier aufgelistet', 'webapp', real), []); + assert.deepEqual(check('de', 'Ask Mattermost support', 'Frag den Support', 'webapp', real), ['soft do-not-translate']); + }); +}); diff --git a/i18n/style.json b/i18n/style.json new file mode 100644 index 00000000000..d11671a31f4 --- /dev/null +++ b/i18n/style.json @@ -0,0 +1,199 @@ +{ + "da": { + "address": "informal", + "use": [ + "du", + "dig", + "din", + "dit", + "dine" + ], + "avoid": [ + "De", + "Dem", + "Deres" + ], + "matchCase": true, + "skipSentenceStart": true + }, + "de": { + "address": "informal", + "use": [ + "du", + "dich", + "dir", + "dein" + ], + "avoid": [ + "Sie", + "Ihnen", + "Ihr", + "Ihre", + "Ihren", + "Ihrem", + "Ihrer", + "Ihres" + ], + "matchCase": true, + "skipSentenceStart": true + }, + "es": { + "address": "informal", + "use": [ + "tú", + "te", + "tu", + "tus" + ], + "avoid": [ + "usted", + "ustedes" + ] + }, + "fr": { + "address": "formal", + "use": [ + "vous", + "votre", + "vos" + ], + "avoid": [ + "tu", + "toi", + "ton", + "tes", + "te" + ] + }, + "it": { + "address": "informal", + "use": [ + "tu", + "ti", + "tuo", + "tua" + ], + "avoid": [ + "Lei", + "Suo", + "Sua", + "Suoi", + "Sue" + ], + "matchCase": true, + "skipSentenceStart": true + }, + "nb-NO": { + "address": "informal", + "use": [ + "du", + "deg", + "din", + "ditt", + "dine" + ], + "avoid": [ + "De", + "Dem", + "Deres" + ], + "matchCase": true, + "skipSentenceStart": true + }, + "nl": { + "address": "informal", + "use": [ + "je", + "jij", + "jouw", + "jou" + ], + "avoid": [ + "u", + "uw" + ] + }, + "pl": { + "address": "informal", + "use": [ + "ty", + "twój", + "ciebie" + ], + "avoid": [ + "Pan", + "Pani", + "Państwo", + "Pana", + "Panu" + ], + "matchCase": true, + "skipSentenceStart": true + }, + "ru": { + "address": "formal", + "use": [ + "вы", + "вас", + "вам", + "ваш" + ], + "avoid": [ + "ты", + "тебя", + "тебе", + "тобой", + "твой", + "твоя", + "твоё", + "твое", + "твои", + "твоего", + "твоей", + "твоих", + "твоим", + "твоими" + ] + }, + "sv": { + "address": "informal", + "use": [ + "du", + "dig", + "din", + "ditt", + "dina" + ], + "avoid": [ + "Ni", + "Er", + "Era", + "Ert" + ], + "matchCase": true, + "skipSentenceStart": true + }, + "uk": { + "address": "formal", + "use": [ + "ви", + "вас", + "вам", + "ваш" + ], + "avoid": [ + "ти", + "тебе", + "тобі", + "тобою", + "твій", + "твоя", + "твоє", + "твої", + "твого", + "твоєї", + "твоїх", + "твоїм" + ] + } +} diff --git a/server/Makefile b/server/Makefile index d78bda36b1b..5ad7a189348 100644 --- a/server/Makefile +++ b/server/Makefile @@ -360,6 +360,14 @@ i18n-verify: ## Verify the non-English translation files against en.json cd ../tools/mmgotool && $(GO) install . $(GOBIN)/mmgotool i18n verify --warn-missing-ids +.PHONY: i18n-verify-all +i18n-verify-all: i18n-verify ## Run every translation check: both surfaces' catalogs, the glossary and the translation pipeline + cd ../tools/mmgotool && $(GO) test ./commands/ -run TestServerPluralTableMatchesGoI18n + @test -d $(BUILD_WEBAPP_DIR)/node_modules || { echo "$(BUILD_WEBAPP_DIR)/node_modules is missing; run make -C $(BUILD_WEBAPP_DIR) node_modules"; exit 1; } + cd $(BUILD_WEBAPP_DIR)/channels && npm run i18n-verify-translations -- --warn-missing-keys + cd .. && node --test i18n/scripts/*.test.mjs + cd .. && node i18n/scripts/check_glossary.mjs + .PHONY: i18n-check i18n-check: ## Exit on empty translation strings and translation source strings cd ../tools/mmgotool && $(GO) install . diff --git a/tools/mmgotool/commands/i18n_verify_test.go b/tools/mmgotool/commands/i18n_verify_test.go index 717c3237e48..c2b0107ae66 100644 --- a/tools/mmgotool/commands/i18n_verify_test.go +++ b/tools/mmgotool/commands/i18n_verify_test.go @@ -467,3 +467,34 @@ func TestRemoveEmptyTranslations(t *testing.T) { } assert.Equal(t, []string{"text", "plural"}, ids) } + +// The translation pipeline checks server plural maps in Node, against its own +// copy of go-i18n's table, i18n/scripts/lib/server_plurals.json. The copy has +// to say what go-i18n says for every server locale. +func TestServerPluralTableMatchesGoI18n(t *testing.T) { + root := filepath.Join("..", "..", "..") + raw, err := os.ReadFile(filepath.Join(root, "i18n", "scripts", "lib", "server_plurals.json")) + require.NoError(t, err) + var table map[string][]string + require.NoError(t, json.Unmarshal(raw, &table)) + + files, err := filepath.Glob(filepath.Join(root, "server", "i18n", "*.json")) + require.NoError(t, err) + locales := map[string]bool{} + for _, f := range files { + locale := strings.TrimSuffix(filepath.Base(f), ".json") + if locale == "en" { + continue + } + locales[locale] = true + + var want []string + for c := range pluralCategories(locale) { + want = append(want, string(c)) + } + assert.ElementsMatch(t, want, table[locale], "server_plurals.json: %s", locale) + } + for locale := range table { + assert.True(t, locales[locale], "server_plurals.json: %s has no server catalog", locale) + } +} diff --git a/webapp/channels/scripts/check_icu.mjs b/webapp/channels/scripts/check_icu.mjs index e9d2fc4f7de..ba56c825f1e 100644 --- a/webapp/channels/scripts/check_icu.mjs +++ b/webapp/channels/scripts/check_icu.mjs @@ -10,7 +10,7 @@ * grammar the other languages cannot follow, and both are worth fixing rather * than recording. * - * - the file is valid JSON and every value is a string + * - the file can be read, is a JSON object, and every value is a string * - the file is in canonical form: keys in code-point order, a 2-space * indent and a trailing newline, so a hand edit changes only what it means to * - no value is empty or whitespace-only. react-intl replaces '' with the @@ -34,9 +34,10 @@ * or variable and everything after it. The message still parses, so this * check is the only thing that catches it. * - * The per-message checks live in icu_rules.mjs. Two of its rules are warnings - * rather than errors, because the catalogs already ship strings that break - * them: + * The per-message checks live in icu_rules.mjs, which the translation rules in + * i18n/scripts/lib/rules.mjs run too, holding new translations to them as + * errors. Two of them are warnings here, because the catalogs already ship + * strings that break them: * * - a horizontal arrow in a right-to-left locale, which points backwards * - a plural that shows its count but lacks a form the locale needs for @@ -95,12 +96,15 @@ if (localeNames.length === 0) { // en.json pairs each key with a {defaultMessage, description}; the locale // catalogs are flat key -> string. Flatten the source to the same shape so // everything below compares one string against another. +const isObject = (value) => value !== null && typeof value === 'object' && !Array.isArray(value); + let en; try { - en = Object.fromEntries( - Object.entries(JSON.parse(readCatalog(SOURCE))). - map(([key, entry]) => [key, entry.defaultMessage]), - ); + const source = JSON.parse(readCatalog(SOURCE)); + if (!isObject(source)) { + throw new Error('not a JSON object of key -> {defaultMessage, description}'); + } + en = Object.fromEntries(Object.entries(source).map(([key, entry]) => [key, entry.defaultMessage])); } catch (e) { console.error(`cannot read ${path.join(i18nDir, SOURCE)}: ${e.message}`); process.exit(2); @@ -118,6 +122,10 @@ for (const [key, message] of Object.entries(en)) { } } +// plural-unused-category is left to the translation pipeline: an arm the +// locale never selects is dead text, not something a user sees. +const REPORTED_WARNINGS = new Set(['rtl-arrow', 'plural-category']); + for (const name of localeNames) { // The plural and direction rules come from Intl, which throws on a name // that is not a locale tag, such as en_US. @@ -129,14 +137,25 @@ for (const name of localeNames) { continue; } + let text; + try { + text = readCatalog(name); + } catch (e) { + errors.push(`${name}: cannot read: ${e.message}`); + continue; + } + let data; - const text = readCatalog(name); try { data = JSON.parse(text); } catch (e) { errors.push(`${name}: invalid JSON: ${e.message}`); continue; } + if (!isObject(data)) { + errors.push(`${name}: not a JSON object of key -> message`); + continue; + } // Keys in code-point order, as every catalog already is, so that a hand edit // cannot reorder or reindent a file and bury its one real change in the diff. @@ -189,8 +208,12 @@ for (const name of localeNames) { continue; } - for (const {severity, message: detail} of checkMessage(source, message, locale)) { - (severity === 'error' ? errors : warnings).push(`${name}:${key}: ${detail}`); + for (const {rule, severity, message: detail} of checkMessage(source, message, locale)) { + if (severity === 'error') { + errors.push(`${name}:${key}: ${detail}`); + } else if (REPORTED_WARNINGS.has(rule)) { + warnings.push(`${name}:${key}: ${detail}`); + } } } } diff --git a/webapp/channels/scripts/check_icu.test.js b/webapp/channels/scripts/check_icu.test.js index 751ce3f01fd..b641e873db6 100644 --- a/webapp/channels/scripts/check_icu.test.js +++ b/webapp/channels/scripts/check_icu.test.js @@ -130,6 +130,34 @@ describe('check_icu', () => { expect(stderr).toContain('invalid JSON'); }); + test.each([ + ['null', 'null\n'], + ['a number', '5\n'], + ['a string', '"Bonjour"\n'], + ['an array', '[]\n'], + ])('reports a catalog that is %s, and checks the rest', (_label, text) => { + const {code, stderr} = check({'a.b': 'Hello {name}'}, {'de.json': text, 'fr.json': {'a.b': 'Bonjour'}}); + + expect(code).toBe(1); + expect(stderr).toContain('de.json: not a JSON object of key -> message'); + expect(stderr).toContain('fr.json:a.b:'); + }); + + // Root reads a file whatever its mode. + (process.getuid?.() === 0 ? test.skip : test)('reports a catalog it cannot read, and checks the rest', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'check-icu-')); + fs.writeFileSync(path.join(dir, 'en.json'), JSON.stringify({'a.b': {defaultMessage: 'Hello {name}'}})); + fs.writeFileSync(path.join(dir, 'de.json'), '{\n "a.b": "Hallo {name}"\n}\n'); + fs.chmodSync(path.join(dir, 'de.json'), 0); + fs.writeFileSync(path.join(dir, 'fr.json'), '{\n "a.b": "Bonjour"\n}\n'); + + const result = spawnSync(process.execPath, [SCRIPT, dir], {encoding: 'utf8'}); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('de.json: cannot read: EACCES'); + expect(result.stderr).toContain('fr.json:a.b:'); + }); + test.each([ ['null', null], ['a number', 5], @@ -490,6 +518,11 @@ describe('check_icu', () => { const malformed = spawnSync(process.execPath, [SCRIPT, dir], {encoding: 'utf8'}); expect(malformed.status).toBe(2); expect(malformed.stderr).toContain('cannot read'); + + fs.writeFileSync(path.join(dir, 'en.json'), '[]'); + const notObject = spawnSync(process.execPath, [SCRIPT, dir], {encoding: 'utf8'}); + expect(notObject.status).toBe(2); + expect(notObject.stderr).toContain('not a JSON object'); }); test('exits 2 when the directory holds no locale catalogs', () => { diff --git a/webapp/channels/scripts/icu_rules.mjs b/webapp/channels/scripts/icu_rules.mjs index bbbdce4c787..2523ee6f4b3 100644 --- a/webapp/channels/scripts/icu_rules.mjs +++ b/webapp/channels/scripts/icu_rules.mjs @@ -2,11 +2,15 @@ // See LICENSE.txt for license information. /** - * The per-message rules for the webapp locale catalogs, which check_icu.mjs - * runs over every catalog in CI. + * The per-message rules for the webapp locale catalogs. * - * Every finding carries a stable rule id and a severity, so a caller can tell - * the rules apart without matching on message text. + * check_icu.mjs runs them over every catalog in CI, and the translation pipeline + * in i18n/scripts/ runs them over each translation before it lands. There is one + * copy so that what the pipeline accepts is exactly what CI accepts. + * + * Every finding carries a stable rule id, and its severity here is the one CI + * uses. The pipeline holds new translations to a stricter standard than the + * catalogs as they stand, so it maps some warnings to errors by rule id. */ import {createRequire} from 'node:module'; @@ -14,12 +18,12 @@ import {pathToFileURL} from 'node:url'; // Resolved through react-intl: a bare import gets whichever copy npm hoisted. const reactIntlRequire = createRequire(import.meta.resolve('react-intl')); -const {parse} = await import(pathToFileURL(reactIntlRequire.resolve('@formatjs/icu-messageformat-parser')).href); +export const {parse} = await import(pathToFileURL(reactIntlRequire.resolve('@formatjs/icu-messageformat-parser')).href); // ignoreTag: false parses ... as tags rather than literal text, so the // tag checks see them; requiresOtherClause: true rejects a plural or select with // no "other" arm, which ICU requires as the fallback. -const PARSE_OPTIONS = {ignoreTag: false, requiresOtherClause: true}; +export const PARSE_OPTIONS = {ignoreTag: false, requiresOtherClause: true}; // Element type ids from @formatjs/icu-messageformat-parser's TYPE enum. They // are inlined as numbers in the AST, so name them for legible messages. @@ -66,9 +70,9 @@ const UNESCAPED_APOSTROPHE = /(? ({rule, severity, message}); * {rule, severity, message}, where severity is "error" or "warning". * * errors parse, unknown-argument, missing-argument, argument-role, apostrophe - * warnings rtl-arrow, plural-category + * warnings rtl-arrow, plural-category, plural-unused-category */ export function checkMessage(source, message, locale) { const findings = []; @@ -260,6 +264,7 @@ export function checkMessage(source, message, locale) { // ICU falls back to "other" for a category the message lacks, so this // renders, just in the wrong grammatical form: "5 каналы" for "5 каналов". + const used = new Intl.PluralRules(locale).resolvedOptions().pluralCategories; for (const node of pluralNodes(ast)) { if (!source.incompletePlurals.has(node.value) && displaysCount(node, ast)) { const missing = missingCategories(node, locale); @@ -269,6 +274,12 @@ export function checkMessage(source, message, locale) { findings.push(finding('plural-category', 'warning', `plural "${node.value}" has no ${forms} form, so counts such as ${counts} render with "other"`)); } } + if ((node.pluralType || 'cardinal') === 'cardinal') { + const unused = Object.keys(node.options).filter((c) => !c.startsWith('=') && !used.includes(c)); + if (unused.length) { + findings.push(finding('plural-unused-category', 'warning', `plural "${node.value}" has a ${unused.join('/')} form that ${locale} never selects${unused.includes('one') ? '; for exactly one, write =1' : ''}`)); + } + } } // Two plurals on one variable fail the same way; say it once.