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

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+488 −0
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1d2c4fa
[syntax-errors] Parenthesized context managers before Python 3.9
ntBre Mar 5, 2025
359686d
simplify tests, improve error message, catch error
ntBre Mar 5, 2025
557142b
clean up unused snapshot
ntBre Mar 5, 2025
73f1047
move check into parse_with_items
ntBre Mar 6, 2025
f524953
document variant
ntBre Mar 6, 2025
256279b
expand test_ok cases
ntBre Mar 6, 2025
4d74013
replace numbered link
ntBre Mar 6, 2025
ba288ba
fix missing block end
ntBre Mar 6, 2025
20c8b0d
Merge branch 'main' into brent/syn-parenthesized-context-managers
ntBre Mar 6, 2025
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: 2 additions & 0 deletions crates/ruff_python_parser/resources/inline/err/parenthesized_context_manager_py38.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# parse_options: {"target-version": "3.8"}
with (foo as x, bar as y): ...
2 changes: 2 additions & 0 deletions crates/ruff_python_parser/resources/inline/ok/parenthesized_context_manager_py39.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# parse_options: {"target-version": "3.9"}
with (foo as x, bar as y): ...
5 changes: 5 additions & 0 deletions crates/ruff_python_parser/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

}

impl Display for UnsupportedSyntaxError {
Expand All @@ -553,6 +554,9 @@ impl Display for UnsupportedSyntaxError {
UnsupportedSyntaxErrorKind::TypeParamDefault => {
"Cannot set default type for a type parameter"
}
UnsupportedSyntaxErrorKind::ParenthesizedContextManager => {
"Cannot use parentheses within a `with` statement"
}
};
write!(
f,
Expand All @@ -575,6 +579,7 @@ impl UnsupportedSyntaxErrorKind {
UnsupportedSyntaxErrorKind::TypeParameterList => PythonVersion::PY312,
UnsupportedSyntaxErrorKind::TypeAliasStatement => PythonVersion::PY312,
UnsupportedSyntaxErrorKind::TypeParamDefault => PythonVersion::PY313,
UnsupportedSyntaxErrorKind::ParenthesizedContextManager => PythonVersion::PY39,
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions crates/ruff_python_parser/src/parser/statement.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2062,6 +2062,7 @@ impl<'src> Parser<'src> {
// We'll start with the assumption that the with items are parenthesized.
let mut with_item_kind = WithItemKind::Parenthesized;

let open_paren_range = self.current_token_range();
self.bump(TokenKind::Lpar);

let mut parsed_with_items = vec![];
Expand Down Expand Up @@ -2103,6 +2104,17 @@ impl<'src> Parser<'src> {
};
self.add_error(error, &parsed_with_item.item.context_expr);
}
// test_ok parenthesized_context_manager_py39
// # parse_options: {"target-version": "3.9"}
// with (foo as x, bar as y): ...

// 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):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that syntax is actually okay. I think (foo, bar) is just a tuple, so it falls under the expression grammar. The tuple doesn't have an __enter__ method, so it causes a runtime error, but it does parse and even compile I guess.

As far as I can tell, the problem with parentheses is specific to the as case since that's part of the with statement grammar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Python 3.7.9 (default, Aug 23 2020, 00:57:53)
[Clang 10.0.1 ] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ast
>>> ast.parse("with (foo, bar): ...")
<_ast.Module object at 0x70849fd20c10>
>>> ast.dump(_)
"Module(body=[With(items=[withitem(context_expr=Tuple(elts=[Name(id='foo', ctx=Load()), Name(id='bar', ctx=Load())], ctx=Load()), optional_vars=None)], body=[Expr(value=Ellipsis())])])"

Copy link
Member

Choose a reason for hiding this comment

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

Should we move this check down below where we're sure that the parenthesis belongs to the with items and not the expression? i.e., on line 2162

        if with_item_kind.is_parenthesized() {

Copy link
Member

Choose a reason for hiding this comment

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

Or, in parse_with_items like:

        if self.at(TokenKind::Lpar) {
			let lpar_range = self.current_token_range();
            if let Some(items) = self.try_parse_parenthesized_with_items() {
				// check target version and raise error
                self.expect(TokenKind::Rpar);
                items
            } else {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh nice, I moved it to parse_with_items. I just had to add a separate check for optional_vars otherwise we get a false positive on the case Micha mentioned above. I added that as a test_ok case to be sure.

UnsupportedSyntaxErrorKind::ParenthesizedContextManager,
open_paren_range,
);
} else if self.at(TokenKind::Rpar)
// test_err with_items_parenthesized_missing_colon
// # `)` followed by a newline
Expand Down
80 changes: 80 additions & 0 deletions ...f_python_parser/tests/snapshots/invalid_syntax@parenthesized_context_manager_py38.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/parenthesized_context_manager_py38.py
---
## AST

```
Module(
ModModule {
range: 0..74,
body: [
With(
StmtWith {
range: 43..73,
is_async: false,
items: [
WithItem {
range: 49..57,
context_expr: Name(
ExprName {
range: 49..52,
id: Name("foo"),
ctx: Load,
},
),
optional_vars: Some(
Name(
ExprName {
range: 56..57,
id: Name("x"),
ctx: Store,
},
),
),
},
WithItem {
range: 59..67,
context_expr: Name(
ExprName {
range: 59..62,
id: Name("bar"),
ctx: Load,
},
),
optional_vars: Some(
Name(
ExprName {
range: 66..67,
id: Name("y"),
ctx: Store,
},
),
),
},
],
body: [
Expr(
StmtExpr {
range: 70..73,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 70..73,
},
),
},
),
],
},
),
],
},
)
```
## Unsupported Syntax Errors

|
1 | # parse_options: {"target-version": "3.8"}
2 | with (foo as x, bar as y): ...
| ^ Syntax Error: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
73 changes: 73 additions & 0 deletions ...uff_python_parser/tests/snapshots/valid_syntax@parenthesized_context_manager_py39.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/parenthesized_context_manager_py39.py
---
## AST

```
Module(
ModModule {
range: 0..74,
body: [
With(
StmtWith {
range: 43..73,
is_async: false,
items: [
WithItem {
range: 49..57,
context_expr: Name(
ExprName {
range: 49..52,
id: Name("foo"),
ctx: Load,
},
),
optional_vars: Some(
Name(
ExprName {
range: 56..57,
id: Name("x"),
ctx: Store,
},
),
),
},
WithItem {
range: 59..67,
context_expr: Name(
ExprName {
range: 59..62,
id: Name("bar"),
ctx: Load,
},
),
optional_vars: Some(
Name(
ExprName {
range: 66..67,
id: Name("y"),
ctx: Store,
},
),
),
},
],
body: [
Expr(
StmtExpr {
range: 70..73,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 70..73,
},
),
},
),
],
},
),
],
},
)
```
Loading