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.

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

ast.parse(type_comments=True) fails on some parenthesized context managers #111420

Closed
JelleZijlstra opened this issue Oct 28, 2023 · 2 comments
Closed
Labels
topic-parser type-bug An unexpected behavior, bug, or error

Comments

@JelleZijlstra
Copy link
Member

JelleZijlstra commented Oct 28, 2023

Bug report

Bug description:

ast.parse(..., type_comments=True) doesn't deal well with parenthesized context managers, as introduced by PEP-617.

>>> ast.dump(ast.parse("with (a as b): # type: something\n    pass", type_comments=True))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    ast.dump(ast.parse("with (a as b): # type: something\n    pass", type_comments=True))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jelle/py/cpython/Lib/ast.py", line 61, in parse
    return compile(
           ^^^^^^^^
  File "<unknown>", line 1
    with (a as b): # type: something
                           ^^^^^^^^^
SyntaxError: invalid syntax

But this works without the parentheses:

>>> ast.dump(ast.parse("with a as b: # type: something\n    pass", type_comments=True))
"Module(body=[With(items=[withitem(context_expr=Name(id='a', ctx=Load()), optional_vars=Name(id='b', ctx=Store()))], body=[Pass()], type_comment='something')], type_ignores=[])"

This code gets interpreted incorrectly:

>>> ast.dump(ast.parse("with (a, b): # type: something\n    pass", type_comments=True))
"Module(body=[With(items=[withitem(context_expr=Tuple(elts=[Name(id='a', ctx=Load()), Name(id='b', ctx=Load())], ctx=Load()))], body=[Pass()], type_comment='something')], type_ignores=[])"

There is a single withitem containing a tuple, when it should instead be two withitems, as happens without the comment:

>>> ast.dump(ast.parse("with (a, b):\n    pass", type_comments=True))
"Module(body=[With(items=[withitem(context_expr=Name(id='a', ctx=Load())), withitem(context_expr=Name(id='b', ctx=Load()))], body=[Pass()])], type_ignores=[])"

Found this while looking into psf/black#3677.

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

@JelleZijlstra JelleZijlstra added type-bug An unexpected behavior, bug, or error topic-parser labels Oct 28, 2023
@tomasr8
Copy link
Contributor

tomasr8 commented Oct 28, 2023

The problem with parentheses seems like a missing/incomplete rule in the grammar to me:

cpython/Grammar/python.gram

Lines 392 to 402 in 2655369

with_stmt[stmt_ty]:
| invalid_with_stmt_indent
| 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' b=block {
CHECK_VERSION(stmt_ty, 9, "Parenthesized context managers are", _PyAST_With(a, b, NULL, EXTRA)) }
| 'with' a[asdl_withitem_seq*]=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
_PyAST_With(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
| 'async' 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' b=block {
CHECK_VERSION(stmt_ty, 5, "Async with statements are", _PyAST_AsyncWith(a, b, NULL, EXTRA)) }
| 'async' 'with' a[asdl_withitem_seq*]=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
CHECK_VERSION(stmt_ty, 5, "Async with statements are", _PyAST_AsyncWith(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
| invalid_with_stmt

There seems to be one rule which allows parentheses but not type comments another one which does the opposite, but no rule which allows both parentheses and type comments at the same time.

Either of these changes fixes the syntax error and extracts the type comment correctly:

with_stmt[stmt_ty]:
    | invalid_with_stmt_indent
-   | 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' b=block {
-       CHECK_VERSION(stmt_ty, 9, "Parenthesized context managers are", _PyAST_With(a, b, NULL, EXTRA)) }
+   | 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' tc=[TYPE_COMMENT] b=block {
+       CHECK_VERSION(stmt_ty, 9, "Parenthesized context managers are", _PyAST_With(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
    | 'with' a[asdl_withitem_seq*]=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
        _PyAST_With(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
    | 'async' 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' b=block {
       CHECK_VERSION(stmt_ty, 5, "Async with statements are", _PyAST_AsyncWith(a, b, NULL, EXTRA)) }
    | 'async' 'with' a[asdl_withitem_seq*]=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
       CHECK_VERSION(stmt_ty, 5, "Async with statements are", _PyAST_AsyncWith(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
    | invalid_with_stmt
with_stmt[stmt_ty]:
    | invalid_with_stmt_indent
    | 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' b=block {
        CHECK_VERSION(stmt_ty, 9, "Parenthesized context managers are", _PyAST_With(a, b, NULL, EXTRA)) }
    | 'with' a[asdl_withitem_seq*]=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
        _PyAST_With(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
+   | 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ')' ':' tc=[TYPE_COMMENT] b=block {
+       _PyAST_With(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA) }
    | 'async' 'with' '(' a[asdl_withitem_seq*]=','.with_item+ ','? ')' ':' b=block {
       CHECK_VERSION(stmt_ty, 5, "Async with statements are", _PyAST_AsyncWith(a, b, NULL, EXTRA)) }
    | 'async' 'with' a[asdl_withitem_seq*]=','.with_item+ ':' tc=[TYPE_COMMENT] b=block {
       CHECK_VERSION(stmt_ty, 5, "Async with statements are", _PyAST_AsyncWith(a, b, NEW_TYPE_COMMENT(p, tc), EXTRA)) }
    | invalid_with_stmt

@sobolevn
Copy link
Member

@tomasr8 please, send a PR! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-parser type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants