GeezerVison is showing github.com — bpo-40334: Correctly identify invalid target in assignment errors by pablogsal · Pull Request #20076 · python/cpython
bpo-40334: Correctly identify invalid target in assignment errors
#20076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement . We’ll occasionally send you account related emails.
Already on GitHub?
Sign in
to your account
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
445c7fd
bpo-40334: Correctly identify invalid target in assignment errors
pablogsal May 13, 2020
d385c29
Update Grammar/python.gram
pablogsal May 13, 2020
b596bba
Deduplicate code with macro
pablogsal May 14, 2020
4f2cb72
Fix error for augassign
pablogsal May 14, 2020
c74ecce
Address Lysandros's feedback
pablogsal May 15, 2020
6d1d63c
Add quotes around the node type
pablogsal May 15, 2020
00a2205
Update test_dictcomps, test_genexps and test_generators
pablogsal May 15, 2020
349ea59
Update some tests to the new errors
pablogsal May 15, 2020
6a5c4ed
Add a comment explaining why we only visit Tuples and Lists
pablogsal May 15, 2020
File filter
Filter by extension
.c
(3)
.gram
(1)
.h
(1)
.py
(5)
All 4 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -640,8 +640,17 @@ invalid_assignment:
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "only single target (not tuple) can be annotated") }
| a=expression ':' expression ['=' annotated_rhs] {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "illegal target for annotation") }
| a=expression ('=' | augassign) (yield_expr | star_expressions) {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "cannot assign to %s", _PyPegen_get_expr_name(a)) }
| a=star_expressions '=' (yield_expr | star_expressions) {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
_PyPegen_get_invalid_target(a),
"cannot assign to %s", _PyPegen_get_expr_name(_PyPegen_get_invalid_target(a))) }
| a=star_expressions augassign (yield_expr | star_expressions) {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
a,
"'%s' is an illegal expression for augmented assignment",
_PyPegen_get_expr_name(a)
)}
invalid_block:
| NEWLINE !INDENT { RAISE_INDENTATION_ERROR("expected an indented block") }
invalid_comprehension:
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -77,7 +77,7 @@ def test_illegal_assignment(self):
compile("{x: y for y, x in ((1, 2), (3, 4))} = 5", "<test>",
"exec")
with self.assertRaisesRegex(SyntaxError, "cannot assign "):
with self.assertRaisesRegex(SyntaxError, "illegal expression "):
compile("{x: y for y, x in ((1, 2), (3, 4))} += 5", "<test>",
"exec")
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -1921,7 +1921,7 @@ def printsolution(self, x):
>>> def f(): (yield bar) += y
Traceback (most recent call last):
...
SyntaxError: cannot assign to yield expression
SyntaxError: 'yield expression' is an illegal expression for augmented assignment
Now check some throw() conditions:
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -158,7 +158,7 @@
>>> (y for y in (1,2)) += 10
Traceback (most recent call last):
...
SyntaxError: cannot assign to generator expression
SyntaxError: 'generator expression' is an illegal expression for augmented assignment
########### Tests borrowed from or inspired by test_generators.py ############
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -625,7 +625,7 @@ def f():
("(a, b): int", "only single target (not tuple) can be annotated"),
("[a, b]: int", "only single target (not list) can be annotated"),
("a(): int", "illegal target for annotation"),
("1 += 1", "cannot assign to literal "),
("1 += 1", "'literal' is an illegal expression for augmented assignment "),
("pass\n pass", "unexpected indent"),
("def f():\npass", "expected an indented block"),
("def f(*): pass", "named arguments must follow bare *"),
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -100,46 +100,53 @@
This test just checks a couple of cases rather than enumerating all of
them.
# All of the following also produce different error messages with pegen
# >>> (a, "b", c) = (1, 2, 3)
# Traceback (most recent call last):
# SyntaxError: cannot assign to literal
>>> (a, "b", c) = (1, 2, 3)
Traceback (most recent call last):
SyntaxError: cannot assign to literal
# >>> (a, True, c) = (1, 2, 3)
# Traceback (most recent call last):
# SyntaxError: cannot assign to True
>>> (a, True, c) = (1, 2, 3)
Traceback (most recent call last):
SyntaxError: cannot assign to True
>>> (a, __debug__, c) = (1, 2, 3)
Traceback (most recent call last):
SyntaxError: cannot assign to __debug__
# >>> (a, *True, c) = (1, 2, 3)
# Traceback (most recent call last):
# SyntaxError: cannot assign to True
>>> (a, *True, c) = (1, 2, 3)
Traceback (most recent call last):
SyntaxError: cannot assign to True
>>> (a, *__debug__, c) = (1, 2, 3)
Traceback (most recent call last):
SyntaxError: cannot assign to __debug__
# >>> [a, b, c + 1] = [1, 2, 3]
# Traceback (most recent call last):
# SyntaxError: cannot assign to operator
>>> [a, b, c + 1] = [1, 2, 3]
Traceback (most recent call last):
SyntaxError: cannot assign to operator
>>> [a, b[1], c + 1] = [1, 2, 3]
Traceback (most recent call last):
SyntaxError: cannot assign to operator
>>> [a, b.c.d, c + 1] = [1, 2, 3]
Traceback (most recent call last):
SyntaxError: cannot assign to operator
>>> a if 1 else b = 1
Traceback (most recent call last):
SyntaxError: cannot assign to conditional expression
>>> a, b += 1, 2
Traceback (most recent call last):
SyntaxError: invalid syntax
SyntaxError: 'tuple' is an illegal expression for augmented assignment
>>> (a, b) += 1, 2
Traceback (most recent call last):
SyntaxError: cannot assign to tuple
SyntaxError: 'tuple' is an illegal expression for augmented assignment
>>> [a, b] += 1, 2
Traceback (most recent call last):
SyntaxError: cannot assign to list
SyntaxError: 'list' is an illegal expression for augmented assignment
From compiler_complex_args():
Expand Down
Expand Up
@@ -346,16 +353,16 @@
>>> (x for x in x) += 1
Traceback (most recent call last):
SyntaxError: cannot assign to generator expression
SyntaxError: 'generator expression' is an illegal expression for augmented assignment
>>> None += 1
Traceback (most recent call last):
SyntaxError: cannot assign to None
SyntaxError: 'None' is an illegal expression for augmented assignment
>>> __debug__ += 1
Traceback (most recent call last):
SyntaxError: cannot assign to __debug__
>>> f() += 1
Traceback (most recent call last):
SyntaxError: cannot assign to function call
SyntaxError: 'function call' is an illegal expression for augmented assignment
Test continue in finally in weird combinations.
Expand Down
Expand Up
@@ -688,6 +695,7 @@ def _check_error(self, code, errtext,
def test_assign_call(self):
self._check_error("f() = 1", "assign")
@unittest.skipIf(support.use_old_parser(), "The old parser cannot generate these error messages")
def test_assign_del(self):
self._check_error("del (,)", "invalid syntax")
self._check_error("del 1", "delete literal")
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -10747,7 +10747,8 @@ invalid_named_expression_rule(Parser *p)
// | tuple ':'
// | star_named_expression ',' star_named_expressions* ':'
// | expression ':' expression ['=' annotated_rhs]
// | expression ('=' | augassign) (yield_expr | star_expressions)
// | star_expressions '=' (yield_expr | star_expressions)
// | star_expressions augassign (yield_expr | star_expressions)
static void *
invalid_assignment_rule(Parser *p)
{
Expand Down
Expand Up
@@ -10841,19 +10842,40 @@ invalid_assignment_rule(Parser *p)
}
p->mark = _mark;
}
{ // expression ('=' | augassign) (yield_expr | star_expressions)
{ // star_expressions '=' (yield_expr | star_expressions)
Token * _literal;
void *_tmp_128_var;
expr_ty a;
if (
(a = star_expressions_rule(p)) // star_expressions
&&
(_literal = _PyPegen_expect_token(p, 22)) // token='='
&&
(_tmp_128_var = _tmp_128_rule(p)) // yield_expr | star_expressions
)
{
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( _PyPegen_get_invalid_target ( a ) , "cannot assign to %s" , _PyPegen_get_expr_name ( _PyPegen_get_invalid_target ( a ) ) );
if (_res == NULL && PyErr_Occurred()) {
p->error_indicator = 1;
return NULL;
}
goto done;
}
p->mark = _mark;
}
{ // star_expressions augassign (yield_expr | star_expressions)
void *_tmp_129_var;
expr_ty a;
AugOperator* augassign_var;
if (
(a = expression_rule (p)) // expression
(a = star_expressions_rule (p)) // star_expressions
&&
(_tmp_128_var = _tmp_128_rule (p)) // '=' | augassign
(augassign_var = augassign_rule (p)) // augassign
&&
(_tmp_129_var = _tmp_129_rule(p)) // yield_expr | star_expressions
)
{
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "cannot assign to %s " , _PyPegen_get_expr_name ( a ) );
_res = RAISE_SYNTAX_ERROR_KNOWN_LOCATION ( a , "'%s' is an illegal expression for augmented assignment " , _PyPegen_get_expr_name ( a ) );
if (_res == NULL && PyErr_Occurred()) {
p->error_indicator = 1;
return NULL;
Expand Down
Expand Up
@@ -16675,7 +16697,7 @@ _tmp_127_rule(Parser *p)
return _res;
}
// _tmp_128: '=' | augassign
// _tmp_128: yield_expr | star_expressions
static void *
_tmp_128_rule(Parser *p)
{
Expand All
@@ -16684,24 +16706,24 @@ _tmp_128_rule(Parser *p)
}
void * _res = NULL;
int _mark = p->mark;
{ // '='
Token * _literal ;
{ // yield_expr
expr_ty yield_expr_var ;
if (
(_literal = _PyPegen_expect_token(p, 22 )) // token='='
(yield_expr_var = yield_expr_rule(p )) // yield_expr
)
{
_res = _literal ;
_res = yield_expr_var ;
goto done;
}
p->mark = _mark;
}
{ // augassign
AugOperator* augassign_var ;
{ // star_expressions
expr_ty star_expressions_var ;
if (
(augassign_var = augassign_rule (p)) // augassign
(star_expressions_var = star_expressions_rule (p)) // star_expressions
)
{
_res = augassign_var ;
_res = star_expressions_var ;
goto done;
}
p->mark = _mark;
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -2054,3 +2054,49 @@ _PyPegen_make_module(Parser *p, asdl_seq *a) {
}
return Module(a, type_ignores, p->arena);
}
// Error reporting helpers
expr_ty
_PyPegen_get_invalid_target(expr_ty e)
{
if (e == NULL) {
return NULL;
}
#define VISIT_CONTAINER(CONTAINER, TYPE) do { \
Py_ssize_t len = asdl_seq_LEN(CONTAINER->v.TYPE.elts);\
for (Py_ssize_t i = 0; i < len; i++) {\
expr_ty other = asdl_seq_GET(CONTAINER->v.TYPE.elts, i);\
expr_ty child = _PyPegen_get_invalid_target(other);\
if (child != NULL) {\
return child;\
}\
}\
} while (0)
// We only need to visit List and Tuple nodes recursively as those
// are the only ones that can contain valid names in targets when
// they are parsed as expressions. Any other kind of expression
// that is a container (like Sets or Dicts) is directly invalid and
// we don't need to visit it recursively.
switch (e->kind) {
Comment thread
pablogsal marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
case List_kind: {
VISIT_CONTAINER(e, List);
return NULL;
}
case Tuple_kind: {
VISIT_CONTAINER(e, Tuple);
return NULL;
}
case Starred_kind:
return _PyPegen_get_invalid_target(e->v.Starred.value);
case Name_kind:
case Subscript_kind:
case Attribute_kind:
return NULL;
default:
return e;
}
}
Original file line number
Diff line number
Diff line change
Expand Up
@@ -260,6 +260,10 @@ void *_PyPegen_arguments_parsing_error(Parser *, expr_ty);
int _PyPegen_check_barry_as_flufl(Parser *);
mod_ty _PyPegen_make_module(Parser *, asdl_seq *);
// Error reporting helpers
expr_ty _PyPegen_get_invalid_target(expr_ty e);
void *_PyPegen_parse(Parser *);
#endif
Original file line number
Diff line number
Diff line change
Expand Up
@@ -3164,10 +3164,7 @@ ast_for_expr_stmt(struct compiling *c, const node *n)
expr1 = ast_for_testlist(c, ch);
if (!expr1)
return NULL;
if(!set_context(c, expr1, Store, ch))
return NULL;
/* set_context checks that most expressions are not the left side.
Augmented assignments can only have a name, a subscript, or an
/* Augmented assignments can only have a name, a subscript, or an
attribute on the left, though, so we have to explicitly check for
those. */
switch (expr1->kind) {
Expand All
@@ -3176,10 +3173,16 @@ ast_for_expr_stmt(struct compiling *c, const node *n)
case Subscript_kind:
break;
default:
ast_error(c, ch, "illegal expression for augmented assignment");
ast_error(c, ch, "'%s' is an illegal expression for augmented assignment",
get_expr_name(expr1));
return NULL;
}
/* set_context checks that most expressions are not the left side. */
if(!set_context(c, expr1, Store, ch)) {
return NULL;
}
ch = CHILD(n, 2);
if (TYPE(ch) == testlist)
expr2 = ast_for_testlist(c, ch);
Expand Down
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.