From bd39b0dc8fa18e259d607ae50b0a8a99d984e7c1 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Thu, 24 Sep 2026 21:24:52 +0200 Subject: [PATCH] fix(i18n): stop a percent sign in prose from making a catalog uncompilable xgettext counts a space as a format flag, so the prose `95% confidence` parses as the directive `% c` and the entry is marked python-format. Every translation moves the word behind the percent sign, msgfmt then reads `% V` and refuses the catalog. Quoted Jinja blocks and percent-encoded URLs fall into the same trap. A run over the documentation catalogs left 312 such entries in 19 files, none of which could be compiled to .mo. Removing the flag when writing does nothing: babel derives it from the msgid on every load, so the value in the file is decoration. gettext's own switch for this is no-python-format, which babel does not touch, and the writer now sets it on entries the derivation flags but that carry no real placeholder. The derived flag has to go at the same time, because gettext reads the flag line in order and `no-python-format, python-format` cancels itself out. What counts as a real placeholder was already spelled out in PROTECTED, so it moves into PRINTF and is used from both places rather than written twice. The repository lint gains the check that would have caught this. It compares the msgid against itself first and reports only when the translation alone fails, so a source string that mixes positional and named placeholders, which gettext accepts, does not raise a finding. Co-Authored-By: Claude Opus 5 (1M context) --- tests/lint/repository/test_i18n_catalogs.py | 29 +++++++ .../utils/i18n/test_no_python_format.py | 76 +++++++++++++++++++ utils/i18n/catalog.py | 5 ++ utils/i18n/placeholders.py | 18 ++++- 4 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 tests/unit/python/utils/i18n/test_no_python_format.py diff --git a/tests/lint/repository/test_i18n_catalogs.py b/tests/lint/repository/test_i18n_catalogs.py index b266dc5861..6b7ae4d69e 100644 --- a/tests/lint/repository/test_i18n_catalogs.py +++ b/tests/lint/repository/test_i18n_catalogs.py @@ -1,6 +1,9 @@ import re import unittest +from babel.messages.catalog import Message, TranslationError +from babel.messages.checkers import python_format + from utils.cache.files import PROJECT_ROOT from utils.i18n.catalog import LOCALE_DIR, catalog_path, read_catalog from utils.i18n.extract import core_messages @@ -34,6 +37,32 @@ class TestI18nCatalogs(unittest.TestCase): missing = sorted(set(domain_languages(self.languages, "core")) - set(self.core)) self.assertEqual(missing, [], "run `make i18n-extract domain=core`") + @staticmethod + def _rejects(catalog, message, string): + try: + python_format(catalog, Message(message.id, string, flags=message.flags)) + except TranslationError: + return True + return False + + def test_every_catalog_compiles(self): + broken = {} + for code, domain, catalog in self.every: + for message in catalog: + if "no-python-format" in message.flags or not message.string: + continue + if self._rejects(catalog, message, message.id): + continue + if self._rejects(catalog, message, message.string): + broken[f"{domain}/{code}"] = broken.get(f"{domain}/{code}", 0) + 1 + self.assertEqual( + broken, + {}, + f"{sum(broken.values())} translations carry placeholders msgfmt " + f"rejects, so these catalogs do not compile: {broken}. Empty them " + "with `make i18n-prune`, then redo them with `make i18n-translate`.", + ) + def test_no_catalog_for_an_unknown_language(self): present = { path.name for path in (PROJECT_ROOT / LOCALE_DIR).iterdir() if path.is_dir() diff --git a/tests/unit/python/utils/i18n/test_no_python_format.py b/tests/unit/python/utils/i18n/test_no_python_format.py new file mode 100644 index 0000000000..c8ba1ada49 --- /dev/null +++ b/tests/unit/python/utils/i18n/test_no_python_format.py @@ -0,0 +1,76 @@ +"""A percent sign in prose must not make a catalog uncompilable. + +xgettext counts a space as a format flag, so ``95% confidence`` parses as the +directive ``% c``. Every translation moves the following word, msgfmt then reads +``% V`` and refuses the catalog. The writer marks such entries +``no-python-format``; the negative cases below are shapes taken from the +documentation catalogs. +""" + +from __future__ import annotations + +import unittest + +from babel.messages.catalog import Catalog + +from utils.i18n.catalog import render +from utils.i18n.placeholders import carries_placeholder + + +class TestCarriesPlaceholder(unittest.TestCase): + def test_a_bare_conversion(self) -> None: + self.assertTrue(carries_placeholder("deployed %s")) + + def test_a_named_conversion(self) -> None: + self.assertTrue(carries_placeholder("deployed %(app)s")) + + def test_an_integer_conversion(self) -> None: + self.assertTrue(carries_placeholder("%d apps")) + + def test_any_plural_form_is_enough(self) -> None: + self.assertTrue(carries_placeholder(("one app", "%d apps"))) + + def test_a_percentage_in_prose(self) -> None: + self.assertFalse(carries_placeholder("reach at least 95% confidence")) + + def test_a_quoted_jinja_block(self) -> None: + self.assertFalse(carries_placeholder("wrap it in `{% if enabled %}`")) + + def test_a_percent_encoded_url(self) -> None: + self.assertFalse(carries_placeholder("see /repo%2FCONTRIBUTING.md")) + + def test_a_plural_of_prose(self) -> None: + self.assertFalse(carries_placeholder(("one at 95% load", "many"))) + + +class TestRenderSuppressesTheFalseFlag(unittest.TestCase): + def _flags(self, msgid: str, msgstr: str) -> set[str]: + catalog = Catalog(locale="de") + catalog.add(msgid, msgstr) + render(catalog) + return next(m.flags for m in catalog if m.id == msgid) + + def test_prose_loses_the_derived_flag(self) -> None: + flags = self._flags("reach at least 95% confidence", "95% Vertrauen") + + self.assertIn("no-python-format", flags) + self.assertNotIn("python-format", flags) + + def test_a_real_placeholder_keeps_its_flag(self) -> None: + flags = self._flags("deployed %s", "%s ausgerollt") + + self.assertIn("python-format", flags) + self.assertNotIn("no-python-format", flags) + + def test_the_written_catalog_carries_only_the_suppression(self) -> None: + catalog = Catalog(locale="de") + catalog.add("reach at least 95% confidence", "95% Vertrauen") + + written = render(catalog).decode("utf-8") + + self.assertIn("#, no-python-format\n", written) + self.assertNotIn(", python-format", written) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/i18n/catalog.py b/utils/i18n/catalog.py index 501c33f4ee..4736c234d2 100644 --- a/utils/i18n/catalog.py +++ b/utils/i18n/catalog.py @@ -10,6 +10,7 @@ from typing import TYPE_CHECKING from babel.messages.catalog import Catalog from babel.messages.pofile import read_po, write_po +from utils.i18n.placeholders import carries_placeholder from utils.software import ( SOFTWARE_AUTHOR, SOFTWARE_CONTACT, @@ -117,6 +118,10 @@ def render(catalog: Catalog) -> bytes: catalog.msgid_bugs_address = SOFTWARE_CONTACT catalog.last_translator = CONTACT catalog.language_team = CONTACT + for message in catalog: + if "python-format" in message.flags and not carries_placeholder(message.id): + message.flags.discard("python-format") + message.flags.add("no-python-format") buffer = io.BytesIO() write_po(buffer, catalog, width=0, sort_output=True, ignore_obsolete=True) return buffer.getvalue() diff --git a/utils/i18n/placeholders.py b/utils/i18n/placeholders.py index 4d322e295f..cdee785a3a 100644 --- a/utils/i18n/placeholders.py +++ b/utils/i18n/placeholders.py @@ -7,6 +7,9 @@ import re from collections import Counter from dataclasses import dataclass +PRINTF = r"%\([A-Za-z_]\w*\)[sdif]|%[sdif]" +PLACEHOLDER = re.compile(PRINTF) + PROTECTED = re.compile( r"``.+?``" r"|:[\w.+-]+:`[^`]+`" @@ -15,8 +18,7 @@ PROTECTED = re.compile( r"|\{%.+?%\}" r"|\{\{.*" r"|\{[A-Za-z_]\w*\}" - r"|%\([A-Za-z_]\w*\)[sdif]" - r"|%[sdif]" + rf"|{PRINTF}" r"|https?://(?:[^\s<>\"'`()\[\]]|\([^\s<>\"'`()]*\))+" r"|<[^<>\s]+>" r"|(?<=\])\((?:[^\s()]|\([^\s()]*\))+\)" @@ -28,6 +30,18 @@ PROTECTED = re.compile( r"|\d+(?:[.,]\d+)*" r"|\*\*|\*|`|\[|\]|\{|\}|\(|\)|\"" ) + + +def carries_placeholder(message_id: str | tuple[str, ...]) -> bool: + """Return whether ``message_id`` holds a printf placeholder a translation must keep. + + Args: + message_id: a catalog message id, singular or a plural tuple. + """ + forms = message_id if isinstance(message_id, tuple) else (message_id,) + return any(PLACEHOLDER.search(form) for form in forms) + + MARKUP = '[]`*{}()"' EMPHASIS = re.compile(r"(\*\*)[ \t]*([^\s]|[^\s].*?[^\s])[ \t]*\1", re.DOTALL)