bpo-40334: Produce better error messages on invalid targets (GH-20106)

The following error messages get produced:
- `cannot delete ...` for invalid `del` targets
- `... is an illegal 'for' target` for invalid targets in for
  statements
- `... is an illegal 'with' target` for invalid targets in
  with statements

Additionally, a few `cut`s were added in various places before the
invocation of the `invalid_*` rule, in order to speed things
up.

Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>
This commit is contained in:
Lysandros Nikolaou 2020-06-19 02:10:43 +03:00 committed by GitHub
parent d906f0ec1a
commit 01ece63d42
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 2651 additions and 2256 deletions

View File

@ -93,7 +93,7 @@ assignment[stmt_ty]:
CHECK_VERSION(6, "Variable annotations syntax is", _Py_AnnAssign(a, b, c, 0, EXTRA)) }
| a=(z=star_targets '=' { z })+ b=(yield_expr | star_expressions) !'=' tc=[TYPE_COMMENT] {
_Py_Assign(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
| a=single_target b=augassign c=(yield_expr | star_expressions) {
| a=single_target b=augassign ~ c=(yield_expr | star_expressions) {
_Py_AugAssign(a, b->kind, c, EXTRA) }
| invalid_assignment
@ -121,7 +121,9 @@ yield_stmt[stmt_ty]: y=yield_expr { _Py_Expr(y, EXTRA) }
assert_stmt[stmt_ty]: 'assert' a=expression b=[',' z=expression { z }] { _Py_Assert(a, b, EXTRA) }
del_stmt[stmt_ty]: 'del' a=del_targets { _Py_Delete(a, EXTRA) }
del_stmt[stmt_ty]:
| 'del' a=del_targets &(';' | NEWLINE) { _Py_Delete(a, EXTRA) }
| invalid_del_stmt
import_stmt[stmt_ty]: import_name | import_from
import_name[stmt_ty]: 'import' a=dotted_as_names { _Py_Import(a, EXTRA) }
@ -164,10 +166,11 @@ while_stmt[stmt_ty]:
| 'while' a=named_expression ':' b=block c=[else_block] { _Py_While(a, b, c, EXTRA) }
for_stmt[stmt_ty]:
| 'for' t=star_targets 'in' ex=star_expressions ':' tc=[TYPE_COMMENT] b=block el=[else_block] {
| 'for' t=star_targets 'in' ~ ex=star_expressions ':' tc=[TYPE_COMMENT] b=block el=[else_block] {
_Py_For(t, ex, b, el, NEW_TYPE_COMMENT(p, tc), EXTRA) }
| ASYNC 'for' t=star_targets 'in' ex=star_expressions ':' tc=[TYPE_COMMENT] b=block el=[else_block] {
| ASYNC 'for' t=star_targets 'in' ~ ex=star_expressions ':' tc=[TYPE_COMMENT] b=block el=[else_block] {
CHECK_VERSION(5, "Async for loops are", _Py_AsyncFor(t, ex, b, el, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
| invalid_for_target
with_stmt[stmt_ty]:
| 'with' '(' a=','.with_item+ ','? ')' ':' b=block {
@ -179,7 +182,9 @@ with_stmt[stmt_ty]:
| ASYNC 'with' a=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
CHECK_VERSION(5, "Async with statements are", _Py_AsyncWith(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
with_item[withitem_ty]:
| e=expression o=['as' t=target { t }] { _Py_withitem(e, o, p->arena) }
| e=expression 'as' t=target &(',' | ')' | ':') { _Py_withitem(e, t, p->arena) }
| invalid_with_item
| e=expression { _Py_withitem(e, NULL, p->arena) }
try_stmt[stmt_ty]:
| 'try' ':' b=block f=finally_block { _Py_Try(b, NULL, NULL, f, EXTRA) }
@ -311,7 +316,7 @@ star_named_expression[expr_ty]:
| '*' a=bitwise_or { _Py_Starred(a, Load, EXTRA) }
| named_expression
named_expression[expr_ty]:
| a=NAME ':=' b=expression { _Py_NamedExpr(CHECK(_PyPegen_set_expr_context(p, a, Store)), b, EXTRA) }
| a=NAME ':=' ~ b=expression { _Py_NamedExpr(CHECK(_PyPegen_set_expr_context(p, a, Store)), b, EXTRA) }
| expression !':='
| invalid_named_expression
@ -487,18 +492,20 @@ strings[expr_ty] (memo): a=STRING+ { _PyPegen_concatenate_strings(p, a) }
list[expr_ty]:
| '[' a=[star_named_expressions] ']' { _Py_List(a, Load, EXTRA) }
listcomp[expr_ty]:
| '[' a=named_expression b=for_if_clauses ']' { _Py_ListComp(a, b, EXTRA) }
| '[' a=named_expression ~ b=for_if_clauses ']' { _Py_ListComp(a, b, EXTRA) }
| invalid_comprehension
tuple[expr_ty]:
| '(' a=[y=star_named_expression ',' z=[star_named_expressions] { _PyPegen_seq_insert_in_front(p, y, z) } ] ')' {
_Py_Tuple(a, Load, EXTRA) }
group[expr_ty]: '(' a=(yield_expr | named_expression) ')' { a }
group[expr_ty]:
| '(' a=(yield_expr | named_expression) ')' { a }
| invalid_group
genexp[expr_ty]:
| '(' a=expression b=for_if_clauses ')' { _Py_GeneratorExp(a, b, EXTRA) }
| '(' a=expression ~ b=for_if_clauses ')' { _Py_GeneratorExp(a, b, EXTRA) }
| invalid_comprehension
set[expr_ty]: '{' a=expressions_list '}' { _Py_Set(a, EXTRA) }
setcomp[expr_ty]:
| '{' a=expression b=for_if_clauses '}' { _Py_SetComp(a, b, EXTRA) }
| '{' a=expression ~ b=for_if_clauses '}' { _Py_SetComp(a, b, EXTRA) }
| invalid_comprehension
dict[expr_ty]:
| '{' a=[double_starred_kvpairs] '}' {
@ -514,10 +521,11 @@ kvpair[KeyValuePair*]: a=expression ':' b=expression { _PyPegen_key_value_pair(p
for_if_clauses[asdl_seq*]:
| for_if_clause+
for_if_clause[comprehension_ty]:
| ASYNC 'for' a=star_targets 'in' b=disjunction c=('if' z=disjunction { z })* {
| ASYNC 'for' a=star_targets 'in' ~ b=disjunction c=('if' z=disjunction { z })* {
CHECK_VERSION(6, "Async comprehensions are", _Py_comprehension(a, b, c, 1, p->arena)) }
| 'for' a=star_targets 'in' b=disjunction c=('if' z=disjunction { z })* {
| 'for' a=star_targets 'in' ~ b=disjunction c=('if' z=disjunction { z })* {
_Py_comprehension(a, b, c, 0, p->arena) }
| invalid_for_target
yield_expr[expr_ty]:
| 'yield' 'from' a=expression { _Py_YieldFrom(a, EXTRA) }
@ -587,19 +595,15 @@ single_subscript_attribute_target[expr_ty]:
| a=t_primary '[' b=slices ']' !t_lookahead { _Py_Subscript(a, b, Store, EXTRA) }
del_targets[asdl_seq*]: a=','.del_target+ [','] { a }
# The lookaheads to del_target_end ensure that we don't match expressions where a prefix of the
# expression matches our rule, thereby letting these cases fall through to invalid_del_target.
del_target[expr_ty] (memo):
| a=t_primary '.' b=NAME &del_target_end { _Py_Attribute(a, b->v.Name.id, Del, EXTRA) }
| a=t_primary '[' b=slices ']' &del_target_end { _Py_Subscript(a, b, Del, EXTRA) }
| a=t_primary '.' b=NAME !t_lookahead { _Py_Attribute(a, b->v.Name.id, Del, EXTRA) }
| a=t_primary '[' b=slices ']' !t_lookahead { _Py_Subscript(a, b, Del, EXTRA) }
| del_t_atom
del_t_atom[expr_ty]:
| a=NAME &del_target_end { _PyPegen_set_expr_context(p, a, Del) }
| a=NAME { _PyPegen_set_expr_context(p, a, Del) }
| '(' a=del_target ')' { _PyPegen_set_expr_context(p, a, Del) }
| '(' a=[del_targets] ')' { _Py_Tuple(a, Del, EXTRA) }
| '[' a=[del_targets] ']' { _Py_List(a, Del, EXTRA) }
| invalid_del_target
del_target_end: ')' | ']' | ',' | ';' | NEWLINE
targets[asdl_seq*]: a=','.target+ [','] { a }
target[expr_ty] (memo):
@ -650,8 +654,8 @@ invalid_assignment:
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "illegal target for annotation") }
| (star_targets '=')* a=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))) }
GET_INVALID_TARGET(a),
"cannot assign to %s", _PyPegen_get_expr_name(GET_INVALID_TARGET(a))) }
| (star_targets '=')* a=yield_expr '=' { RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "assignment to yield expression not possible") }
| a=star_expressions augassign (yield_expr | star_expressions) {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
@ -659,7 +663,14 @@ invalid_assignment:
"'%s' is an illegal expression for augmented assignment",
_PyPegen_get_expr_name(a)
)}
invalid_del_stmt:
| 'del' a=star_expressions {
GET_INVALID_DEL_TARGET(a) != NULL ?
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
GET_INVALID_DEL_TARGET(a),
"cannot delete %s", _PyPegen_get_expr_name(GET_INVALID_DEL_TARGET(a))
) :
RAISE_SYNTAX_ERROR("invalid syntax") }
invalid_block:
| NEWLINE !INDENT { RAISE_INDENTATION_ERROR("expected an indented block") }
invalid_comprehension:
@ -682,9 +693,25 @@ invalid_lambda_star_etc:
invalid_double_type_comments:
| TYPE_COMMENT NEWLINE TYPE_COMMENT NEWLINE INDENT {
RAISE_SYNTAX_ERROR("Cannot have two type comments on def") }
invalid_del_target:
| a=star_expression &del_target_end {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "cannot delete %s", _PyPegen_get_expr_name(a)) }
invalid_with_item:
| expression 'as' a=expression {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
GET_INVALID_TARGET(a),
"cannot assign to %s", _PyPegen_get_expr_name(GET_INVALID_TARGET(a))
) }
invalid_for_target:
| ASYNC? 'for' a=star_expressions {
GET_INVALID_FOR_TARGET(a) != NULL ?
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
GET_INVALID_FOR_TARGET(a),
"cannot assign to %s", _PyPegen_get_expr_name(GET_INVALID_FOR_TARGET(a))
) :
RAISE_SYNTAX_ERROR("invalid syntax") }
invalid_group:
| '(' a=starred_expression ')' {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "can't use starred expression here") }
invalid_import_from_targets:
| import_from_as_names ',' {
RAISE_SYNTAX_ERROR("trailing comma not allowed without surrounding parentheses") }

View File

@ -251,9 +251,9 @@ class ExceptionTests(unittest.TestCase):
check('def f():\n x, y: int', 2, 3)
check('[*x for x in xs]', 1, 2)
check('foo(x for x in range(10), 100)', 1, 5)
check('for 1 in []: pass', 1, 5)
check('(yield i) = 2', 1, 2)
check('def f(*):\n pass', 1, 8)
check('for 1 in []: pass', 1, 7)
@cpython_only
def testSettingException(self):

View File

@ -164,6 +164,65 @@ SyntaxError: 'tuple' is an illegal expression for augmented assignment
Traceback (most recent call last):
SyntaxError: 'list' is an illegal expression for augmented assignment
Invalid targets in `for` loops and `with` statements should also
produce a specialized error message
>>> for a() in b: pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> for (a, b()) in b: pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> for [a, b()] in b: pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> for (*a, b, c+1) in b: pass
Traceback (most recent call last):
SyntaxError: cannot assign to operator
>>> for (x, *(y, z.d())) in b: pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> for a, b() in c: pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> for a, b, (c + 1, d()): pass
Traceback (most recent call last):
SyntaxError: cannot assign to operator
>>> for i < (): pass
Traceback (most recent call last):
SyntaxError: invalid syntax
>>> with a as b(): pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> with a as (b, c()): pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> with a as [b, c()]: pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> with a as (*b, c, d+1): pass
Traceback (most recent call last):
SyntaxError: cannot assign to operator
>>> with a as (x, *(y, z.d())): pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> with a as b, c as d(): pass
Traceback (most recent call last):
SyntaxError: cannot assign to function call
>>> p = p =
Traceback (most recent call last):
SyntaxError: invalid syntax
@ -739,7 +798,7 @@ class SyntaxTestCase(unittest.TestCase):
self._check_error("del (1, 2)", "delete literal")
self._check_error("del None", "delete None")
self._check_error("del *x", "delete starred")
self._check_error("del (*x)", "delete starred")
self._check_error("del (*x)", "use starred expression")
self._check_error("del (*x,)", "delete starred")
self._check_error("del [*x,]", "delete starred")
self._check_error("del f()", "delete function call")

File diff suppressed because it is too large Load Diff

View File

@ -380,7 +380,6 @@ _PyPegen_raise_error(Parser *p, PyObject *errtype, const char *errmsg, ...)
return NULL;
}
void *
_PyPegen_raise_error_known_location(Parser *p, PyObject *errtype,
Py_ssize_t lineno, Py_ssize_t col_offset,
@ -2086,7 +2085,7 @@ _PyPegen_make_module(Parser *p, asdl_seq *a) {
// Error reporting helpers
expr_ty
_PyPegen_get_invalid_target(expr_ty e)
_PyPegen_get_invalid_target(expr_ty e, TARGETS_TYPE targets_type)
{
if (e == NULL) {
return NULL;
@ -2096,7 +2095,7 @@ _PyPegen_get_invalid_target(expr_ty e)
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);\
expr_ty child = _PyPegen_get_invalid_target(other, targets_type);\
if (child != NULL) {\
return child;\
}\
@ -2110,16 +2109,29 @@ _PyPegen_get_invalid_target(expr_ty e)
// we don't need to visit it recursively.
switch (e->kind) {
case List_kind: {
case List_kind:
VISIT_CONTAINER(e, List);
return NULL;
}
case Tuple_kind: {
case Tuple_kind:
VISIT_CONTAINER(e, Tuple);
return NULL;
}
case Starred_kind:
return _PyPegen_get_invalid_target(e->v.Starred.value);
if (targets_type == DEL_TARGETS) {
return e;
}
return _PyPegen_get_invalid_target(e->v.Starred.value, targets_type);
case Compare_kind:
// This is needed, because the `a in b` in `for a in b` gets parsed
// as a comparison, and so we need to search the left side of the comparison
// for invalid targets.
if (targets_type == FOR_TARGETS) {
cmpop_ty cmpop = (cmpop_ty) asdl_seq_GET(e->v.Compare.ops, 0);
if (cmpop == In) {
return _PyPegen_get_invalid_target(e->v.Compare.left, targets_type);
}
return NULL;
}
return e;
case Name_kind:
case Subscript_kind:
case Attribute_kind:

View File

@ -263,11 +263,21 @@ 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);
typedef enum {
STAR_TARGETS,
DEL_TARGETS,
FOR_TARGETS
} TARGETS_TYPE;
expr_ty _PyPegen_get_invalid_target(expr_ty e, TARGETS_TYPE targets_type);
#define GET_INVALID_TARGET(e) (expr_ty)CHECK(_PyPegen_get_invalid_target(e, STAR_TARGETS))
#define GET_INVALID_DEL_TARGET(e) (expr_ty)CHECK_NULL_ALLOWED(_PyPegen_get_invalid_target(e, DEL_TARGETS))
#define GET_INVALID_FOR_TARGET(e) (expr_ty)CHECK_NULL_ALLOWED(_PyPegen_get_invalid_target(e, FOR_TARGETS))
void *_PyPegen_arguments_parsing_error(Parser *, expr_ty);
void *_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args);
// Generated function in parse.c - function definition in python.gram
void *_PyPegen_parse(Parser *);
#endif