python-babel / babel

The official repository for Babel, the Python Internationalization Library
http://babel.pocoo.org/
BSD 3-Clause "New" or "Revised" License
1.33k stars 444 forks source link

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

Open fabiommendes opened 4 years ago

fabiommendes commented 4 years ago

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 commented 3 years ago

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

iranzo commented 3 years ago

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 commented 3 years ago

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.