mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-17 22:07:42 +00:00
Fix syntax error false positives on parenthesized context managers (#20846)
This PR resolves the issue noticed in https://github.com/astral-sh/ruff/pull/20777#discussion_r2417233227. Namely, cases like this were being flagged as syntax errors despite being perfectly valid on Python 3.8: ```pycon Python 3.8.20 (default, Oct 2 2024, 16:34:12) [Clang 18.1.8 ] on linux Type "help", "copyright", "credits" or "license" for more information. >>> with (open("foo.txt", "w")): ... ... Ellipsis >>> with (open("foo.txt", "w")) as f: print(f) ... <_io.TextIOWrapper name='foo.txt' mode='w' encoding='UTF-8'> ``` The second of these was already allowed but not the first: ```shell > ruff check --target-version py38 --ignore ALL - <<EOF with (open("foo.txt", "w")): ... with (open("foo.txt", "w")) as f: print(f) EOF invalid-syntax: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9) --> -:1:6 | 1 | with (open("foo.txt", "w")): ... | ^ 2 | with (open("foo.txt", "w")) as f: print(f) | Found 1 error. ``` There was some discussion of related cases in https://github.com/astral-sh/ruff/pull/16523#discussion_r1984657793, but it seems I overlooked the single-element case when flagging tuples. As suggested in the other thread, we can just check if there's more than one element or a trailing comma, which will cause the tuple parsing on <=3.8 and avoid the false positives.
This commit is contained in:
Brent Westbrook
GitHub
parent
373fe8a39c
commit
71f8389f61
6 changed files with 222 additions and 149 deletions
|
|
@ -807,31 +807,6 @@ with (
|
|||
```
|
||||
|
||||
|
||||
### Unsupported Syntax Errors
|
||||
error[invalid-syntax]: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
||||
--> with.py:333:10
|
||||
|
|
||||
332 | if True:
|
||||
333 | with (
|
||||
| ^
|
||||
334 | anyio.CancelScope(shield=True)
|
||||
335 | if get_running_loop()
|
||||
|
|
||||
warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors.
|
||||
|
||||
error[invalid-syntax]: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
||||
--> with.py:359:6
|
||||
|
|
||||
357 | pass
|
||||
358 |
|
||||
359 | with (
|
||||
| ^
|
||||
360 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
|
||||
361 | ):
|
||||
|
|
||||
warning: Only accept new syntax errors if they are also present in the input. The formatter should not introduce syntax errors.
|
||||
|
||||
|
||||
### Output 2
|
||||
```
|
||||
indent-style = space
|
||||
|
|
|
|||
|
|
@ -3,8 +3,6 @@
|
|||
# is parsed as a tuple, but this will always cause a runtime error, so we flag it
|
||||
# anyway
|
||||
with (foo, bar): ...
|
||||
with (
|
||||
open('foo.txt')) as foo: ...
|
||||
with (
|
||||
foo,
|
||||
bar,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
# parse_options: {"target-version": "3.8"}
|
||||
with (
|
||||
open('foo.txt')) as foo: ...
|
||||
with (
|
||||
open('foo.txt')): ...
|
||||
|
|
@ -2130,7 +2130,7 @@ impl<'src> Parser<'src> {
|
|||
let open_paren_range = self.current_token_range();
|
||||
|
||||
if self.at(TokenKind::Lpar) {
|
||||
if let Some(items) = self.try_parse_parenthesized_with_items() {
|
||||
if let (Some(items), has_trailing_comma) = self.try_parse_parenthesized_with_items() {
|
||||
// test_ok tuple_context_manager_py38
|
||||
// # parse_options: {"target-version": "3.8"}
|
||||
// with (
|
||||
|
|
@ -2139,6 +2139,13 @@ impl<'src> Parser<'src> {
|
|||
// baz,
|
||||
// ) as tup: ...
|
||||
|
||||
// test_ok single_parenthesized_item_context_manager_py38
|
||||
// # parse_options: {"target-version": "3.8"}
|
||||
// with (
|
||||
// open('foo.txt')) as foo: ...
|
||||
// with (
|
||||
// open('foo.txt')): ...
|
||||
|
||||
// test_err tuple_context_manager_py38
|
||||
// # parse_options: {"target-version": "3.8"}
|
||||
// # these cases are _syntactically_ valid before Python 3.9 because the `with` item
|
||||
|
|
@ -2146,8 +2153,6 @@ impl<'src> Parser<'src> {
|
|||
// # anyway
|
||||
// with (foo, bar): ...
|
||||
// with (
|
||||
// open('foo.txt')) as foo: ...
|
||||
// with (
|
||||
// foo,
|
||||
// bar,
|
||||
// baz,
|
||||
|
|
@ -2165,10 +2170,12 @@ impl<'src> Parser<'src> {
|
|||
// with (foo as x, bar as y): ...
|
||||
// with (foo, bar as y): ...
|
||||
// with (foo as x, bar): ...
|
||||
self.add_unsupported_syntax_error(
|
||||
UnsupportedSyntaxErrorKind::ParenthesizedContextManager,
|
||||
open_paren_range,
|
||||
);
|
||||
if items.len() > 1 || has_trailing_comma {
|
||||
self.add_unsupported_syntax_error(
|
||||
UnsupportedSyntaxErrorKind::ParenthesizedContextManager,
|
||||
open_paren_range,
|
||||
);
|
||||
}
|
||||
|
||||
self.expect(TokenKind::Rpar);
|
||||
items
|
||||
|
|
@ -2228,7 +2235,7 @@ impl<'src> Parser<'src> {
|
|||
/// If the parser isn't positioned at a `(` token.
|
||||
///
|
||||
/// See: <https://docs.python.org/3/reference/compound_stmts.html#grammar-token-python-grammar-with_stmt_contents>
|
||||
fn try_parse_parenthesized_with_items(&mut self) -> Option<Vec<WithItem>> {
|
||||
fn try_parse_parenthesized_with_items(&mut self) -> (Option<Vec<WithItem>>, bool) {
|
||||
let checkpoint = self.checkpoint();
|
||||
|
||||
// We'll start with the assumption that the with items are parenthesized.
|
||||
|
|
@ -2245,11 +2252,12 @@ impl<'src> Parser<'src> {
|
|||
// with (item1, item2 item3, item4): ...
|
||||
// with (item1, item2 as f1 item3, item4): ...
|
||||
// with (item1, item2: ...
|
||||
self.parse_comma_separated_list(RecoveryContextKind::WithItems(with_item_kind), |p| {
|
||||
let parsed_with_item = p.parse_with_item(WithItemParsingState::Speculative);
|
||||
has_optional_vars |= parsed_with_item.item.optional_vars.is_some();
|
||||
parsed_with_items.push(parsed_with_item);
|
||||
});
|
||||
let has_trailing_comma =
|
||||
self.parse_comma_separated_list(RecoveryContextKind::WithItems(with_item_kind), |p| {
|
||||
let parsed_with_item = p.parse_with_item(WithItemParsingState::Speculative);
|
||||
has_optional_vars |= parsed_with_item.item.optional_vars.is_some();
|
||||
parsed_with_items.push(parsed_with_item);
|
||||
});
|
||||
|
||||
// Check if our assumption is incorrect and it's actually a parenthesized expression.
|
||||
if has_optional_vars {
|
||||
|
|
@ -2319,7 +2327,7 @@ impl<'src> Parser<'src> {
|
|||
with_item_kind = WithItemKind::ParenthesizedExpression;
|
||||
}
|
||||
|
||||
if with_item_kind.is_parenthesized() {
|
||||
let with_items = if with_item_kind.is_parenthesized() {
|
||||
Some(
|
||||
parsed_with_items
|
||||
.into_iter()
|
||||
|
|
@ -2330,7 +2338,9 @@ impl<'src> Parser<'src> {
|
|||
self.rewind(checkpoint);
|
||||
|
||||
None
|
||||
}
|
||||
};
|
||||
|
||||
(with_items, has_trailing_comma)
|
||||
}
|
||||
|
||||
/// Parses a single `with` item.
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ input_file: crates/ruff_python_parser/resources/inline/err/tuple_context_manager
|
|||
Module(
|
||||
ModModule {
|
||||
node_index: NodeIndex(None),
|
||||
range: 0..327,
|
||||
range: 0..289,
|
||||
body: [
|
||||
With(
|
||||
StmtWith {
|
||||
|
|
@ -62,94 +62,16 @@ Module(
|
|||
With(
|
||||
StmtWith {
|
||||
node_index: NodeIndex(None),
|
||||
range: 237..274,
|
||||
range: 237..271,
|
||||
is_async: false,
|
||||
items: [
|
||||
WithItem {
|
||||
range: 242..269,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Call(
|
||||
ExprCall {
|
||||
node_index: NodeIndex(None),
|
||||
range: 246..261,
|
||||
func: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 246..250,
|
||||
id: Name("open"),
|
||||
ctx: Load,
|
||||
},
|
||||
),
|
||||
arguments: Arguments {
|
||||
range: 250..261,
|
||||
node_index: NodeIndex(None),
|
||||
args: [
|
||||
StringLiteral(
|
||||
ExprStringLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 251..260,
|
||||
value: StringLiteralValue {
|
||||
inner: Single(
|
||||
StringLiteral {
|
||||
range: 251..260,
|
||||
node_index: NodeIndex(None),
|
||||
value: "foo.txt",
|
||||
flags: StringLiteralFlags {
|
||||
quote_style: Single,
|
||||
prefix: Empty,
|
||||
triple_quoted: false,
|
||||
},
|
||||
},
|
||||
),
|
||||
},
|
||||
},
|
||||
),
|
||||
],
|
||||
keywords: [],
|
||||
},
|
||||
},
|
||||
),
|
||||
optional_vars: Some(
|
||||
Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 266..269,
|
||||
id: Name("foo"),
|
||||
ctx: Store,
|
||||
},
|
||||
),
|
||||
),
|
||||
},
|
||||
],
|
||||
body: [
|
||||
Expr(
|
||||
StmtExpr {
|
||||
node_index: NodeIndex(None),
|
||||
range: 271..274,
|
||||
value: EllipsisLiteral(
|
||||
ExprEllipsisLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 271..274,
|
||||
},
|
||||
),
|
||||
},
|
||||
),
|
||||
],
|
||||
},
|
||||
),
|
||||
With(
|
||||
StmtWith {
|
||||
node_index: NodeIndex(None),
|
||||
range: 275..309,
|
||||
is_async: false,
|
||||
items: [
|
||||
WithItem {
|
||||
range: 284..287,
|
||||
range: 246..249,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 284..287,
|
||||
range: 246..249,
|
||||
id: Name("foo"),
|
||||
ctx: Load,
|
||||
},
|
||||
|
|
@ -157,12 +79,12 @@ Module(
|
|||
optional_vars: None,
|
||||
},
|
||||
WithItem {
|
||||
range: 291..294,
|
||||
range: 253..256,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 291..294,
|
||||
range: 253..256,
|
||||
id: Name("bar"),
|
||||
ctx: Load,
|
||||
},
|
||||
|
|
@ -170,12 +92,12 @@ Module(
|
|||
optional_vars: None,
|
||||
},
|
||||
WithItem {
|
||||
range: 298..301,
|
||||
range: 260..263,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 298..301,
|
||||
range: 260..263,
|
||||
id: Name("baz"),
|
||||
ctx: Load,
|
||||
},
|
||||
|
|
@ -187,11 +109,11 @@ Module(
|
|||
Expr(
|
||||
StmtExpr {
|
||||
node_index: NodeIndex(None),
|
||||
range: 306..309,
|
||||
range: 268..271,
|
||||
value: EllipsisLiteral(
|
||||
ExprEllipsisLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 306..309,
|
||||
range: 268..271,
|
||||
},
|
||||
),
|
||||
},
|
||||
|
|
@ -202,16 +124,16 @@ Module(
|
|||
With(
|
||||
StmtWith {
|
||||
node_index: NodeIndex(None),
|
||||
range: 310..326,
|
||||
range: 272..288,
|
||||
is_async: false,
|
||||
items: [
|
||||
WithItem {
|
||||
range: 316..319,
|
||||
range: 278..281,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 316..319,
|
||||
range: 278..281,
|
||||
id: Name("foo"),
|
||||
ctx: Load,
|
||||
},
|
||||
|
|
@ -223,11 +145,11 @@ Module(
|
|||
Expr(
|
||||
StmtExpr {
|
||||
node_index: NodeIndex(None),
|
||||
range: 323..326,
|
||||
range: 285..288,
|
||||
value: EllipsisLiteral(
|
||||
ExprEllipsisLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 323..326,
|
||||
range: 285..288,
|
||||
},
|
||||
),
|
||||
},
|
||||
|
|
@ -247,23 +169,23 @@ Module(
|
|||
5 | with (foo, bar): ...
|
||||
| ^ Syntax Error: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
||||
6 | with (
|
||||
7 | open('foo.txt')) as foo: ...
|
||||
7 | foo,
|
||||
|
|
||||
|
||||
|
||||
|
|
||||
4 | # anyway
|
||||
5 | with (foo, bar): ...
|
||||
6 | with (
|
||||
| ^ Syntax Error: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
||||
7 | foo,
|
||||
8 | bar,
|
||||
|
|
||||
|
||||
|
||||
|
|
||||
6 | with (
|
||||
7 | open('foo.txt')) as foo: ...
|
||||
8 | with (
|
||||
| ^ Syntax Error: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
||||
9 | foo,
|
||||
10 | bar,
|
||||
|
|
||||
|
||||
|
||||
|
|
||||
11 | baz,
|
||||
12 | ): ...
|
||||
13 | with (foo,): ...
|
||||
9 | baz,
|
||||
10 | ): ...
|
||||
11 | with (foo,): ...
|
||||
| ^ Syntax Error: Cannot use parentheses within a `with` statement on Python 3.8 (syntax was added in Python 3.9)
|
||||
|
|
||||
|
|
|
|||
|
|
@ -0,0 +1,163 @@
|
|||
---
|
||||
source: crates/ruff_python_parser/tests/fixtures.rs
|
||||
input_file: crates/ruff_python_parser/resources/inline/ok/single_parenthesized_item_context_manager_py38.py
|
||||
---
|
||||
## AST
|
||||
|
||||
```
|
||||
Module(
|
||||
ModModule {
|
||||
node_index: NodeIndex(None),
|
||||
range: 0..112,
|
||||
body: [
|
||||
With(
|
||||
StmtWith {
|
||||
node_index: NodeIndex(None),
|
||||
range: 43..80,
|
||||
is_async: false,
|
||||
items: [
|
||||
WithItem {
|
||||
range: 48..75,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Call(
|
||||
ExprCall {
|
||||
node_index: NodeIndex(None),
|
||||
range: 52..67,
|
||||
func: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 52..56,
|
||||
id: Name("open"),
|
||||
ctx: Load,
|
||||
},
|
||||
),
|
||||
arguments: Arguments {
|
||||
range: 56..67,
|
||||
node_index: NodeIndex(None),
|
||||
args: [
|
||||
StringLiteral(
|
||||
ExprStringLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 57..66,
|
||||
value: StringLiteralValue {
|
||||
inner: Single(
|
||||
StringLiteral {
|
||||
range: 57..66,
|
||||
node_index: NodeIndex(None),
|
||||
value: "foo.txt",
|
||||
flags: StringLiteralFlags {
|
||||
quote_style: Single,
|
||||
prefix: Empty,
|
||||
triple_quoted: false,
|
||||
},
|
||||
},
|
||||
),
|
||||
},
|
||||
},
|
||||
),
|
||||
],
|
||||
keywords: [],
|
||||
},
|
||||
},
|
||||
),
|
||||
optional_vars: Some(
|
||||
Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 72..75,
|
||||
id: Name("foo"),
|
||||
ctx: Store,
|
||||
},
|
||||
),
|
||||
),
|
||||
},
|
||||
],
|
||||
body: [
|
||||
Expr(
|
||||
StmtExpr {
|
||||
node_index: NodeIndex(None),
|
||||
range: 77..80,
|
||||
value: EllipsisLiteral(
|
||||
ExprEllipsisLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 77..80,
|
||||
},
|
||||
),
|
||||
},
|
||||
),
|
||||
],
|
||||
},
|
||||
),
|
||||
With(
|
||||
StmtWith {
|
||||
node_index: NodeIndex(None),
|
||||
range: 81..111,
|
||||
is_async: false,
|
||||
items: [
|
||||
WithItem {
|
||||
range: 90..105,
|
||||
node_index: NodeIndex(None),
|
||||
context_expr: Call(
|
||||
ExprCall {
|
||||
node_index: NodeIndex(None),
|
||||
range: 90..105,
|
||||
func: Name(
|
||||
ExprName {
|
||||
node_index: NodeIndex(None),
|
||||
range: 90..94,
|
||||
id: Name("open"),
|
||||
ctx: Load,
|
||||
},
|
||||
),
|
||||
arguments: Arguments {
|
||||
range: 94..105,
|
||||
node_index: NodeIndex(None),
|
||||
args: [
|
||||
StringLiteral(
|
||||
ExprStringLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 95..104,
|
||||
value: StringLiteralValue {
|
||||
inner: Single(
|
||||
StringLiteral {
|
||||
range: 95..104,
|
||||
node_index: NodeIndex(None),
|
||||
value: "foo.txt",
|
||||
flags: StringLiteralFlags {
|
||||
quote_style: Single,
|
||||
prefix: Empty,
|
||||
triple_quoted: false,
|
||||
},
|
||||
},
|
||||
),
|
||||
},
|
||||
},
|
||||
),
|
||||
],
|
||||
keywords: [],
|
||||
},
|
||||
},
|
||||
),
|
||||
optional_vars: None,
|
||||
},
|
||||
],
|
||||
body: [
|
||||
Expr(
|
||||
StmtExpr {
|
||||
node_index: NodeIndex(None),
|
||||
range: 108..111,
|
||||
value: EllipsisLiteral(
|
||||
ExprEllipsisLiteral {
|
||||
node_index: NodeIndex(None),
|
||||
range: 108..111,
|
||||
},
|
||||
),
|
||||
},
|
||||
),
|
||||
],
|
||||
},
|
||||
),
|
||||
],
|
||||
},
|
||||
)
|
||||
```
|
||||
Loading…
Add table
Add a link
Reference in a new issue