Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pybabel should raise an error when it encounters f-strings #715

Open
fabiommendes opened this issue Jun 4, 2020 · 3 comments
Open

Pybabel should raise an error when it encounters f-strings #715

fabiommendes opened this issue Jun 4, 2020 · 3 comments

Comments

@fabiommendes
Copy link

I mistakenly put something like that in my code:

_(f"Name: {x}")

and Babel fails (as in it crashes with an exception) to extract strings from this file. The reason why it crashes is that the extract_python function relies on eval(token_code) to obtain the string from its source code representation. While this is legitimate in general, it does not work for f-strings (evaluation will almost certainly crash due to missing variables).

I understand that including f-strings inside the _() function is a mistake, I think Babel should warn the user about the incorrect usage instead of simply crashing. This can be easily fixed by simply wrapping the offending eval() call into a try/except block:

# extract_python() in babel.messages.extract.py
            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)
                try:
                    value = eval(code, {'__builtins__': {}}, {})
                except NameError:
                    continue
                else:
                    if PY2 and not isinstance(value, text_type):
                        value = value.decode(encoding)
                    buf.append(value)

It still accepts f-strings with no interpolation (which is a bad style, but do not cause any damage).

@evonbevern
Copy link

Hi, I can send in a pull request for this. Will be done in the next few days.

@iranzo
Copy link

iranzo commented Jul 13, 2021

I've recently falled into this one, as per python 3.9 I was moving to using fstrings and for one of the files it fails with error. With the previous version of the file, using regular "%s" substitutions it worked, but when running pyupgrade over the code and getting some fstrings instead, it started failing.

It would also be great to identify the line that caused the failure to better trace and debug those

@Dreamsorcerer
Copy link

I've recently falled into this one, as per python 3.9 I was moving to using fstrings and for one of the files it fails with error.

Just to clarify, while it should probably produce a reasonable warning, rather than an exception, the problem in your code is the fact that you were formatting a string before sending it for translation.

i.e. _("Hello, {name}".format(name="Joe")) will try to lookup a translation for the string "Hello, Joe", which will obviously fail, unless you've added a translation for every single user individually..
Instead you need to format a string which has already been translated like _("Hello, {name}").format(name="Joe"). This looks up the translation for "Hello, {name}", which will be found, and then formats the resulting string.
If done correctly, pyupgrade would be unable to convert to an f-string.

phargogh added a commit to phargogh/invest that referenced this issue Oct 21, 2022
Since f-strings are rendered at import time, pybabel needs some extra
contextual information from the string's location in order to import the
string properly.  This is a known issue in pybabel: python-babel/babel#715
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Oct 31, 2022
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)
akx added a commit that referenced this issue Nov 1, 2022
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)
almet added a commit to spiral-project/ihatemoney that referenced this issue Dec 10, 2022
almet added a commit to spiral-project/ihatemoney that referenced this issue Dec 10, 2022
almet added a commit to spiral-project/ihatemoney that referenced this issue Dec 11, 2022
Inspired by code from @a-angeles13, thanks to him!

Add .format() fixes needed for pybabel.

See python-babel/babel#715
zorun pushed a commit to zorun/ihatemoney that referenced this issue Jan 31, 2023
F-strings are a bad idea for translations, because they cause Babel to
crash when collecting strings to translate:
python-babel/babel#715

But even if we replaced f-strings with new-style string interpolation such
as `_("{foo}").format(foo=foo)`, it's still a bad idea, because a wrong
translation can crash Ihatemoney at runtime with a KeyError.

Instead, we must really use old-style python formatting since they are
well supported in Babel.  Wrong translations that mess with string
interpolations will cause Babel to give an error when compiling
translation files, which is exactly what we want.
zorun pushed a commit to zorun/ihatemoney that referenced this issue Jan 31, 2023
F-strings are a bad idea for translations, because they cause Babel to
crash when collecting strings to translate:
python-babel/babel#715

But even if we replaced f-strings with new-style string interpolation such
as `_("{foo}").format(foo=foo)`, it's still a bad idea, because a wrong
translation can crash Ihatemoney at runtime with a KeyError.

Instead, we must really use old-style python formatting since they are
well supported in Babel.  Wrong translations that mess with string
interpolations will cause Babel to give an error when compiling
translation files, which is exactly what we want.
zorun pushed a commit to zorun/ihatemoney that referenced this issue Feb 2, 2023
F-strings are a bad idea for translations, because they cause Babel to
crash when collecting strings to translate:
python-babel/babel#715

