Skip to content

Commit ff62d8b

Browse files
ericvsmithericvsmith
committed
Move f-string compilation of the expression earlier, before the conversion character and format_spec are checked. This allows for error messages that more closely match what a user would expect.
1 parent 09adae4 commit ff62d8b

2 files changed

Lines changed: 60 additions & 15 deletions

File tree

‎Lib/test/test_fstring.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,15 @@ def test_missing_expression(self):
287287
"f' { } '",
288288
r"f'{\n}'",
289289
r"f'{\n \n}'",
290+
291+
# Catch the empty expression before the
292+
# invalid conversion.
293+
"f'{!x}'",
294+
"f'{ !xr}'",
295+
"f'{!x:}'",
296+
"f'{!x:a}'",
297+
"f'{ !xr:}'",
298+
"f'{ !xr:a}'",
290299
])
291300

292301
def test_parens_in_expressions(self):

‎Python/ast.c‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4007,13 +4007,14 @@ decode_unicode(struct compiling *c, const char *s, size_t len, const char *encod
40074007
expression. This is to allow strings with embedded newlines, for
40084008
example. */
40094009
static expr_ty
4010-
fstring_expression_compile(PyObject *str, Py_ssize_t expr_start,
4011-
Py_ssize_t expr_end, PyArena *arena)
4010+
fstring_compile_expr(PyObject *str, Py_ssize_t expr_start,
4011+
Py_ssize_t expr_end, PyArena *arena)
40124012
{
40134013
PyCompilerFlags cf;
40144014
mod_ty mod;
40154015
char *utf_expr;
40164016
Py_ssize_t i;
4017+
Py_UCS4 end_ch = -1;
40174018
int all_whitespace;
40184019
PyObject *sub = NULL;
40194020

@@ -4023,6 +4024,16 @@ fstring_expression_compile(PyObject *str, Py_ssize_t expr_start,
40234024

40244025
assert(str);
40254026

4027+
assert(expr_start >= 0 && expr_start < PyUnicode_GET_LENGTH(str));
4028+
assert(expr_end >= 0 && expr_end < PyUnicode_GET_LENGTH(str));
4029+
assert(expr_end >= expr_start);
4030+
4031+
/* There has to be at least on character on each side of the
4032+
expression inside this str. This will have been caught before
4033+
we're called. */
4034+
assert(expr_start >= 1);
4035+
assert(expr_end <= PyUnicode_GET_LENGTH(str)-1);
4036+
40264037
/* If the substring is all whitespace, it's an error. We need to
40274038
catch this here, and not when we call PyParser_ASTFromString,
40284039
because turning the expression '' in to '()' would go from
@@ -4049,10 +4060,17 @@ fstring_expression_compile(PyObject *str, Py_ssize_t expr_start,
40494060
string directly. */
40504061

40514062
if (expr_start-1 == 0 && expr_end+1 == PyUnicode_GET_LENGTH(str)) {
4052-
/* No need to actually remember these characters, because we
4053-
know they must be braces. */
4063+
/* If str is well formed, then the first and last chars must
4064+
be '{' and '}', respectively. But, if there's a syntax
4065+
error, for example f'{3!', then the last char won't be a
4066+
closing brace. So, remember the last character we read in
4067+
order for us to restore it. */
4068+
end_ch = PyUnicode_ReadChar(str, expr_end-expr_start+1);
4069+
assert(end_ch != (Py_UCS4)-1);
4070+
4071+
/* In all cases, however, start_ch must be '{'. */
40544072
assert(PyUnicode_ReadChar(str, 0) == '{');
4055-
assert(PyUnicode_ReadChar(str, expr_end-expr_start+1) == '}');
4073+
40564074
sub = str;
40574075
} else {
40584076
/* Create a substring object. It must be a new object, with
@@ -4064,21 +4082,23 @@ fstring_expression_compile(PyObject *str, Py_ssize_t expr_start,
40644082
decref_sub = 1; /* Remember to deallocate it on error. */
40654083
}
40664084

4085+
/* Put () around the expression. */
40674086
if (PyUnicode_WriteChar(sub, 0, '(') < 0 ||
40684087
PyUnicode_WriteChar(sub, expr_end-expr_start+1, ')') < 0)
40694088
goto error;
40704089

4071-
cf.cf_flags = PyCF_ONLY_AST;
4072-
40734090
/* No need to free the memory returned here: it's managed by the
40744091
string. */
40754092
utf_expr = PyUnicode_AsUTF8(sub);
40764093
if (!utf_expr)
40774094
goto error;
4095+
4096+
cf.cf_flags = PyCF_ONLY_AST;
40784097
mod = PyParser_ASTFromString(utf_expr, "<fstring>",
40794098
Py_eval_input, &cf, arena);
40804099
if (!mod)
40814100
goto error;
4101+
40824102
if (sub != str)
40834103
/* Clear instead of decref in case we ever modify this code to change
40844104
the error handling: this is safest because the XDECREF won't try
@@ -4089,9 +4109,10 @@ fstring_expression_compile(PyObject *str, Py_ssize_t expr_start,
40894109
Py_CLEAR(sub);
40904110
else {
40914111
assert(!decref_sub);
4112+
assert(end_ch != (Py_UCS4)-1);
40924113
/* Restore str, which we earlier modified directly. */
40934114
if (PyUnicode_WriteChar(str, 0, '{') < 0 ||
4094-
PyUnicode_WriteChar(str, expr_end-expr_start+1, '}') < 0)
4115+
PyUnicode_WriteChar(str, expr_end-expr_start+1, end_ch) < 0)
40954116
goto error;
40964117
}
40974118
return mod->v.Expression.body;
@@ -4100,6 +4121,18 @@ fstring_expression_compile(PyObject *str, Py_ssize_t expr_start,
41004121
/* Only decref sub if it was the result of a call to SubString. */
41014122
if (decref_sub)
41024123
Py_XDECREF(sub);
4124+
4125+
if (end_ch != (Py_UCS4)-1) {
4126+
/* We only get here if we modified str. Make sure that's the
4127+
case: str will be equal to sub. */
4128+
if (str == sub) {
4129+
/* Don't check the error, because we've already set the
4130+
error state (that's why we're in 'error', after
4131+
all). */
4132+
PyUnicode_WriteChar(str, 0, '{');
4133+
PyUnicode_WriteChar(str, expr_end-expr_start+1, end_ch);
4134+
}
4135+
}
41034136
return NULL;
41044137
}
41054138

@@ -4331,9 +4364,18 @@ fstring_find_expr(PyObject *str, Py_ssize_t *ofs, int recurse_lvl,
43314364
return -1;
43324365
}
43334366

4334-
/* Check for a conversion char, if present. */
43354367
if (*ofs >= PyUnicode_GET_LENGTH(str))
43364368
goto unexpected_end_of_string;
4369+
4370+
/* Compile the expression as soon as possible, so we show errors
4371+
related to the expression before errors related to the
4372+
conversion or format_spec. */
4373+
simple_expression = fstring_compile_expr(str, expr_start, expr_end,
4374+
c->c_arena);
4375+
if (!simple_expression)
4376+
return -1;
4377+
4378+
/* Check for a conversion char, if present. */
43374379
if (PyUnicode_READ(kind, data, *ofs) == '!') {
43384380
*ofs += 1;
43394381
if (*ofs >= PyUnicode_GET_LENGTH(str))
@@ -4374,12 +4416,6 @@ fstring_find_expr(PyObject *str, Py_ssize_t *ofs, int recurse_lvl,
43744416
assert(PyUnicode_READ(kind, data, *ofs) == '}');
43754417
*ofs += 1;
43764418

4377-
/* Compile the expression. */
4378-
simple_expression = fstring_expression_compile(str, expr_start, expr_end,
4379-
c->c_arena);
4380-
if (!simple_expression)
4381-
return -1;
4382-
43834419
/* And now create the FormattedValue node that represents this entire
43844420
expression with the conversion and format spec. */
43854421
*expression = FormattedValue(simple_expression, (int)conversion,

0 commit comments

Comments
 (0)