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

[syntax-errors] Parenthesized context managers before Python 3.9 #16523

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Mar 5, 2025

Summary

I thought this was very complicated based on the comment here: #16106 (comment) and on some of the discussion in the CPython issue here: python/cpython#56991. However, after a little bit of experimentation, I think it boils down to this example:

with (x as y): ...

The issue is parentheses around a with item with an optional_var, as we (and Python) call the trailing variable name (y in this case). It's not actually about line breaks after all, except that line breaks are allowed in parenthesized expressions, which explains the validity of cases like

>>> with (
...     x,
...     y
... ) as foo:
...     pass
... 

even on Python 3.8.

I followed pyright's example again here on the diagnostic range (just the opening paren) and the wording of the error.

Test Plan

Inline tests

Summary
--

WIP currently I just added all of the valid cases from Alex's comment here
#16106 (comment)

Test Plan
--
Inline tests
@ntBre ntBre added parser Related to the parser preview Related to preview mode features labels Mar 5, 2025
Copy link
Contributor

github-actions bot commented Mar 5, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@ntBre ntBre marked this pull request as ready for review March 5, 2025 18:45
// test_err parenthesized_context_manager_py38
// # parse_options: {"target-version": "3.8"}
// with (foo as x, bar as y): ...
self.add_unsupported_syntax_error(
Copy link
Member

Choose a reason for hiding this comment

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

Do we also need to emit the same diagnostic on line 2144, e.g. for with (foo, bar):

@@ -536,6 +536,7 @@ pub enum UnsupportedSyntaxErrorKind {
TypeParameterList,
TypeAliasStatement,
TypeParamDefault,
ParenthesizedContextManager,
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to add documentation, similar to what you did for other variants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Related to the parser preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants