Skip to content

Commit 16ab070

Browse files
pablogsallysnikolaou
pablogsal
and
lysnikolaou
authored
bpo-40334: Correctly identify invalid target in assignment errors (GH-20076)
Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
1 parent 7ba1f75 commit 16ab070

File tree

10 files changed

+136
-44
lines changed

10 files changed

+136
-44
lines changed

‎Grammar/python.gram

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -640,8 +640,17 @@ invalid_assignment:
640640
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "only single target (not tuple) can be annotated") }
641641
| a=expression ':' expression ['=' annotated_rhs] {
642642
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "illegal target for annotation") }
643-
| a=expression ('=' | augassign) (yield_expr | star_expressions) {
644-
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "cannot assign to %s", _PyPegen_get_expr_name(a)) }
643+
| a=star_expressions '=' (yield_expr | star_expressions) {
644+
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
645+
_PyPegen_get_invalid_target(a),
646+
"cannot assign to %s", _PyPegen_get_expr_name(_PyPegen_get_invalid_target(a))) }
647+
| a=star_expressions augassign (yield_expr | star_expressions) {
648+
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
649+
a,
650+
"'%s' is an illegal expression for augmented assignment",
651+
_PyPegen_get_expr_name(a)
652+
)}
653+
645654
invalid_block:
646655
| NEWLINE !INDENT { RAISE_INDENTATION_ERROR("expected an indented block") }
647656
invalid_comprehension:

‎Lib/test/test_dictcomps.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ def test_illegal_assignment(self):
7777
compile("{x: y for y, x in ((1, 2), (3, 4))} = 5", "<test>",
7878
"exec")
7979

80-
with self.assertRaisesRegex(SyntaxError, "cannot assign"):
80+
with self.assertRaisesRegex(SyntaxError, "illegal expression"):
8181
compile("{x: y for y, x in ((1, 2), (3, 4))} += 5", "<test>",
8282
"exec")
8383

‎Lib/test/test_generators.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1921,7 +1921,7 @@ def printsolution(self, x):
19211921
>>> def f(): (yield bar) += y
19221922
Traceback (most recent call last):
19231923
...
1924-
SyntaxError: cannot assign to yield expression
1924+
SyntaxError: 'yield expression' is an illegal expression for augmented assignment
19251925
19261926
19271927
Now check some throw() conditions:

‎Lib/test/test_genexps.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@
158158
>>> (y for y in (1,2)) += 10
159159
Traceback (most recent call last):
160160
...
161-
SyntaxError: cannot assign to generator expression
161+
SyntaxError: 'generator expression' is an illegal expression for augmented assignment
162162
163163
164164
########### Tests borrowed from or inspired by test_generators.py ############

‎Lib/test/test_peg_parser.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,7 @@ def f():
625625
("(a, b): int", "only single target (not tuple) can be annotated"),
626626
("[a, b]: int", "only single target (not list) can be annotated"),
627627
("a(): int", "illegal target for annotation"),
628-
("1 += 1", "cannot assign to literal"),
628+
("1 += 1", "'literal' is an illegal expression for augmented assignment"),
629629
("pass\n pass", "unexpected indent"),
630630
("def f():\npass", "expected an indented block"),
631631
("def f(*): pass", "named arguments must follow bare *"),

‎Lib/test/test_syntax.py

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -100,46 +100,53 @@
100100
This test just checks a couple of cases rather than enumerating all of
101101
them.
102102
103-
# All of the following also produce different error messages with pegen
104-
# >>> (a, "b", c) = (1, 2, 3)
105-
# Traceback (most recent call last):
106-
# SyntaxError: cannot assign to literal
103+
>>> (a, "b", c) = (1, 2, 3)
104+
Traceback (most recent call last):
105+
SyntaxError: cannot assign to literal
107106
108-
# >>> (a, True, c) = (1, 2, 3)
109-
# Traceback (most recent call last):
110-
# SyntaxError: cannot assign to True
107+
>>> (a, True, c) = (1, 2, 3)
108+
Traceback (most recent call last):
109+
SyntaxError: cannot assign to True
111110
112111
>>> (a, __debug__, c) = (1, 2, 3)
113112
Traceback (most recent call last):
114113
SyntaxError: cannot assign to __debug__
115114
116-
# >>> (a, *True, c) = (1, 2, 3)
117-
# Traceback (most recent call last):
118-
# SyntaxError: cannot assign to True
115+
>>> (a, *True, c) = (1, 2, 3)
116+
Traceback (most recent call last):
117+
SyntaxError: cannot assign to True
119118
120119
>>> (a, *__debug__, c) = (1, 2, 3)
121120
Traceback (most recent call last):
122121
SyntaxError: cannot assign to __debug__
123122
124-
# >>> [a, b, c + 1] = [1, 2, 3]
125-
# Traceback (most recent call last):
126-
# SyntaxError: cannot assign to operator
123+
>>> [a, b, c + 1] = [1, 2, 3]
124+
Traceback (most recent call last):
125+
SyntaxError: cannot assign to operator
126+
127+
>>> [a, b[1], c + 1] = [1, 2, 3]
128+
Traceback (most recent call last):
129+
SyntaxError: cannot assign to operator
130+
131+
>>> [a, b.c.d, c + 1] = [1, 2, 3]
132+
Traceback (most recent call last):
133+
SyntaxError: cannot assign to operator
127134
128135
>>> a if 1 else b = 1
129136
Traceback (most recent call last):
130137
SyntaxError: cannot assign to conditional expression
131138
132139
>>> a, b += 1, 2
133140
Traceback (most recent call last):
134-
SyntaxError: invalid syntax
141+
SyntaxError: 'tuple' is an illegal expression for augmented assignment
135142
136143
>>> (a, b) += 1, 2
137144
Traceback (most recent call last):
138-
SyntaxError: cannot assign to tuple
145+
SyntaxError: 'tuple' is an illegal expression for augmented assignment
139146
140147
>>> [a, b] += 1, 2
141148
Traceback (most recent call last):
142-
SyntaxError: cannot assign to list
149+
SyntaxError: 'list' is an illegal expression for augmented assignment
143150
144151
From compiler_complex_args():
145152
@@ -346,16 +353,16 @@
346353
347354
>>> (x for x in x) += 1
348355
Traceback (most recent call last):
349-
SyntaxError: cannot assign to generator expression
356+
SyntaxError: 'generator expression' is an illegal expression for augmented assignment
350357
>>> None += 1
351358
Traceback (most recent call last):
352-
SyntaxError: cannot assign to None
359+
SyntaxError: 'None' is an illegal expression for augmented assignment
353360
>>> __debug__ += 1
354361
Traceback (most recent call last):
355362
SyntaxError: cannot assign to __debug__
356363
>>> f() += 1
357364
Traceback (most recent call last):
358-
SyntaxError: cannot assign to function call
365+
SyntaxError: 'function call' is an illegal expression for augmented assignment
359366
360367
361368
Test continue in finally in weird combinations.
@@ -688,6 +695,7 @@ def _check_error(self, code, errtext,
688695
def test_assign_call(self):
689696
self._check_error("f() = 1", "assign")
690697

698+
@unittest.skipIf(support.use_old_parser(), "The old parser cannot generate these error messages")
691699
def test_assign_del(self):
692700
self._check_error("del (,)", "invalid syntax")
693701
self._check_error("del 1", "delete literal")

‎Parser/pegen/parse.c

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10747,7 +10747,8 @@ invalid_named_expression_rule(Parser *p)
1074710747
// | tuple ':'
1074810748
// | star_named_expression ',' star_named_expressions* ':'
1074910749
// | expression ':' expression ['=' annotated_rhs]
10750-
// | expression ('=' | augassign) (yield_expr | star_expressions)
10750+
// | star_expressions '=' (yield_expr | star_expressions)
10751+
// | star_expressions augassign (yield_expr | star_expressions)
1075110752
static void *
1075210753
invalid_assignment_rule(Parser *p)
1075310754
{
@@ -10841,19 +10842,40 @@ invalid_assignment_rule(Parser *p)
1084110842
}
1084210843
p->mark = _mark;
1084310844
}
10844-
{ // expression ('=' | augassign) (yield_expr | star_expressions)
10845+
{ // star_expressions '=' (yield_expr | star_expressions)
10846+
Token * _literal;
1084510847
void *_tmp_128_var;
10848+
expr_ty a;
10849+
if (
10850+
(a = star_expressions_rule(p)) // star_expressions
10851+
&&
10852+
(_literal = _PyPegen_expect_token(p, 22)) // token='='
10853+
&&
10854+
(_tmp_128_var = _tmp_128_rule(p)) // yield_expr | star_expressions
10855+
)
10856+
{
10857+
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( _PyPegen_get_invalid_target ( a ) , "cannot assign to %s" , _PyPegen_get_expr_name ( _PyPegen_get_invalid_target ( a ) ) );
10858+
if (_res == NULL && PyErr_Occurred()) {
10859+
p->error_indicator = 1;
10860+
return NULL;
10861+
}
10862+
goto done;
10863+
}
10864+
p->mark = _mark;
10865+
}
10866+
{ // star_expressions augassign (yield_expr | star_expressions)
1084610867
void *_tmp_129_var;
1084710868
expr_ty a;
10869+
AugOperator* augassign_var;
1084810870
if (
10849-
(a = expression_rule(p)) // expression
10871+
(a = star_expressions_rule(p)) // star_expressions
1085010872
&&
10851-
(_tmp_128_var = _tmp_128_rule(p)) // '=' | augassign
10873+
(augassign_var = augassign_rule(p)) // augassign
1085210874
&&
1085310875
(_tmp_129_var = _tmp_129_rule(p)) // yield_expr | star_expressions
1085410876
)
1085510877
{
10856-
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "cannot assign to %s" , _PyPegen_get_expr_name ( a ) );
10878+
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "'%s' is an illegal expression for augmented assignment" , _PyPegen_get_expr_name ( a ) );
1085710879
if (_res == NULL && PyErr_Occurred()) {
1085810880
p->error_indicator = 1;
1085910881
return NULL;
@@ -16675,7 +16697,7 @@ _tmp_127_rule(Parser *p)
1667516697
return _res;
1667616698
}
1667716699

16678-
// _tmp_128: '=' | augassign
16700+
// _tmp_128: yield_expr | star_expressions
1667916701
static void *
1668016702
_tmp_128_rule(Parser *p)
1668116703
{
@@ -16684,24 +16706,24 @@ _tmp_128_rule(Parser *p)
1668416706
}
1668516707
void * _res = NULL;
1668616708
int _mark = p->mark;
16687-
{ // '='
16688-
Token * _literal;
16709+
{ // yield_expr
16710+
expr_ty yield_expr_var;
1668916711
if (
16690-
(_literal = _PyPegen_expect_token(p, 22)) // token='='
16712+
(yield_expr_var = yield_expr_rule(p)) // yield_expr
1669116713
)
1669216714
{
16693-
_res = _literal;
16715+
_res = yield_expr_var;
1669416716
goto done;
1669516717
}
1669616718
p->mark = _mark;
1669716719
}
16698-
{ // augassign
16699-
AugOperator* augassign_var;
16720+
{ // star_expressions
16721+
expr_ty star_expressions_var;
1670016722
if (
16701-
(augassign_var = augassign_rule(p)) // augassign
16723+
(star_expressions_var = star_expressions_rule(p)) // star_expressions
1670216724
)
1670316725
{
16704-
_res = augassign_var;
16726+
_res = star_expressions_var;
1670516727
goto done;
1670616728
}
1670716729
p->mark = _mark;

‎Parser/pegen/pegen.c

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,3 +2054,49 @@ _PyPegen_make_module(Parser *p, asdl_seq *a) {
20542054
}
20552055
return Module(a, type_ignores, p->arena);
20562056
}
2057+
2058+
// Error reporting helpers
2059+
2060+
expr_ty
2061+
_PyPegen_get_invalid_target(expr_ty e)
2062+
{
2063+
if (e == NULL) {
2064+
return NULL;
2065+
}
2066+
2067+
#define VISIT_CONTAINER(CONTAINER, TYPE) do { \
2068+
Py_ssize_t len = asdl_seq_LEN(CONTAINER->v.TYPE.elts);\
2069+
for (Py_ssize_t i = 0; i < len; i++) {\
2070+
expr_ty other = asdl_seq_GET(CONTAINER->v.TYPE.elts, i);\
2071+
expr_ty child = _PyPegen_get_invalid_target(other);\
2072+
if (child != NULL) {\
2073+
return child;\
2074+
}\
2075+
}\
2076+
} while (0)
2077+
2078+
// We only need to visit List and Tuple nodes recursively as those
2079+
// are the only ones that can contain valid names in targets when
2080+
// they are parsed as expressions. Any other kind of expression
2081+
// that is a container (like Sets or Dicts) is directly invalid and
2082+
// we don't need to visit it recursively.
2083+
2084+
switch (e->kind) {
2085+
case List_kind: {
2086+
VISIT_CONTAINER(e, List);
2087+
return NULL;
2088+
}
2089+
case Tuple_kind: {
2090+
VISIT_CONTAINER(e, Tuple);
2091+
return NULL;
2092+
}
2093+
case Starred_kind:
2094+
return _PyPegen_get_invalid_target(e->v.Starred.value);
2095+
case Name_kind:
2096+
case Subscript_kind:
2097+
case Attribute_kind:
2098+
return NULL;
2099+
default:
2100+
return e;
2101+
}
2102+
}

‎Parser/pegen/pegen.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,10 @@ void *_PyPegen_arguments_parsing_error(Parser *, expr_ty);
260260
int _PyPegen_check_barry_as_flufl(Parser *);
261261
mod_ty _PyPegen_make_module(Parser *, asdl_seq *);
262262

263+
// Error reporting helpers
264+
265+
expr_ty _PyPegen_get_invalid_target(expr_ty e);
266+
263267
void *_PyPegen_parse(Parser *);
264268

265269
#endif

‎Python/ast.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3164,10 +3164,7 @@ ast_for_expr_stmt(struct compiling *c, const node *n)
31643164
expr1 = ast_for_testlist(c, ch);
31653165
if (!expr1)
31663166
return NULL;
3167-
if(!set_context(c, expr1, Store, ch))
3168-
return NULL;
3169-
/* set_context checks that most expressions are not the left side.
3170-
Augmented assignments can only have a name, a subscript, or an
3167+
/* Augmented assignments can only have a name, a subscript, or an
31713168
attribute on the left, though, so we have to explicitly check for
31723169
those. */
31733170
switch (expr1->kind) {
@@ -3176,10 +3173,16 @@ ast_for_expr_stmt(struct compiling *c, const node *n)
31763173
case Subscript_kind:
31773174
break;
31783175
default:
3179-
ast_error(c, ch, "illegal expression for augmented assignment");
3176+
ast_error(c, ch, "'%s' is an illegal expression for augmented assignment",
3177+
get_expr_name(expr1));
31803178
return NULL;
31813179
}
31823180

3181+
/* set_context checks that most expressions are not the left side. */
3182+
if(!set_context(c, expr1, Store, ch)) {
3183+
return NULL;
3184+
}
3185+
31833186
ch = CHILD(n, 2);
31843187
if (TYPE(ch) == testlist)
31853188
expr2 = ast_for_testlist(c, ch);

0 commit comments

Comments
 (0)