Skip to content

Commit aa1afc7

Browse files
miss-islingtonericvsmith
miss-islington
authored and
ericvsmith
committed
bpo-30465: Fix lineno and col_offset in fstring AST nodes (GH-1800) (gh-3409)
For f-string ast nodes, fix the line and columns so that tools such as flake8 can identify them correctly. (cherry picked from commit e7c566c)
1 parent e00e1b8 commit aa1afc7

3 files changed

Lines changed: 318 additions & 5 deletions

File tree

‎Lib/test/test_fstring.py‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,253 @@ def __call__(self):
7070
# Make sure x was called.
7171
self.assertTrue(x.called)
7272

73+
def test_ast_line_numbers(self):
74+
expr = """
75+
a = 10
76+
f'{a * x()}'"""
77+
t = ast.parse(expr)
78+
self.assertEqual(type(t), ast.Module)
79+
self.assertEqual(len(t.body), 2)
80+
# check `a = 10`
81+
self.assertEqual(type(t.body[0]), ast.Assign)
82+
self.assertEqual(t.body[0].lineno, 2)
83+
# check `f'...'`
84+
self.assertEqual(type(t.body[1]), ast.Expr)
85+
self.assertEqual(type(t.body[1].value), ast.JoinedStr)
86+
self.assertEqual(len(t.body[1].value.values), 1)
87+
self.assertEqual(type(t.body[1].value.values[0]), ast.FormattedValue)
88+
self.assertEqual(t.body[1].lineno, 3)
89+
self.assertEqual(t.body[1].value.lineno, 3)
90+
self.assertEqual(t.body[1].value.values[0].lineno, 3)
91+
# check the binop location
92+
binop = t.body[1].value.values[0].value
93+
self.assertEqual(type(binop), ast.BinOp)
94+
self.assertEqual(type(binop.left), ast.Name)
95+
self.assertEqual(type(binop.op), ast.Mult)
96+
self.assertEqual(type(binop.right), ast.Call)
97+
self.assertEqual(binop.lineno, 3)
98+
self.assertEqual(binop.left.lineno, 3)
99+
self.assertEqual(binop.right.lineno, 3)
100+
self.assertEqual(binop.col_offset, 3)
101+
self.assertEqual(binop.left.col_offset, 3)
102+
self.assertEqual(binop.right.col_offset, 7)
103+
104+
def test_ast_line_numbers_multiple_formattedvalues(self):
105+
expr = """
106+
f'no formatted values'
107+
f'eggs {a * x()} spam {b + y()}'"""
108+
t = ast.parse(expr)
109+
self.assertEqual(type(t), ast.Module)
110+
self.assertEqual(len(t.body), 2)
111+
# check `f'no formatted value'`
112+
self.assertEqual(type(t.body[0]), ast.Expr)
113+
self.assertEqual(type(t.body[0].value), ast.JoinedStr)
114+
self.assertEqual(t.body[0].lineno, 2)
115+
# check `f'...'`
116+
self.assertEqual(type(t.body[1]), ast.Expr)
117+
self.assertEqual(type(t.body[1].value), ast.JoinedStr)
118+
self.assertEqual(len(t.body[1].value.values), 4)
119+
self.assertEqual(type(t.body[1].value.values[0]), ast.Str)
120+
self.assertEqual(type(t.body[1].value.values[1]), ast.FormattedValue)
121+
self.assertEqual(type(t.body[1].value.values[2]), ast.Str)
122+
self.assertEqual(type(t.body[1].value.values[3]), ast.FormattedValue)
123+
self.assertEqual(t.body[1].lineno, 3)
124+
self.assertEqual(t.body[1].value.lineno, 3)
125+
self.assertEqual(t.body[1].value.values[0].lineno, 3)
126+
self.assertEqual(t.body[1].value.values[1].lineno, 3)
127+
self.assertEqual(t.body[1].value.values[2].lineno, 3)
128+
self.assertEqual(t.body[1].value.values[3].lineno, 3)
129+
# check the first binop location
130+
binop1 = t.body[1].value.values[1].value
131+
self.assertEqual(type(binop1), ast.BinOp)
132+
self.assertEqual(type(binop1.left), ast.Name)
133+
self.assertEqual(type(binop1.op), ast.Mult)
134+
self.assertEqual(type(binop1.right), ast.Call)
135+
self.assertEqual(binop1.lineno, 3)
136+
self.assertEqual(binop1.left.lineno, 3)
137+
self.assertEqual(binop1.right.lineno, 3)
138+
self.assertEqual(binop1.col_offset, 8)
139+
self.assertEqual(binop1.left.col_offset, 8)
140+
self.assertEqual(binop1.right.col_offset, 12)
141+
# check the second binop location
142+
binop2 = t.body[1].value.values[3].value
143+
self.assertEqual(type(binop2), ast.BinOp)
144+
self.assertEqual(type(binop2.left), ast.Name)
145+
self.assertEqual(type(binop2.op), ast.Add)
146+
self.assertEqual(type(binop2.right), ast.Call)
147+
self.assertEqual(binop2.lineno, 3)
148+
self.assertEqual(binop2.left.lineno, 3)
149+
self.assertEqual(binop2.right.lineno, 3)
150+
self.assertEqual(binop2.col_offset, 23)
151+
self.assertEqual(binop2.left.col_offset, 23)
152+
self.assertEqual(binop2.right.col_offset, 27)
153+
154+
def test_ast_line_numbers_nested(self):
155+
expr = """
156+
a = 10
157+
f'{a * f"-{x()}-"}'"""
158+
t = ast.parse(expr)
159+
self.assertEqual(type(t), ast.Module)
160+
self.assertEqual(len(t.body), 2)
161+
# check `a = 10`
162+
self.assertEqual(type(t.body[0]), ast.Assign)
163+
self.assertEqual(t.body[0].lineno, 2)
164+
# check `f'...'`
165+
self.assertEqual(type(t.body[1]), ast.Expr)
166+
self.assertEqual(type(t.body[1].value), ast.JoinedStr)
167+
self.assertEqual(len(t.body[1].value.values), 1)
168+
self.assertEqual(type(t.body[1].value.values[0]), ast.FormattedValue)
169+
self.assertEqual(t.body[1].lineno, 3)
170+
self.assertEqual(t.body[1].value.lineno, 3)
171+
self.assertEqual(t.body[1].value.values[0].lineno, 3)
172+
# check the binop location
173+
binop = t.body[1].value.values[0].value
174+
self.assertEqual(type(binop), ast.BinOp)
175+
self.assertEqual(type(binop.left), ast.Name)
176+
self.assertEqual(type(binop.op), ast.Mult)
177+
self.assertEqual(type(binop.right), ast.JoinedStr)
178+
self.assertEqual(binop.lineno, 3)
179+
self.assertEqual(binop.left.lineno, 3)
180+
self.assertEqual(binop.right.lineno, 3)
181+
self.assertEqual(binop.col_offset, 3)
182+
self.assertEqual(binop.left.col_offset, 3)
183+
self.assertEqual(binop.right.col_offset, 7)
184+
# check the nested call location
185+
self.assertEqual(len(binop.right.values), 3)
186+
self.assertEqual(type(binop.right.values[0]), ast.Str)
187+
self.assertEqual(type(binop.right.values[1]), ast.FormattedValue)
188+
self.assertEqual(type(binop.right.values[2]), ast.Str)
189+
self.assertEqual(binop.right.values[0].lineno, 3)
190+
self.assertEqual(binop.right.values[1].lineno, 3)
191+
self.assertEqual(binop.right.values[2].lineno, 3)
192+
call = binop.right.values[1].value
193+
self.assertEqual(type(call), ast.Call)
194+
self.assertEqual(call.lineno, 3)
195+
self.assertEqual(call.col_offset, 11)
196+
197+
def test_ast_line_numbers_duplicate_expression(self):
198+
"""Duplicate expression
199+
200+
NOTE: this is currently broken, always sets location of the first
201+
expression.
202+
"""
203+
expr = """
204+
a = 10
205+
f'{a * x()} {a * x()} {a * x()}'
206+
"""
207+
t = ast.parse(expr)
208+
self.assertEqual(type(t), ast.Module)
209+
self.assertEqual(len(t.body), 2)
210+
# check `a = 10`
211+
self.assertEqual(type(t.body[0]), ast.Assign)
212+
self.assertEqual(t.body[0].lineno, 2)
213+
# check `f'...'`
214+
self.assertEqual(type(t.body[1]), ast.Expr)
215+
self.assertEqual(type(t.body[1].value), ast.JoinedStr)
216+
self.assertEqual(len(t.body[1].value.values), 5)
217+
self.assertEqual(type(t.body[1].value.values[0]), ast.FormattedValue)
218+
self.assertEqual(type(t.body[1].value.values[1]), ast.Str)
219+
self.assertEqual(type(t.body[1].value.values[2]), ast.FormattedValue)
220+
self.assertEqual(type(t.body[1].value.values[3]), ast.Str)
221+
self.assertEqual(type(t.body[1].value.values[4]), ast.FormattedValue)
222+
self.assertEqual(t.body[1].lineno, 3)
223+
self.assertEqual(t.body[1].value.lineno, 3)
224+
self.assertEqual(t.body[1].value.values[0].lineno, 3)
225+
self.assertEqual(t.body[1].value.values[1].lineno, 3)
226+
self.assertEqual(t.body[1].value.values[2].lineno, 3)
227+
self.assertEqual(t.body[1].value.values[3].lineno, 3)
228+
self.assertEqual(t.body[1].value.values[4].lineno, 3)
229+
# check the first binop location
230+
binop = t.body[1].value.values[0].value
231+
self.assertEqual(type(binop), ast.BinOp)
232+
self.assertEqual(type(binop.left), ast.Name)
233+
self.assertEqual(type(binop.op), ast.Mult)
234+
self.assertEqual(type(binop.right), ast.Call)
235+
self.assertEqual(binop.lineno, 3)
236+
self.assertEqual(binop.left.lineno, 3)
237+
self.assertEqual(binop.right.lineno, 3)
238+
self.assertEqual(binop.col_offset, 3)
239+
self.assertEqual(binop.left.col_offset, 3)
240+
self.assertEqual(binop.right.col_offset, 7)
241+
# check the second binop location
242+
binop = t.body[1].value.values[2].value
243+
self.assertEqual(type(binop), ast.BinOp)
244+
self.assertEqual(type(binop.left), ast.Name)
245+
self.assertEqual(type(binop.op), ast.Mult)
246+
self.assertEqual(type(binop.right), ast.Call)
247+
self.assertEqual(binop.lineno, 3)
248+
self.assertEqual(binop.left.lineno, 3)
249+
self.assertEqual(binop.right.lineno, 3)
250+
self.assertEqual(binop.col_offset, 3) # FIXME: this is wrong
251+
self.assertEqual(binop.left.col_offset, 3) # FIXME: this is wrong
252+
self.assertEqual(binop.right.col_offset, 7) # FIXME: this is wrong
253+
# check the third binop location
254+
binop = t.body[1].value.values[4].value
255+
self.assertEqual(type(binop), ast.BinOp)
256+
self.assertEqual(type(binop.left), ast.Name)
257+
self.assertEqual(type(binop.op), ast.Mult)
258+
self.assertEqual(type(binop.right), ast.Call)
259+
self.assertEqual(binop.lineno, 3)
260+
self.assertEqual(binop.left.lineno, 3)
261+
self.assertEqual(binop.right.lineno, 3)
262+
self.assertEqual(binop.col_offset, 3) # FIXME: this is wrong
263+
self.assertEqual(binop.left.col_offset, 3) # FIXME: this is wrong
264+
self.assertEqual(binop.right.col_offset, 7) # FIXME: this is wrong
265+
266+
def test_ast_line_numbers_multiline_fstring(self):
267+
# FIXME: This test demonstrates invalid behavior due to JoinedStr's
268+
# immediate child nodes containing the wrong lineno. The enclosed
269+
# expressions have valid line information and column offsets.
270+
# See bpo-16806 and bpo-30465 for details.
271+
expr = """
272+
a = 10
273+
f'''
274+
{a
275+
*
276+
x()}
277+
non-important content
278+
'''
279+
"""
280+
t = ast.parse(expr)
281+
self.assertEqual(type(t), ast.Module)
282+
self.assertEqual(len(t.body), 2)
283+
# check `a = 10`
284+
self.assertEqual(type(t.body[0]), ast.Assign)
285+
self.assertEqual(t.body[0].lineno, 2)
286+
# check `f'...'`
287+
self.assertEqual(type(t.body[1]), ast.Expr)
288+
self.assertEqual(type(t.body[1].value), ast.JoinedStr)
289+
self.assertEqual(len(t.body[1].value.values), 3)
290+
self.assertEqual(type(t.body[1].value.values[0]), ast.Str)
291+
self.assertEqual(type(t.body[1].value.values[1]), ast.FormattedValue)
292+
self.assertEqual(type(t.body[1].value.values[2]), ast.Str)
293+
# NOTE: the following invalid behavior is described in bpo-16806.
294+
# - line number should be the *first* line (3), not the *last* (8)
295+
# - column offset should not be -1
296+
self.assertEqual(t.body[1].lineno, 8)
297+
self.assertEqual(t.body[1].value.lineno, 8)
298+
self.assertEqual(t.body[1].value.values[0].lineno, 8)
299+
self.assertEqual(t.body[1].value.values[1].lineno, 8)
300+
self.assertEqual(t.body[1].value.values[2].lineno, 8)
301+
self.assertEqual(t.body[1].col_offset, -1)
302+
self.assertEqual(t.body[1].value.col_offset, -1)
303+
self.assertEqual(t.body[1].value.values[0].col_offset, -1)
304+
self.assertEqual(t.body[1].value.values[1].col_offset, -1)
305+
self.assertEqual(t.body[1].value.values[2].col_offset, -1)
306+
# NOTE: the following lineno information and col_offset is correct for
307+
# expressions within FormattedValues.
308+
binop = t.body[1].value.values[1].value
309+
self.assertEqual(type(binop), ast.BinOp)
310+
self.assertEqual(type(binop.left), ast.Name)
311+
self.assertEqual(type(binop.op), ast.Mult)
312+
self.assertEqual(type(binop.right), ast.Call)
313+
self.assertEqual(binop.lineno, 4)
314+
self.assertEqual(binop.left.lineno, 4)
315+
self.assertEqual(binop.right.lineno, 6)
316+
self.assertEqual(binop.col_offset, 3)
317+
self.assertEqual(binop.left.col_offset, 3)
318+
self.assertEqual(binop.right.col_offset, 7)
319+
73320
def test_docstring(self):
74321
def f():
75322
f'''Not a docstring'''
@@ -786,5 +1033,6 @@ def test_backslash_char(self):
7861033
self.assertEqual(eval('f"\\\n"'), '')
7871034
self.assertEqual(eval('f"\\\r"'), '')
7881035