But even if we replaced f-strings with new-style string interpolation such
as `_("{foo}").format(foo=foo)`, it's still a bad idea, because a wrong
translation can crash Ihatemoney at runtime with a KeyError.

Instead, we must really use old-style python formatting since they are
well supported in Babel.  Wrong translations that mess with string
interpolations will cause Babel to give an error when compiling
translation files, which is exactly what we want.
zorun pushed a commit to zorun/ihatemoney that referenced this issue Feb 3, 2023
F-strings are a bad idea for translations, because they cause Babel to
crash when collecting strings to translate:
python-babel/babel#715

But even if we replaced f-strings with new-style string interpolation such
as `_("{foo}").format(foo=foo)`, it's still a bad idea, because a wrong
translation can crash Ihatemoney at runtime with a KeyError.

Instead, we must really use old-style python formatting since they are
well supported in Babel.  Wrong translations that mess with string
interpolations will cause Babel to give an error when compiling
translation files, which is exactly what we want.
zorun pushed a commit to spiral-project/ihatemoney that referenced this issue Feb 3, 2023
F-strings are a bad idea for translations, because they cause Babel to
crash when collecting strings to translate:
python-babel/babel#715

But even if we replaced f-strings with new-style string interpolation such
as `_("{foo}").format(foo=foo)`, it's still a bad idea, because a wrong
translation can crash Ihatemoney at runtime with a KeyError.

Instead, we must really use old-style python formatting since they are
well supported in Babel.  Wrong translations that mess with string
interpolations will cause Babel to give an error when compiling
translation files, which is exactly what we want.
TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this issue Mar 2, 2024
F-strings are a bad idea for translations, because they cause Babel to
crash when collecting strings to translate:
python-babel/babel#715

But even if we replaced f-strings with new-style string interpolation such
as `_("{foo}").format(foo=foo)`, it's still a bad idea, because a wrong
translation can crash Ihatemoney at runtime with a KeyError.

Instead, we must really use old-style python formatting since they are
well supported in Babel.  Wrong translations that mess with string
interpolations will cause Babel to give an error when compiling
translation files, which is exactly what we want.
lscheibel added a commit to lscheibel/superset that referenced this issue Apr 15, 2024
lscheibel added a commit to lscheibel/superset that referenced this issue Apr 15, 2024
return42 added a commit to return42/searxng that referenced this issue Apr 21, 2024
To test this patch I used .. and checked the diff of the `messages.pot` file::

    $ ./manage pyenv.cmd pybabel extract -F babel.cfg \
              -o ./searx/translations/messages.pot searx/
    $ git diff ./searx/translations/messages.pot

----

hint from @dalf: f-string are not supported [1] but there is no error [2].

searx/translations/messages.pot

[1] python-babel/babel#594
[2] python-babel/babel#715

Closes: searxng#3412
Signed-off-by: Markus Heiser <[email protected]>
return42 added a commit to return42/searxng that referenced this issue Apr 21, 2024
To test this patch I used .. and checked the diff of the `messages.pot` file::

    $ ./manage pyenv.cmd pybabel extract -F babel.cfg \
              -o ./searx/translations/messages.pot searx/
    $ git diff ./searx/translations/messages.pot

----

hint from @dalf: f-string are not supported [1] but there is no error [2].

[1] python-babel/babel#594
[2] python-babel/babel#715

Closes: searxng#3412
Signed-off-by: Markus Heiser <[email protected]>
return42 added a commit to searxng/searxng that referenced this issue Apr 26, 2024
To test this patch I used .. and checked the diff of the `messages.pot` file::

    $ ./manage pyenv.cmd pybabel extract -F babel.cfg \
              -o ./searx/translations/messages.pot searx/
    $ git diff ./searx/translations/messages.pot

----

hint from @dalf: f-string are not supported [1] but there is no error [2].

[1] python-babel/babel#594
[2] python-babel/babel#715

Closes: #3412
Signed-off-by: Markus Heiser <[email protected]>
VieuL pushed a commit to VieuL/searxng that referenced this issue Jul 5, 2024
To test this patch I used .. and checked the diff of the `messages.pot` file::

    $ ./manage pyenv.cmd pybabel extract -F babel.cfg \
              -o ./searx/translations/messages.pot searx/
    $ git diff ./searx/translations/messages.pot

----

hint from @dalf: f-string are not supported [1] but there is no error [2].

[1] python-babel/babel#594
[2] python-babel/babel#715

Closes: searxng#3412
Signed-off-by: Markus Heiser <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants