Skip to content

bpo-43914: Correctly highlight SyntaxError exceptions for invalid generator expression in function calls #28576

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

Merged
merged 1 commit into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Grammar/python.gram
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ invalid_arguments:
RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, PyPegen_last_item(b, comprehension_ty)->target, "Generator expression must be parenthesized") }
| a=NAME b='=' expression for_if_clauses {
RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "invalid syntax. Maybe you meant '==' or ':=' instead of '='?")}
| a=args for_if_clauses { _PyPegen_nonparen_genexp_in_call(p, a) }
| a=args b=for_if_clauses { _PyPegen_nonparen_genexp_in_call(p, a, b) }
| args ',' a=expression b=for_if_clauses {
RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, asdl_seq_GET(b, b->size-1)->target, "Generator expression must be parenthesized") }
| a=args ',' args { _PyPegen_arguments_parsing_error(p, a) }
Expand Down
13 changes: 12 additions & 1 deletion Lib/test/test_syntax.py
Original file line number Diff line number Diff line change
Expand Up @@ -1274,7 +1274,8 @@
class SyntaxTestCase(unittest.TestCase):

def _check_error(self, code, errtext,
filename="<testcase>", mode="exec", subclass=None, lineno=None, offset=None):
filename="<testcase>", mode="exec", subclass=None,
lineno=None, offset=None, end_lineno=None, end_offset=None):
"""Check that compiling code raises SyntaxError with errtext.

errtest is a regular expression that must be present in the
Expand All @@ -1294,6 +1295,11 @@ def _check_error(self, code, errtext,
self.assertEqual(err.lineno, lineno)
if offset is not None:
self.assertEqual(err.offset, offset)
if end_lineno is not None:
self.assertEqual(err.end_lineno, end_lineno)
if end_offset is not None:
self.assertEqual(err.end_offset, end_offset)

else:
self.fail("compile() did not raise SyntaxError")

Expand Down Expand Up @@ -1433,6 +1439,11 @@ def test_kwargs_last3(self):
self._check_error("int(**{'base': 10}, *['2'])",
"iterable argument unpacking follows "
"keyword argument unpacking")

def test_generator_in_function_call(self):
self._check_error("foo(x, y for y in range(3) for z in range(2) if z , p)",
"Generator expression must be parenthesized",
lineno=1, end_lineno=1, offset=11, end_offset=53)

def test_empty_line_after_linecont(self):
# See issue-40847
Expand Down
6 changes: 3 additions & 3 deletions Parser/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -17880,15 +17880,15 @@ invalid_arguments_rule(Parser *p)
}
D(fprintf(stderr, "%*c> invalid_arguments[%d-%d]: %s\n", p->level, ' ', _mark, p->mark, "args for_if_clauses"));
expr_ty a;
asdl_comprehension_seq* for_if_clauses_var;
asdl_comprehension_seq* b;
if (
(a = args_rule(p)) // args
&&
(for_if_clauses_var = for_if_clauses_rule(p)) // for_if_clauses
(b = for_if_clauses_rule(p)) // for_if_clauses
)
{
D(fprintf(stderr, "%*c+ invalid_arguments[%d-%d]: %s succeeded!\n", p->level, ' ', _mark, p->mark, "args for_if_clauses"));
_res = _PyPegen_nonparen_genexp_in_call ( p , a );
_res = _PyPegen_nonparen_genexp_in_call ( p , a , b );
if (_res == NULL && PyErr_Occurred()) {
p->error_indicator = 1;
D(p->level--);
Expand Down
16 changes: 14 additions & 2 deletions Parser/pegen.c
Original file line number Diff line number Diff line change
Expand Up @@ -2551,8 +2551,17 @@ void *_PyPegen_arguments_parsing_error(Parser *p, expr_ty e) {
return RAISE_SYNTAX_ERROR(msg);
}


static inline expr_ty
_PyPegen_get_last_comprehension_item(comprehension_ty comprehension) {
if (comprehension->ifs == NULL || asdl_seq_LEN(comprehension->ifs) == 0) {
return comprehension->iter;
}
return PyPegen_last_item(comprehension->ifs, expr_ty);
}

void *
_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args)
_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args, asdl_comprehension_seq *comprehensions)
{
/* The rule that calls this function is 'args for_if_clauses'.
For the input f(L, x for x in y), L and x are in args and
Expand All @@ -2566,8 +2575,11 @@ _PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args)
return NULL;
}

return RAISE_SYNTAX_ERROR_STARTING_FROM(
comprehension_ty last_comprehension = PyPegen_last_item(comprehensions, comprehension_ty);

return RAISE_SYNTAX_ERROR_KNOWN_RANGE(
(expr_ty) asdl_seq_GET(args->v.Call.args, len - 1),
_PyPegen_get_last_comprehension_item(last_comprehension),
"Generator expression must be parenthesized"
);
}
Expand Down
2 changes: 1 addition & 1 deletion Parser/pegen.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ _RAISE_SYNTAX_ERROR_INVALID_TARGET(Parser *p, TARGETS_TYPE type, void *e)
}

void *_PyPegen_arguments_parsing_error(Parser *, expr_ty);
void *_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args);
void *_PyPegen_nonparen_genexp_in_call(Parser *p, expr_ty args, asdl_comprehension_seq *comprehensions);


// Generated function in parse.c - function definition in python.gram
Expand Down