1036+
7891037
if __name__ == '__main__':
7901038
unittest.main()

‎Misc/NEWS.d/next/Core and Builtins/2017-09-06-10-47-29.bpo-30465.oe-3GD.rst‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Location information (``lineno`` and ``col_offset``) in f-strings is now
2+
(mostly) correct. This fixes tools like flake8 from showing warnings on the
3+
wrong line (typically the first line of the file).

‎Python/ast.c‎

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "node.h"
99
#include "ast.h"
1010
#include "token.h"
11+
#include "pythonrun.h"
1112

1213
#include <assert.h>
1314

@@ -4240,6 +4241,55 @@ decode_bytes_with_escapes(struct compiling *c, const node *n, const char *s,
42404241
return result;
42414242
}
42424243

4244+
/* Shift locations for the given node and all its children by adding `lineno`
4245+
and `col_offset` to existing locations. */
4246+
static void fstring_shift_node_locations(node *n, int lineno, int col_offset)
4247+
{
4248+
n->n_col_offset = n->n_col_offset + col_offset;
4249+
for (int i = 0; i < NCH(n); ++i) {
4250+
if (n->n_lineno && n->n_lineno < CHILD(n, i)->n_lineno) {
4251+
/* Shifting column offsets unnecessary if there's been newlines. */
4252+
col_offset = 0;
4253+
}
4254+
fstring_shift_node_locations(CHILD(n, i), lineno, col_offset);
4255+
}
4256+
n->n_lineno = n->n_lineno + lineno;
4257+
}
4258+
4259+
/* Fix locations for the given node and its children.
4260+
4261+
`parent` is the enclosing node.
4262+
`n` is the node which locations are going to be fixed relative to parent.
4263+
`expr_str` is the child node's string representation, incuding braces.
4264+
*/
4265+
static void
4266+
fstring_fix_node_location(const node *parent, node *n, char *expr_str)
4267+
{
4268+
char *substr = NULL;
4269+
char *start;
4270+
int lines = LINENO(parent) - 1;
4271+
int cols = parent->n_col_offset;
4272+
/* Find the full fstring to fix location information in `n`. */
4273+
while (parent && parent->n_type != STRING)
4274+
parent = parent->n_child;
4275+
if (parent && parent->n_str) {
4276+
substr = strstr(parent->n_str, expr_str);
4277+
if (substr) {
4278+
start = substr;
4279+
while (start > parent->n_str) {
4280+
if (start[0] == '\n')
4281+
break;
4282+
start--;
4283+
}
4284+
cols += substr - start;
4285+
/* Fix lineno in mulitline strings. */
4286+
while ((substr = strchr(substr + 1, '\n')))
4287+
lines--;
4288+
}
4289+
}
4290+
fstring_shift_node_locations(n, lines, cols);
4291+
}
4292+
42434293
/* Compile this expression in to an expr_ty. Add parens around the
42444294
expression, in order to allow leading spaces in the expression. */
42454295
static expr_ty
@@ -4248,6 +4298,7 @@ fstring_compile_expr(const char *expr_start, const char *expr_end,
42484298

42494299
{
42504300
PyCompilerFlags cf;
4301+
node *mod_n;
42514302
mod_ty mod;
42524303
char *str;
42534304
Py_ssize_t len;
@@ -4257,9 +4308,10 @@ fstring_compile_expr(const char *expr_start, const char *expr_end,
42574308
assert(*(expr_start-1) == '{');
42584309
assert(*expr_end == '}' || *expr_end == '!' || *expr_end == ':');
42594310

4260-
/* If the substring is all whitespace, it's an error. We need to catch
4261-
this here, and not when we call PyParser_ASTFromString, because turning
4262-
the expression '' in to '()' would go from being invalid to valid. */
4311+
/* If the substring is all whitespace, it's an error. We need to catch this
4312+
here, and not when we call PyParser_SimpleParseStringFlagsFilename,
4313+
because turning the expression '' in to '()' would go from being invalid
4314+
to valid. */
42634315
for (s = expr_start; s != expr_end; s++) {
42644316
char c = *s;
42654317
/* The Python parser ignores only the following whitespace
@@ -4285,9 +4337,19 @@ fstring_compile_expr(const char *expr_start, const char *expr_end,
42854337
str[len+2] = 0;
42864338

42874339
cf.cf_flags = PyCF_ONLY_AST;
4288-
mod = PyParser_ASTFromString(str, "<fstring>",
4289-
Py_eval_input, &cf, c->c_arena);
4340+
mod_n = PyParser_SimpleParseStringFlagsFilename(str, "<fstring>",
4341+
Py_eval_input, 0);
4342+
if (!mod_n) {
4343+
PyMem_RawFree(str);
4344+
return NULL;
4345+
}
4346+
/* Reuse str to find the correct column offset. */
4347+
str[0] = '{';
4348+
str[len+1] = '}';
4349+
fstring_fix_node_location(n, mod_n, str);
4350+
mod = PyAST_FromNode(mod_n, &cf, "<fstring>", c->c_arena);
42904351
PyMem_RawFree(str);
4352+
PyNode_Free(mod_n);
42914353
if (!mod)
42924354
return NULL;
42934355
return mod->v.Expression.body;

0 commit comments

Comments
 (0)