gh-104400: pygettext: use an AST parser instead of a tokenizer (GH-104402)

This greatly simplifies the code and fixes many corner cases.

Author: tomasr8

Lib/test/test_tools/i18n_data/docstrings.pot

@@ -15,26 +15,40 @@ msgstr ""
 "Generated-By: pygettext.py 1.5\n"
 
 
-#: docstrings.py:7
+#: docstrings.py:1
+#, docstring
+msgid "Module docstring"
+msgstr ""
+
+#: docstrings.py:9
 #, docstring
 msgid ""
 msgstr ""
 
-#: docstrings.py:18
+#: docstrings.py:15
+#, docstring
+msgid "docstring"
+msgstr ""
+
+#: docstrings.py:20
 #, docstring
 msgid ""
 "multiline\n"
-"    docstring\n"
-"    "
+"docstring"
 msgstr ""
 
-#: docstrings.py:25
+#: docstrings.py:27
 #, docstring
 msgid "docstring1"
 msgstr ""
 
-#: docstrings.py:30
+#: docstrings.py:38
+#, docstring
+msgid "nested docstring"
+msgstr ""
+
+#: docstrings.py:43
 #, docstring
-msgid "Hello, {}!"
+msgid "nested class docstring"
 msgstr ""
 

Lib/test/test_tools/i18n_data/docstrings.py

@@ -1,3 +1,5 @@
+"""Module docstring"""
+
 # Test docstring extraction
 from gettext import gettext as _
 
@@ -10,10 +12,10 @@ def test(x):
 # Leading empty line
 def test2(x):
 
