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)