Skip to content

[syntax-errors] Type parameter lists before Python 3.12 #16479

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

Merged
ntBre merged 11 commits into from
Mar 5, 2025
Merged

[syntax-errors] Type parameter lists before Python 3.12 #16479

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
17ccf37
[syntax-errors] Type parameter lists before Python 3.12
ntBre Mar 3, 2025
5686e97
document variant
ntBre Mar 4, 2025
1b259f1
TypeParams -> TypeParameterList
ntBre Mar 4, 2025
6bc9876
include multiple type parameters (and all kinds) in class test
ntBre Mar 4, 2025
ae50ba8
Merge branch 'main' into brent/syn-type-params
ntBre Mar 4, 2025
ed689c7
revert default addition after merge
ntBre Mar 4, 2025
83b8d56
don't add a second error for type alias statements, reorganize tests
ntBre Mar 4, 2025
fc35635
test that empty cases only give a ParseError
ntBre Mar 4, 2025
61f2ad7
clean up unused snapshots
ntBre Mar 4, 2025
1d6a6df
remove is_empty checks and accompanying comments
ntBre Mar 5, 2025
a35bf2c
join reference link
ntBre Mar 5, 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
3 changes: 3 additions & 0 deletions crates/ruff_python_parser/resources/inline/err/class_type_params_py311.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# parse_options: {"target-version": "3.11"}
class Foo[S: (str, bytes), T: float, *Ts, **P]: ...
class Foo[]: ...
3 changes: 3 additions & 0 deletions crates/ruff_python_parser/resources/inline/err/function_type_params_py311.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# parse_options: {"target-version": "3.11"}
def foo[T](): ...
def foo[](): ...
2 changes: 2 additions & 0 deletions crates/ruff_python_parser/resources/inline/ok/class_type_params_py312.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.12"}
class Foo[S: (str, bytes), T: float, *Ts, **P]: ...
2 changes: 2 additions & 0 deletions crates/ruff_python_parser/resources/inline/ok/function_type_params_py312.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.12"}
def foo[T](): ...
31 changes: 31 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 @@ -449,6 +449,35 @@ pub enum UnsupportedSyntaxErrorKind {
Match,
Walrus,
ExceptStar,
/// Represents the use of a [type parameter list] before Python 3.12.
///
/// ## Examples
///
/// Before Python 3.12, generic parameters had to be declared separately using a class like
/// [`typing.TypeVar`], which could then be used in a function or class definition:
///
/// ```python
/// from typing import Generic, TypeVar
///
/// T = TypeVar("T")
///
/// def f(t: T): ...
/// class C(Generic[T]): ...
/// ```
///
/// [PEP 695], included in Python 3.12, introduced the new type parameter syntax, which allows
/// these to be written more compactly and without a separate type variable:
///
/// ```python
/// def f[T](t: T): ...
/// class C[T]: ...
/// ```
///
/// [type parameter list]:
/// https://docs.python.org/3/reference/compound_stmts.html#type-parameter-lists
Copy link
Member

@dhruvmanila dhruvmanila Mar 5, 2025

Choose a reason for hiding this comment

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

Does this work? Shouldn't the link be after [..]: on the same line?

Copy link
Contributor Author

@ntBre ntBre Mar 5, 2025

Choose a reason for hiding this comment

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

It appeared to work in Emacs, but I can put it on one line to be safe!

/// [PEP 695]: https://peps.python.org/pep-0695/
/// [`typing.TypeVar`]: https://docs.python.org/3/library/typing.html#typevar
TypeParameterList,
TypeAliasStatement,
TypeParamDefault,
}
Expand All @@ -459,6 +488,7 @@ impl Display for UnsupportedSyntaxError {
UnsupportedSyntaxErrorKind::Match => "Cannot use `match` statement",
UnsupportedSyntaxErrorKind::Walrus => "Cannot use named assignment expression (`:=`)",
UnsupportedSyntaxErrorKind::ExceptStar => "Cannot use `except*`",
UnsupportedSyntaxErrorKind::TypeParameterList => "Cannot use type parameter lists",
UnsupportedSyntaxErrorKind::TypeAliasStatement => "Cannot use `type` alias statement",
UnsupportedSyntaxErrorKind::TypeParamDefault => {
"Cannot set default type for a type parameter"
Expand All @@ -480,6 +510,7 @@ impl UnsupportedSyntaxErrorKind {
UnsupportedSyntaxErrorKind::Match => PythonVersion::PY310,
UnsupportedSyntaxErrorKind::Walrus => PythonVersion::PY38,
UnsupportedSyntaxErrorKind::ExceptStar => PythonVersion::PY311,
UnsupportedSyntaxErrorKind::TypeParameterList => PythonVersion::PY312,
UnsupportedSyntaxErrorKind::TypeAliasStatement => PythonVersion::PY312,
UnsupportedSyntaxErrorKind::TypeParamDefault => PythonVersion::PY313,
}
Expand Down
38 changes: 38 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 @@ -1786,6 +1786,25 @@ impl<'src> Parser<'src> {
// x = 10
let type_params = self.try_parse_type_params();

// test_ok function_type_params_py312
// # parse_options: {"target-version": "3.12"}
// def foo[T](): ...

// test_err function_type_params_py311
// # parse_options: {"target-version": "3.11"}
// def foo[T](): ...
// def foo[](): ...
if let Some(ast::TypeParams { range, type_params }) = &type_params {
// Only emit the `ParseError` for an empty parameter list instead of also including an
// `UnsupportedSyntaxError`.
if !type_params.is_empty() {
Copy link
Member

@dhruvmanila dhruvmanila Mar 5, 2025

Choose a reason for hiding this comment

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

What's the reason for this condition? As a user I'd find it confusing if Ruff told me to first add the type parameters and then tell me to remove the entire list. Sorry if I missed this in my initial review if it was already present.

Copy link
Contributor Author

@ntBre ntBre Mar 5, 2025

Choose a reason for hiding this comment

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

Oh that's a good point... I was just trying to avoid duplicates, but if anything, it might make sense to suppress the ParseError in this case. Removing the condition is certainly easiest though.

And you didn't miss it, I added it here in the revision when I added the empty test cases.

Copy link
Member

@dhruvmanila dhruvmanila Mar 5, 2025

Choose a reason for hiding this comment

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

I think I'd prefer keep it both. I don't think we should suppress any syntax errors.

I know this goes against what I said earlier in #16479 (comment) but my argument for that would be that it's a new statement altogether. Maybe for now we can just avoid suppressing any syntax errors based on other syntax errors and we can act on it based on user feedback? Happy to hear others thoughts on this.

Copy link
Member

@MichaReiser MichaReiser Mar 5, 2025

Choose a reason for hiding this comment

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

The parser does have some logic to avoid multiple errors at the same location for invalid syntax errors.

ruff/crates/ruff_python_parser/src/parser/mod.rs

Lines 424 to 438 in 37fbe58

fn inner(errors: &mut Vec<ParseError>, error: ParseErrorType, range: TextRange) {
// Avoid flagging multiple errors at the same location
let is_same_location = errors
.last()
.is_some_and(|last| last.location.start() == range.start());
if !is_same_location {
errors.push(ParseError {
error,
location: range,
});
}
}
inner(&mut self.errors, error, ranged.range());

But I agree that we should solve this holisticly because it will otherwise be very error prone to avoid all possible errors at the same location

Copy link
Contributor Author

@ntBre ntBre Mar 5, 2025

Choose a reason for hiding this comment

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

Ah nice! I'll remove these checks for now, and then maybe we can add UnsupportedSyntaxErrors to that check or a similar one later.

self.add_unsupported_syntax_error(
UnsupportedSyntaxErrorKind::TypeParameterList,
*range,
);
}
}

// test_ok function_def_parameter_range
// def foo(
// first: int,
Expand Down Expand Up @@ -1900,6 +1919,25 @@ impl<'src> Parser<'src> {
// x = 10
let type_params = self.try_parse_type_params();

// test_ok class_type_params_py312
// # parse_options: {"target-version": "3.12"}
// class Foo[S: (str, bytes), T: float, *Ts, **P]: ...

// test_err class_type_params_py311
// # parse_options: {"target-version": "3.11"}
// class Foo[S: (str, bytes), T: float, *Ts, **P]: ...
// class Foo[]: ...
if let Some(ast::TypeParams { range, type_params }) = &type_params {
// Only emit the `ParseError` for an empty parameter list instead of also including an
// `UnsupportedSyntaxError`.
if !type_params.is_empty() {
Copy link
Member

@dhruvmanila dhruvmanila Mar 5, 2025

Choose a reason for hiding this comment

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

Same as above.

self.add_unsupported_syntax_error(
UnsupportedSyntaxErrorKind::TypeParameterList,
*range,
);
}
}

// test_ok class_def_arguments
// class Foo: ...
// class Foo(): ...
Expand Down
166 changes: 166 additions & 0 deletions crates/ruff_python_parser/tests/snapshots/invalid_syntax@class_type_params_py311.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/class_type_params_py311.py
---
## AST

```
Module(
ModModule {
range: 0..113,
body: [
ClassDef(
StmtClassDef {
range: 44..95,
decorator_list: [],
name: Identifier {
id: Name("Foo"),
range: 50..53,
},
type_params: Some(
TypeParams {
range: 53..90,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 54..69,
name: Identifier {
id: Name("S"),
range: 54..55,
},
bound: Some(
Tuple(
ExprTuple {
range: 57..69,
elts: [
Name(
ExprName {
range: 58..61,
id: Name("str"),
ctx: Load,
},
),
Name(
ExprName {
range: 63..68,
id: Name("bytes"),
ctx: Load,
},
),
],
ctx: Load,
parenthesized: true,
},
),
),
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 71..79,
name: Identifier {
id: Name("T"),
range: 71..72,
},
bound: Some(
Name(
ExprName {
range: 74..79,
id: Name("float"),
ctx: Load,
},
),
),
default: None,
},
),
TypeVarTuple(
TypeParamTypeVarTuple {
range: 81..84,
name: Identifier {
id: Name("Ts"),
range: 82..84,
},
default: None,
},
),
ParamSpec(
TypeParamParamSpec {
range: 86..89,
name: Identifier {
id: Name("P"),
range: 88..89,
},
default: None,
},
),
],
},
),
arguments: None,
body: [
Expr(
StmtExpr {
range: 92..95,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 92..95,
},
),
},
),
],
},
),
ClassDef(
StmtClassDef {
range: 96..112,
decorator_list: [],
name: Identifier {
id: Name("Foo"),
range: 102..105,
},
type_params: Some(
TypeParams {
range: 105..107,
type_params: [],
},
),
arguments: None,
body: [
Expr(
StmtExpr {
range: 109..112,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 109..112,
},
),
},
),
],
},
),
],
},
)
```
## Errors

|
1 | # parse_options: {"target-version": "3.11"}
2 | class Foo[S: (str, bytes), T: float, *Ts, **P]: ...
3 | class Foo[]: ...
| ^ Syntax Error: Type parameter list cannot be empty
|


## Unsupported Syntax Errors

|
1 | # parse_options: {"target-version": "3.11"}
2 | class Foo[S: (str, bytes), T: float, *Ts, **P]: ...
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Syntax Error: Cannot use type parameter lists on Python 3.11 (syntax was added in Python 3.12)
3 | class Foo[]: ...
|
Loading
Loading