Skip to content
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.

Sign up for GitHub

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

bpo-43797: Improve syntax error for invalid comparisons #25317

Merged
merged 4 commits into from
Apr 12, 2021
Merged

bpo-43797: Improve syntax error for invalid comparisons #25317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c4ccd9
bpo-43797: Improve syntax error for invalid comparisons
pablogsal Apr 9, 2021
80a727c
Update Lib/test/test_fstring.py
pablogsal Apr 10, 2021
c952132
Apply review comments
pablogsal Apr 10, 2021
6599121
can't -> cannot
pablogsal Apr 10, 2021
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
33 changes: 24 additions & 9 deletions Grammar/python.gram
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,17 +163,20 @@ dotted_name[expr_ty]:
| NAME

if_stmt[stmt_ty]:
| 'if' a=named_expression &&':' b=block c=elif_stmt {
| 'if' a=named_expression ':' b=block c=elif_stmt {
_PyAST_If(a, b, CHECK(asdl_stmt_seq*, _PyPegen_singleton_seq(p, c)), EXTRA) }
| 'if' a=named_expression &&':' b=block c=[else_block] { _PyAST_If(a, b, c, EXTRA) }
| 'if' a=named_expression ':' b=block c=[else_block] { _PyAST_If(a, b, c, EXTRA) }
| invalid_if_stmt
elif_stmt[stmt_ty]:
| 'elif' a=named_expression &&':' b=block c=elif_stmt {
| 'elif' a=named_expression ':' b=block c=elif_stmt {
_PyAST_If(a, b, CHECK(asdl_stmt_seq*, _PyPegen_singleton_seq(p, c)), EXTRA) }
| 'elif' a=named_expression &&':' b=block c=[else_block] { _PyAST_If(a, b, c, EXTRA) }
| 'elif' a=named_expression ':' b=block c=[else_block] { _PyAST_If(a, b, c, EXTRA) }
| invalid_elif_stmt
else_block[asdl_stmt_seq*]: 'else' &&':' b=block { b }

while_stmt[stmt_ty]:
| 'while' a=named_expression &&':' b=block c=[else_block] { _PyAST_While(a, b, c, EXTRA) }
| 'while' a=named_expression ':' b=block c=[else_block] { _PyAST_While(a, b, c, EXTRA) }
| invalid_while_stmt

for_stmt[stmt_ty]:
| 'for' t=star_targets 'in' ~ ex=star_expressions &&':' tc=[TYPE_COMMENT] b=block el=[else_block] {
Expand Down Expand Up @@ -438,10 +441,11 @@ star_named_expressions[asdl_expr_seq*]: a[asdl_expr_seq*]=','.star_named_express
star_named_expression[expr_ty]:
| '*' a=bitwise_or { _PyAST_Starred(a, Load, EXTRA) }
| named_expression

named_expression[expr_ty]:
| a=NAME ':=' ~ b=expression { _PyAST_NamedExpr(CHECK(expr_ty, _PyPegen_set_expr_context(p, a, Store)), b, EXTRA) }
| expression !':='
| invalid_named_expression
| expression !':='

annotated_rhs[expr_ty]: yield_expr | star_expressions

Expand Down Expand Up @@ -772,6 +776,12 @@ invalid_named_expression:
| a=expression ':=' expression {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
a, "cannot use assignment expressions with %s", _PyPegen_get_expr_name(a)) }
| a=NAME b='=' bitwise_or !('='|':='|',') {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(b, "invalid syntax. Maybe you meant '==' or ':=' instead of '='?") }
| !(list|tuple|genexp|'True'|'None'|'False') a=bitwise_or b='=' bitwise_or !('='|':='|',') {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(b, "cannot assign to %s here. Maybe you meant '==' instead of '='?",
_PyPegen_get_expr_name(a)) }

invalid_assignment:
| a=invalid_ann_assign_target ':' expression {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
Expand Down Expand Up @@ -841,9 +851,9 @@ invalid_for_target:

invalid_group:
| '(' a=starred_expression ')' {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "can't use starred expression here") }
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "cannot use starred expression here") }
| '(' a='**' expression ')' {
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "can't use double starred expression here") }
RAISE_SYNTAX_ERROR_KNOWN_LOCATION(a, "cannot use double starred expression here") }
invalid_import_from_targets:
| import_from_as_names ',' {
RAISE_SYNTAX_ERROR("trailing comma not allowed without surrounding parentheses") }
Expand All @@ -860,6 +870,11 @@ invalid_except_block:

