Skip to content

Commit 8e832fb

Browse files
pablogsalpablogsal
authored
bpo-44885: Correct the ast locations of f-strings with format specs and repeated expressions (pythonGH-27729)
1 parent 789a6af commit 8e832fb

File tree

6 files changed

+103
-91
lines changed

6 files changed

+103
-91
lines changed

‎Lib/test/test_fstring.py‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,6 @@ def test_ast_line_numbers_nested(self):
212212
self.assertEqual(call.col_offset, 11)
213213

214214
def test_ast_line_numbers_duplicate_expression(self):
215-
"""Duplicate expression
216-
217-
NOTE: this is currently broken, always sets location of the first
218-
expression.
219-
"""
220215
expr = """
221216
a = 10
222217
f'{a * x()} {a * x()} {a * x()}'
@@ -266,9 +261,9 @@ def test_ast_line_numbers_duplicate_expression(self):
266261
self.assertEqual(binop.lineno, 3)
267262
self.assertEqual(binop.left.lineno, 3)
268263
self.assertEqual(binop.right.lineno, 3)
269-
self.assertEqual(binop.col_offset, 3) # FIXME: this is wrong
270-
self.assertEqual(binop.left.col_offset, 3) # FIXME: this is wrong
271-
self.assertEqual(binop.right.col_offset, 7) # FIXME: this is wrong
264+
self.assertEqual(binop.col_offset, 13)
265+
self.assertEqual(binop.left.col_offset, 13)
266+
self.assertEqual(binop.right.col_offset, 17)
272267
# check the third binop location
273268
binop = t.body[1].value.values[4].value
274269
self.assertEqual(type(binop), ast.BinOp)
@@ -278,9 +273,32 @@ def test_ast_line_numbers_duplicate_expression(self):
278273
self.assertEqual(binop.lineno, 3)
279274
self.assertEqual(binop.left.lineno, 3)
280275
self.assertEqual(binop.right.lineno, 3)
281-
self.assertEqual(binop.col_offset, 3) # FIXME: this is wrong
282-
self.assertEqual(binop.left.col_offset, 3) # FIXME: this is wrong
283-
self.assertEqual(binop.right.col_offset, 7) # FIXME: this is wrong
276+
self.assertEqual(binop.col_offset, 23)
277+
self.assertEqual(binop.left.col_offset, 23)
278+
self.assertEqual(binop.right.col_offset, 27)
279+
280+
def test_ast_numbers_fstring_with_formatting(self):
281+
282+
t = ast.parse('f"Here is that pesky {xxx:.3f} again"')
283+
self.assertEqual(len(t.body), 1)
284+
self.assertEqual(t.body[0].lineno, 1)
285+
286+
self.assertEqual(type(t.body[0]), ast.Expr)
287+
self.assertEqual(type(t.body[0].value), ast.JoinedStr)
288+
self.assertEqual(len(t.body[0].value.values), 3)
289+
290+
self.assertEqual(type(t.body[0].value.values[0]), ast.Constant)
291+
self.assertEqual(type(t.body[0].value.values[1]), ast.FormattedValue)
292+
self.assertEqual(type(t.body[0].value.values[2]), ast.Constant)
293+
294+
_, expr, _ = t.body[0].value.values
295+
296+
name = expr.value
297+
self.assertEqual(type(name), ast.Name)
298+
self.assertEqual(name.lineno, 1)
299+
self.assertEqual(name.end_lineno, 1)
300+
self.assertEqual(name.col_offset, 22)
301+
self.assertEqual(name.end_col_offset, 25)
284302

285303
def test_ast_line_numbers_multiline_fstring(self):
286304
# See bpo-30465 for details.

‎Misc/NEWS.d/next/Core and Builtins/2021-08-11-15-39-57.bpo-44885.i4noUO.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Correct the ast locations of f-strings with format specs and repeated
2+
expressions. Patch by Pablo Galindo

‎Parser/string_parser.c‎

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -279,49 +279,48 @@ _PyPegen_parsestr(Parser *p, int *bytesmode, int *rawmode, PyObject **result,
279279
/* Fix locations for the given node and its children.
280280
281281
`parent` is the enclosing node.
282+
`expr_start` is the starting position of the expression (pointing to the open brace).
282283
`n` is the node which locations are going to be fixed relative to parent.
283284
`expr_str` is the child node's string representation, including braces.
284285
*/
285286
static bool
286-
fstring_find_expr_location(Token *parent, char *expr_str, int *p_lines, int *p_cols)
287+
fstring_find_expr_location(Token *parent, const char* expr_start, char *expr_str, int *p_lines, int *p_cols)
287288
{
288289
*p_lines = 0;
289290
*p_cols = 0;
291+
assert(expr_start != NULL && *expr_start == '{');
290292
if (parent && parent->bytes) {
291293
const char *parent_str = PyBytes_AsString(parent->bytes);
292294
if (!parent_str) {
293295
return false;
294296
}
295-
const char *substr = strstr(parent_str, expr_str);
296-
if (substr) {
297-
// The following is needed, in order to correctly shift the column
298-
// offset, in the case that (disregarding any whitespace) a newline
299-
// immediately follows the opening curly brace of the fstring expression.
300-
bool newline_after_brace = 1;
301-
const char *start = substr + 1;
302-
while (start && *start != '}' && *start != '\n') {
303-
if (*start != ' ' && *start != '\t' && *start != '\f') {
304-
newline_after_brace = 0;
305-
break;
306-
}
307-
start++;
297+
// The following is needed, in order to correctly shift the column
298+
// offset, in the case that (disregarding any whitespace) a newline
299+
// immediately follows the opening curly brace of the fstring expression.
300+
bool newline_after_brace = 1;
301+
const char *start = expr_start + 1;
302+
while (start && *start != '}' && *start != '\n') {
303+
if (*start != ' ' && *start != '\t' && *start != '\f') {
304+
newline_after_brace = 0;
305+
break;
308306
}
307+
start++;
308+
}
309309

310-
// Account for the characters from the last newline character to our
311-
// left until the beginning of substr.
312-
if (!newline_after_brace) {
313-
start = substr;
314-
while (start > parent_str && *start != '\n') {
315-
start--;
316-
}
317-
*p_cols += (int)(substr - start);
310+
// Account for the characters from the last newline character to our
311+
// left until the beginning of expr_start.
312+
if (!newline_after_brace) {
313+
start = expr_start;
314+
while (start > parent_str && *start != '\n') {
315+
start--;
318316
}
319-
/* adjust the start based on the number of newlines encountered
320-
before the f-string expression */
321-
for (const char *p = parent_str; p < substr; p++) {
322-
if (*p == '\n') {
323-
(*p_lines)++;
324-
}
317+
*p_cols += (int)(expr_start - start);
318+
}
319+
/* adjust the start based on the number of newlines encountered
320+
before the f-string expression */
321+
for (const char *p = parent_str; p < expr_start; p++) {
322+
if (*p == '\n') {
323+
(*p_lines)++;
325324
}
326325
}
327326
}
@@ -365,25 +364,18 @@ fstring_compile_expr(Parser *p, const char *expr_start, const char *expr_end,
365364

366365
len = expr_end - expr_start;
367366
/* Allocate 3 extra bytes: open paren, close paren, null byte. */
368-
str = PyMem_Malloc(len + 3);
367+
str = PyMem_Calloc(len + 3, sizeof(char));
369368
if (str == NULL) {
370369
PyErr_NoMemory();
371370
return NULL;
372371
}
373372

374373
// The call to fstring_find_expr_location is responsible for finding the column offset
375374
// the generated AST nodes need to be shifted to the right, which is equal to the number
376-
// of the f-string characters before the expression starts. In order to correctly compute
377-
// this offset, strstr gets called in fstring_find_expr_location which only succeeds
378-
// if curly braces appear before and after the f-string expression (exactly like they do
379-
// in the f-string itself), hence the following lines.
380-
str[0] = '{';
375+
// of the f-string characters before the expression starts.
381376
memcpy(str+1, expr_start, len);
382-
str[len+1] = '}';
383-
str[len+2] = 0;
384-
385377
int lines, cols;
386-
if (!fstring_find_expr_location(t, str, &lines, &cols)) {
378+
if (!fstring_find_expr_location(t, expr_start-1, str+1, &lines, &cols)) {
387379
PyMem_Free(str);
388380
return NULL;
389381
}

‎Python/importlib.h‎

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/importlib_external.h‎

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)