mirror of
https://github.com/mattermost/mattermost.git
synced 2026-10-13 09:27:38 +08:00
Jesse Hallam
5e6f509f36
* Add deterministic i18n checkers for both surfaces
Nothing in CI reads the locale catalogs. webapp/channels has an
i18n-verify-translations script -- `formatjs verify --structural-equality` --
but no workflow calls it, and it has no extra-key check. The server has nothing
at all. So a translation that throws at format time, invents a variable the
source does not have, or carries an id en.json retired is invisible until a
user in that language hits it.
Webapp: scripts/check_icu.mjs, wired up as the existing i18n-verify-translations
script. It imports @formatjs/icu-messageformat-parser, declared as a dependency
of channels and pinned to 2.11.4 -- the copy react-intl 7.1.14 resolves -- so
"parses here" really does imply "parses there". The pin is load-bearing: three
versions coexist in the tree, and a bare import resolved to the 2.4.0 that
babel-plugin-formatjs hoists to the root, the build-time transformer's grammar,
seven minors behind the parser that actually renders the catalogs, and a dedupe
could have moved it again without notice. The header records that the pin has
to move with react-intl.
The checks are in two layers. Layer 1 is the runtime guarantee and always
fails: valid JSON, string values, key parity with en.json, every value parses,
and no variable or tag the source does not have (an unknown variable throws
MISSING_VALUE at format time; an unknown tag renders as raw markup). Layer 2 is
fidelity: a dropped variable or tag, a variable demoted to a different type --
{count, plural, ...} becoming a bare {count} still parses and renders, it just
silently stops pluralizing -- and an ASCII apostrophe before < or {, which in
ICU opens a quoted literal that swallows the tag and everything after it. That
last one still parses, so this check is the only thing that catches it.
Type parity compares the set of types each variable is used as rather than a
single type per variable. A name is routinely used in two roles at once --
"{count, number} new {count, plural, one {message} other {messages}}" -- and a
type-per-name map mis-flags every one of those.
The comparison is asymmetric, because only one direction is a defect. Demotion
loses the plural. Promotion is how a translator makes a sentence grammatical:
English needs a plural far less often than the languages it is translated into
-- "{count} items were deleted" is one sentence in English and four in Russian
-- and react-intl reads the value out of the same values object either way. A
PROMOTIONS table names the roles that are safe to widen into and everything
else stays fatal. The formatting types stay out of it: {x, number} over a
string renders NaN and {x, date} over a non-date renders "Invalid Date", so
reaching for one the source did not use is a defect rather than a grammar fix.
Server: `mmgotool i18n verify` and `make i18n-verify`. The strongest check
available there is the loader itself: go-i18n's LoadTranslationFile is what the
server runs at startup, so a file it rejects is a file that breaks the server.
Each locale loads into its own bundle rather than the package-global one, so
one file cannot mask a defect in another depending on the order they happen to
be walked in, and the checks can be run in isolation. On top of the loader, id
parity, {{.Field}} parity in both directions, and exactly the CLDR plural
categories the locale uses -- stricter than the webapp, because go-i18n rejects
a category the locale does not have.
Plural shape is compared against en.json in the collapsing direction too, since
flattening a pluralized entry into a single string still loads and still
renders and just silently stops pluralizing. The opposite direction is left
alone: whether a count reaches go-i18n is a property of the call site, not of
en.json -- the batched email title is a plain string in English and pluralized
in six locales, and email_batching.go passes len(notifications)-1 -- so a
language that has to inflect where English does not is translating correctly.
Empty plural forms are flagged as well. countEmptyItems and
removeEmptyTranslations both test the raw JSON for `""`, so every plural map
slips past them, and go-i18n builds no template for an empty form, finds none
at format time, and returns the translation id. Empty single-string
translations render that same raw id and are deliberately left alone: several
hundred already exist across es, fr and it, and `i18n clean-empty` already
removes exactly that shape.
There is no allowlist. A translation that deviates from its source is either
wrong, or the source is encoding English grammar the other languages cannot
follow -- and both of those are worth fixing rather than recording. Every key
that wanted an exemption while this was being written turned out to be the
second kind, and the four source strings responsible landed upstream in #38228,
so the corpus passes with nothing exempted. If a case ever turns up that
genuinely is a property of a target language, the mechanism is ten lines and
should be scoped to key and locale, which a real example would show how to do.
Both take a flag (--warn-missing-keys / --warn-missing-ids) that downgrades
missing entries to warnings, since key parity is a separate problem from
whether the entries that exist are correct.
Pulling in go-i18n resolves the minimum versions its go.mod requires --
pelletier/go-toml v1.2.0 and yaml.v2 v2.2.1, both with known advisories. Pinned
to the versions the server module already resolves, v1.9.5 and v2.4.0, so the
tool module and the server agree rather than each carrying its own copy.
The server tests cover each check the command makes: the runtime loader gate,
id parity in both directions and under --warn-missing-ids, template token
parity, plural shape against en.json in both directions, CLDR categories per
locale, and empty plural forms. They also pin the two behaviours the command
walk depends on, that en.json, non-JSON files and subdirectories are skipped.
Writing them turned up that the "no CLDR plural spec" branch is unreachable:
the loader derives the language with language.Parse, which only returns one
when GetPluralSpec is non-nil for the same tag, so a file that loaded always
has a spec. It is left in place as an invariant guard and labelled as one, with
the test asserting what an unrecognized filename actually does.
The webapp tests drive the real script in a subprocess, since its contract is
its exit code: 0 clean, 1 defects found, 2 misuse. They cover well-formedness,
key parity in both directions and under --warn-missing-keys, variable and tag
parity including names nested inside plural branches, the argument role rules
in both directions, and the apostrophe check with its doubled and typographic
escapes. The first test asserts that the resolved ICU parser is the version
react-intl depends on, which is the property the rest of the suite rests on and
the one that silently regressed before. It fails against a node_modules tree
installed before that pin landed; npm ci fixes it.
Both are wired into the i18n workflow, which until now only ever looked at
which files changed. The existing check-files job stays as it is -- reporting a
direct edit to a non-English catalog is still worth doing, and it answers a
different question from whether the edit is correct.
Both run with missing entries downgraded to warnings (--warn-missing-keys /
--warn-missing-ids). Key parity is a real gap, 55,164 entries on the webapp and
21,748 on the server, but it is a separate problem from whether the entries
that do exist are valid, and a hard parity gate is only fair once a contributor
adding a string has a documented way to produce the other 21 locales.
The workflow triggers on server/i18n/** and webapp/channels/src/i18n/**, and on
the checkers themselves -- check_icu.mjs, mmgotool, and the workflow files. It
is the only workflow that invokes either checker, so without those paths a PR
touching one of them never ran it and a broken checker could land green. That
matches how webapp-ci.yml already lists its own workflow and composite action.
The catalogs do not pass yet; the three commits that follow repair them.
* Fix the locale catalog defects the checkers found
Running the new checkers over the shipped catalogs turns up 2,474 webapp errors
and 1,495 server errors. They are not stylistic: 98 webapp strings fail to parse
with the parser react-intl runs, so they throw the moment a user in that
language opens the screen. vi has 36 of them, ro 18, fa 15. Another 395 name a
variable or tag the source does not have, which throws MISSING_VALUE or renders
raw markup, and on the server 57 interpolate a {{.Token}} that is not there and
render "<no value>".
Two kinds of change here, and no new translation of anything.
Repaired mechanically, where the intended text is unambiguous:
webapp 74 variable or tag the source renamed -- {fullUsers} -> {fullSeats},
{displayname} -> {displayName}, {linkApi} -> {linkAPI}. Paired
only when each unknown name has one clear counterpart.
17 an apostrophe opening an ICU quoted literal. After a letter it is
an elision, so it becomes the typographic apostrophe -- French
l'<link>; otherwise it is a quote mark and every lone apostrophe
in the string is doubled, which is the ICU escape and keeps an
opening and closing pair matched.
15 a translated variable name -- {потребителско име} for {username}.
These are MALFORMED_ARGUMENT, so the string never parsed at all.
3 a pseudo-tag the source quotes deliberately, '<blank>', copied
without the quoting and so an UNCLOSED_TAG.
server 22 a plural branch for a category the locale does not have. go-i18n
rejects those outright; pt-BR, es, fr and it carry a "many" that
its CLDR spec does not define.
14 a {{.Token}} the source renamed.
Removed, because the entry translates an English string that no longer exists:
webapp 1285 keys en.json no longer has
702 translations whose structure no longer matches the source
server 809 ids en.json no longer has
497 translations whose tokens or plural categories no longer match
Removing is the conservative option, not the destructive one. react-intl falls
back to the English defaultMessage for a key a catalog does not carry, and the
server falls back to the id; what these entries render today is a stray
'<your-mattermost-url>' literal, unrendered markdown, a missing hyperlink, or an
exception. The catalogs are already 55,164 webapp and 21,748 server entries
short of parity, so this does not change what a translator has to do next -- it
changes what a user sees in the meantime. Every removed entry is recoverable
from history if someone wants to repair rather than retranslate it.
Nothing here is exempted, because there is nothing to exempt with. Six keys
wanted an allowlist entry over the course of this work, and every one turned out
to be the same thing: an English sentence assembled from parts, forcing
translators into a shape the source did not have. The four source strings
responsible are fixed in the commit this branch is stacked on, which is why the
checker ships without an allowlist at all.
Both checkers now pass with --warn-missing-keys / --warn-missing-ids. Key parity
is a separate problem and is left as warnings.
* Fix the French batched email title plural forms
The entry used the CLDR category "many", which the go-i18n plural spec for
French does not define, and left "other" empty. French resolves any count of
two or more to "other", so every French batched email digest covering more
than one message shipped with an empty subject line.
Move the plural text into "other" and drop the unused category.
* Remove the half-translated plural entries from six locale catalogs
Eighteen entries across bg, es, fa, fr, it and ro filled "one" and left the
remaining categories empty. An empty form is worse than an absent translation:
go-i18n builds no template for it, finds none at format time, and returns the
translation id, so a Spanish user inviting two people who are already members
saw "api.command_invite.user_already_in_channel.app_error".
Deleting the entry lets the en fallback serve real English instead. No
translation is invented here -- the singular text goes away with the entry,
because a catalog that answers only for count == 1 cannot be served safely.
* Use singular agreement in the Italian and Romanian out-of-channel mention strings
Tools
This directory aims to provide a set of tools that simplify and enhance various development tasks. This README file serves as a guide to help you understand the directory, features of these tools, and how to get started using it. This is a collection of utilities and scripts designed to streamline common development tasks for Mattermost. These tools aim to help automate repetitive tasks and improve productivity.
Included tools
- mattermost-govet: custom Go vet analyzers enforcing Mattermost-specific code conventions (structured logging, error handling, SQL safety, etc.). Used by
make vetin the server. - mmgotool: is a CLI to help with i18n related checks for the mattermost/server development.
- sharedchannel-test: integration test tool that validates shared channel synchronization (posts, reactions, membership) between two real Mattermost server instances.
Installation & Usage
mmgotool
To install mmgotool, simply run the following command: go install github.com/mattermost/mattermost/tools/mmgotool
Make sure you have the necessary prerequisites such as Go compiler.
mmgotool i18n has following subcommands described below:
check: Check translationscheck-empty-src: Check for empty translation source stringsclean-empty: Clean empty translationsextract: Extract translations
sharedchannel-test
Stands up two Mattermost Enterprise instances, creates a remote cluster connection, and runs integration tests for shared channel synchronization (membership, posts, reactions).
Prerequisites:
make start-docker(Postgres and friends running)- Enterprise repo present at
../../enterprise - An enterprise license file
Usage:
# Managed mode (builds server, starts/stops both instances automatically)
cd tools/sharedchannel-test
go run . --license /path/to/license.mattermost-license --server-dir ../../server
# External mode (connect to already-running instances)
go run . --license /path/to/license.mattermost-license --manage=false \
--server-a http://localhost:9065 --server-b http://localhost:9066