invalid_match_stmt:
| "match" subject_expr !':' { CHECK_VERSION(void*, 10, "Pattern matching is", RAISE_SYNTAX_ERROR("expected ':'") ) }

invalid_case_block:
| "case" patterns guard? !':' { RAISE_SYNTAX_ERROR("expected ':'") }
invalid_if_stmt:
| 'if' named_expression NEWLINE { RAISE_SYNTAX_ERROR("expected ':'") }
invalid_elif_stmt:
| 'elif' named_expression NEWLINE { RAISE_SYNTAX_ERROR("expected ':'") }
invalid_while_stmt:
| 'while' named_expression NEWLINE { RAISE_SYNTAX_ERROR("expected ':'") }
8 changes: 4 additions & 4 deletions Lib/test/test_cmd_line_script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,8 @@ def test_syntaxerror_unindented_caret_position(self):
script_name = _make_test_script(script_dir, 'script', script)
exitcode, stdout, stderr = assert_python_failure(script_name)
text = io.TextIOWrapper(io.BytesIO(stderr), 'ascii').read()
# Confirm that the caret is located under the first 1 character
self.assertIn("\n 1 + 1 = 2\n ^", text)
# Confirm that the caret is located under the '=' sign
self.assertIn("\n 1 + 1 = 2\n ^\n", text)

def test_syntaxerror_indented_caret_position(self):
script = textwrap.dedent("""\
Expand All @@ -613,7 +613,7 @@ def test_syntaxerror_indented_caret_position(self):
exitcode, stdout, stderr = assert_python_failure(script_name)
text = io.TextIOWrapper(io.BytesIO(stderr), 'ascii').read()
# Confirm that the caret is located under the first 1 character
self.assertIn("\n 1 + 1 = 2\n ^", text)
self.assertIn("\n 1 + 1 = 2\n ^\n", text)

# Try the same with a form feed at the start of the indented line
script = (
Expand All @@ -624,7 +624,7 @@ def test_syntaxerror_indented_caret_position(self):
exitcode, stdout, stderr = assert_python_failure(script_name)
text = io.TextIOWrapper(io.BytesIO(stderr), "ascii").read()
self.assertNotIn("\f", text)
self.assertIn("\n 1 + 1 = 2\n ^", text)
self.assertIn("\n 1 + 1 = 2\n ^\n", text)

def test_syntaxerror_multi_line_fstring(self):
script = 'foo = f"""{}\nfoo"""\n'
Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_codeop.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,6 @@ def test_invalid(self):
ai("a = 'a\\\n")

ai("a = 1","eval")
ai("a = (","eval")
ai("]","eval")
ai("())","eval")
ai("[}","eval")
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ def baz():
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('(yield i) = 2', 1, 11)
check('def f(*):\n pass', 1, 8)

@cpython_only
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_fstring.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ def test_conversions(self):
])

def test_assignment(self):
self.assertAllRaise(SyntaxError, 'invalid syntax',
self.assertAllRaise(SyntaxError, r'invalid syntax',
["f'' = 3",
"f'{0}' = x",
"f'{x}' = x",
Expand Down Expand Up @@ -1276,11 +1276,11 @@ def test_with_an_underscore_and_a_comma_in_format_specifier(self):
f'{1:_,}'

def test_syntax_error_for_starred_expressions(self):
error_msg = re.escape("can't use starred expression here")
error_msg = re.escape("cannot use starred expression here")
with self.assertRaisesRegex(SyntaxError, error_msg):
compile("f'{*a}'", "?", "exec")

error_msg = re.escape("can't use double starred expression here")
error_msg = re.escape("cannot use double starred expression here")
with self.assertRaisesRegex(SyntaxError, error_msg):
compile("f'{**a}'", "?", "exec")

Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_generators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2013,7 +2013,7 @@ def printsolution(self, x):
>>> def f(): (yield bar) = y
Traceback (most recent call last):
...
SyntaxError: cannot assign to yield expression
SyntaxError: cannot assign to yield expression here. Maybe you meant '==' instead of '='?

>>> def f(): (yield bar) += y
Traceback (most recent call last):
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_genexps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
>>> dict(a = i for i in range(10))
Traceback (most recent call last):
...
SyntaxError: invalid syntax
SyntaxError: invalid syntax. Maybe you meant '==' or ':=' instead of '='?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the suggestion here is very useful. More likely some other misunderstanding or typo happened.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this particular example yes, but imagine something like all(x = y for x in elements for y in other_elements). As it makes more sense depending on the actual function, is going to be hard to contextualize.


Verify that parenthesis are required when used as a keyword argument value

Expand Down
Loading