Repository: python-babel/babel
Pull request #915 (2022-10-31 11:14:25)

Use `ast` instead of `eval` for string extraction
This is safer (as we don't actually execute anything), and allows us to parse f-strings too.

Closes #769 (supersedes it)
Refs #715 (doesn't add an error yet, but doesn't crash on f-strings)

codecov[bot]: # [Codecov](https://codecov.io/gh/python-babel/babel/pull/915?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) Report
> Merging [#915](https://codecov.io/gh/python-babel/babel/pull/915?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) (4393f17) into [master](https://codecov.io/gh/python-babel/babel/commit/c7d04e8cb7bd60ff305d8bc6df2ce8a3f92c0223?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) (c7d04e8) will **decrease** coverage by `1.08%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #915      +/-   ##
==========================================
- Coverage   91.35%   90.26%   -1.09%     
==========================================
  Files          21       21              
  Lines        4186     4192       +6     
==========================================
- Hits         3824     3784      -40     
- Misses        362      408      +46     
```


| [Impacted Files](https://codecov.io/gh/python-babel/babel/pull/915?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | Coverage Δ | |
|---|---|---|
| [babel/messages/extract.py](https://codecov.io/gh/python-babel/babel/pull/915/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbWVzc2FnZXMvZXh0cmFjdC5weQ==) | `94.98% <100.00%> (+0.11%)` | :arrow_up: |
| [babel/localtime/\_win32.py](https://codecov.io/gh/python-babel/babel/pull/915/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWx0aW1lL193aW4zMi5weQ==) | `21.56% <0.00%> (-39.22%)` | :arrow_down: |
| [babel/support.py](https://codecov.io/gh/python-babel/babel/pull/915/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvc3VwcG9ydC5weQ==) | `76.63% <0.00%> (-8.20%)` | :arrow_down: |
| [babel/localtime/\_\_init\_\_.py](https://codecov.io/gh/python-babel/babel/pull/915/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWx0aW1lL19faW5pdF9fLnB5) | `61.53% <0.00%> (-2.57%)` | :arrow_down: |
| [babel/localedata.py](https://codecov.io/gh/python-babel/babel/pull/915/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWxlZGF0YS5weQ==) | `95.86% <0.00%> (-0.83%)` | :arrow_down: |
| [babel/messages/frontend.py](https://codecov.io/gh/python-babel/babel/pull/915/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbWVzc2FnZXMvZnJvbnRlbmQucHk=) | `87.17% <0.00%> (-0.75%)` | :arrow_down: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel)

akx: I've requested a couple of extra reviews off-GitHub from colleagues and such, and I suppose this is good to go :)

We'll get limited f-string support and less `eval`.


Patch:
diff --git a/babel/messages/extract.py b/babel/messages/extract.py
index c95f1cb..74e57a1 100644
--- a/babel/messages/extract.py
+++ b/babel/messages/extract.py
@@ -15,7 +15,7 @@
     :copyright: (c) 2013-2022 by the Babel Team.
     :license: BSD, see LICENSE for more details.
 """
-
+import ast
 import os
 from os.path import relpath
 import sys
@@ -487,14 +487,9 @@ def extract_python(fileobj, keywords, comment_tags, options):
                 if nested:
                     funcname = value
             elif tok == STRING:
-                # Unwrap quotes in a safe manner, maintaining the string's
-                # encoding
-                # https://sourceforge.net/tracker/?func=detail&atid=355470&
-                # aid=617979&group_id=5470
-                code = compile('# coding=%s\n%s' % (str(encoding), value),
-                               '<string>', 'eval', future_flags)
-                value = eval(code, {'__builtins__': {}}, {})
-                buf.append(value)
+                val = _parse_python_string(value, encoding, future_flags)
+                if val is not None:
+                    buf.append(val)
             elif tok == OP and value == ',':
                 if buf:
                     messages.append(''.join(buf))
@@ -516,6 +511,28 @@ def extract_python(fileobj, keywords, comment_tags, options):
             funcname = value
 
 
+def _parse_python_string(value, encoding, future_flags):
+    # Unwrap quotes in a safe manner, maintaining the string's encoding
+    # https://sourceforge.net/tracker/?func=detail&atid=355470&aid=617979&group_id=5470
+    code = compile(
+        f'# coding={str(encoding)}\n{value}',
+        '<string>',
+        'eval',
+        ast.PyCF_ONLY_AST | future_flags,
+    )
+    if isinstance(code, ast.Expression):
+        body = code.body
+        if isinstance(body, ast.Str):
+            return body.s
+        if isinstance(body, ast.JoinedStr):  # f-string
+            if all(isinstance(node, ast.Str) for node in body.values):
+                return ''.join(node.s for node in body.values)
+            if all(isinstance(node, ast.Constant) for node in body.values):
+                return ''.join(str(node.value) for node in body.values)
+            # TODO: we could raise an error or warning when not all nodes are constants
+    return None
+
+
 def extract_javascript(fileobj, keywords, comment_tags, options):
     """Extract messages from JavaScript source code.
 


Test patch:
diff --git a/tests/messages/test_extract.py b/tests/messages/test_extract.py
index 47fe306..3873191 100644
--- a/tests/messages/test_extract.py
+++ b/tests/messages/test_extract.py
@@ -528,3 +528,30 @@ nbsp = _('\xa0')
         messages = list(extract.extract('python', buf,
                                         extract.DEFAULT_KEYWORDS, [], {}))
         assert messages[0][1] == u'\xa0'
+
+    def test_f_strings(self):
+        buf = BytesIO(br"""
+t1 = _('foobar')
+t2 = _(f'spameggs' f'feast')  # should be extracted; constant parts only
+t2 = _(f'spameggs' 'kerroshampurilainen')  # should be extracted (mixing f with no f)
+t3 = _(f'''whoa! a '''  # should be extracted (continues on following lines)
+f'flying shark'
+    '... hello'
+)
+t4 = _(f'spameggs {t1}')  # should not be extracted
+""")
+        messages = list(extract.extract('python', buf, extract.DEFAULT_KEYWORDS, [], {}))
+        assert len(messages) == 4
+        assert messages[0][1] == u'foobar'
+        assert messages[1][1] == u'spameggsfeast'
+        assert messages[2][1] == u'spameggskerroshampurilainen'
+        assert messages[3][1] == u'whoa! a flying shark... hello'
+
+    def test_f_strings_non_utf8(self):
+        buf = BytesIO(b"""
+# -- coding: latin-1 --
+t2 = _(f'\xe5\xe4\xf6' f'\xc5\xc4\xd6')
+""")
+        messages = list(extract.extract('python', buf, extract.DEFAULT_KEYWORDS, [], {}))
+        assert len(messages) == 1
+        assert messages[0][1] == u'åäöÅÄÖ'