-    """docstring"""  # XXX This should be extracted but isn't.
+    """docstring"""
 
 
-# XXX Multiline docstrings should be cleaned with `inspect.cleandoc`.
+# Multiline docstrings are cleaned with `inspect.cleandoc`.
 def test3(x):
     """multiline
     docstring
@@ -27,15 +29,15 @@ def test4(x):
 
 
 def test5(x):
-    """Hello, {}!""".format("world!")  # XXX This should not be extracted.
+    """Hello, {}!""".format("world!")  # This should not be extracted.
 
 
 # Nested docstrings
 def test6(x):
     def inner(y):
-        """nested docstring"""  # XXX This should be extracted but isn't.
+        """nested docstring"""
 
 
 class Outer:
     class Inner:
-        "nested class docstring"  # XXX This should be extracted but isn't.
+        "nested class docstring"

Lib/test/test_tools/i18n_data/messages.pot

@@ -19,22 +19,22 @@ msgstr ""
 msgid ""
 msgstr ""
 
-#: messages.py:19 messages.py:20
+#: messages.py:19 messages.py:20 messages.py:21
 msgid "parentheses"
 msgstr ""
 
-#: messages.py:23
+#: messages.py:24
 msgid "Hello, world!"
 msgstr ""
 
-#: messages.py:26
+#: messages.py:27
 msgid ""
 "Hello,\n"
 "    multiline!\n"
 msgstr ""
 
 #: messages.py:46 messages.py:89 messages.py:90 messages.py:93 messages.py:94
-#: messages.py:99
+#: messages.py:99 messages.py:100 messages.py:101
 msgid "foo"
 msgid_plural "foos"
 msgstr[0] ""
@@ -68,22 +68,32 @@ msgstr ""
 msgid "set"
 msgstr ""
 
-#: messages.py:63
+#: messages.py:62 messages.py:63
 msgid "nested string"
 msgstr ""
 
 #: messages.py:68
 msgid "baz"
 msgstr ""
 
+#: messages.py:71 messages.py:75
+msgid "default value"
+msgstr ""
+
 #: messages.py:91 messages.py:92 messages.py:95 messages.py:96
 msgctxt "context"
 msgid "foo"
 msgid_plural "foos"
 msgstr[0] ""
 msgstr[1] ""
 
-#: messages.py:100
+#: messages.py:102
 msgid "domain foo"
 msgstr ""
 
+#: messages.py:118 messages.py:119
+msgid "world"
+msgid_plural "worlds"
+msgstr[0] ""
+msgstr[1] ""
+

Lib/test/test_tools/i18n_data/messages.py

@@ -18,6 +18,7 @@
 # Extra parentheses
 (_("parentheses"))
 ((_("parentheses")))
+_(("parentheses"))
 
 # Multiline strings
 _("Hello, "
@@ -32,23 +33,22 @@
 _(None)
 _(1)
 _(False)
-_(("invalid"))
 _(["invalid"])
 _({"invalid"})
 _("string"[3])
 _("string"[:3])
 _({"string": "foo"})
 
 # pygettext does not allow keyword arguments, but both xgettext and pybabel do
-_(x="kwargs work!")
+_(x="kwargs are not allowed!")
 
 # Unusual, but valid arguments
 _("foo", "bar")
 _("something", x="something else")
 
 # .format()
 _("Hello, {}!").format("world")  # valid
-_("Hello, {}!".format("world"))  # invalid, but xgettext and pybabel extract the first string
+_("Hello, {}!".format("world"))  # invalid, but xgettext extracts the first string
 
 # Nested structures
 _("1"), _("2")
@@ -59,7 +59,7 @@
 
 # Nested functions and classes
 def test():
-    _("nested string")  # XXX This should be extracted but isn't.
+    _("nested string")
     [_("nested string")]
 
 
@@ -68,11 +68,11 @@ def bar(self):
         return _("baz")
 
 
-def bar(x=_('default value')):  # XXX This should be extracted but isn't.
+def bar(x=_('default value')):
     pass
 
 
-def baz(x=[_('default value')]):  # XXX This should be extracted but isn't.
+def baz(x=[_('default value')]):
     pass
 
 
@@ -97,6 +97,8 @@ def _(x="don't extract me"):
 
 # Complex arguments
 ngettext("foo", "foos", 42 + (10 - 20))
+ngettext("foo", "foos", *args)
+ngettext("foo", "foos", **kwargs)
 dgettext(["some", {"complex"}, ("argument",)], "domain foo")
 
 # Invalid calls which are not extracted
@@ -108,3 +110,10 @@ def _(x="don't extract me"):
 dngettext('domain', 'foo')
 dpgettext('domain', 'context')
 dnpgettext('domain', 'context', 'foo')
+dgettext(*args, 'foo')
+dpgettext(*args, 'context', 'foo')
+dnpgettext(*args, 'context', 'foo', 'foos')
+
+# f-strings
+f"Hello, {_('world')}!"
+f"Hello, {ngettext('world', 'worlds', 3)}!"

Lib/test/test_tools/test_i18n.py

@@ -87,7 +87,7 @@ def assert_POT_equal(self, expected, actual):
         self.maxDiff = None
         self.assertEqual(normalize_POT_file(expected), normalize_POT_file(actual))
 
-    def extract_from_str(self, module_content, *, args=(), strict=True):
+    def extract_from_str(self, module_content, *, args=(), strict=True, with_stderr=False):
         """Return all msgids extracted from module_content."""
         filename = 'test.py'
         with temp_cwd(None):
@@ -98,12 +98,18 @@ def extract_from_str(self, module_content, *, args=(), strict=True):
                 self.assertEqual(res.err, b'')
             with open('messages.pot', encoding='utf-8') as fp:
                 data = fp.read()
-        return self.get_msgids(data)
+        msgids = self.get_msgids(data)
+        if not with_stderr:
+            return msgids
+        return msgids, res.err
 
     def extract_docstrings_from_str(self, module_content):
         """Return all docstrings extracted from module_content."""
         return self.extract_from_str(module_content, args=('--docstrings',), strict=False)
 
+    def get_stderr(self, module_content):
+        return self.extract_from_str(module_content, strict=False, with_stderr=True)[1]
+
     def test_header(self):
         """Make sure the required fields are in the header, according to:
            http://www.gnu.org/software/gettext/manual/gettext.html#Header-Entry
@@ -407,6 +413,24 @@ def test_files_list(self):
             self.assertIn(f'msgid "{text2}"', data)
             self.assertNotIn(text3, data)
 
+    def test_error_messages(self):
+        """Test that pygettext outputs error messages to stderr."""
+        stderr = self.get_stderr(dedent('''\
+        _(1+2)
+        ngettext('foo')
+        dgettext(*args, 'foo')
+        '''))
+
+        # Normalize line endings on Windows
+        stderr = stderr.decode('utf-8').replace('\r', '')
+
+        self.assertEqual(
+            stderr,
+            "*** test.py:1: Expected a string constant for argument 1, got 1 + 2\n"
+            "*** test.py:2: Expected at least 2 positional argument(s) in gettext call, got 1\n"
+            "*** test.py:3: Variable positional arguments are not allowed in gettext calls\n"
+        )
+
 
 def update_POT_snapshots():
     for input_file in DATA_DIR.glob('*.py'):

Misc/NEWS.d/next/Tools-Demos/2023-05-11-23-32-25.gh-issue-104400.23vxm7.rst

@@ -0,0 +1 @@
+Fix several bugs in extraction by switching to an AST parser in :program:`pygettext`.