[syntax-errors] Duplicate type parameter names (#16858)
Some checks are pending
CI / cargo fmt (push) Waiting to run
CI / Determine changes (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (linux, release) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Waiting to run
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz build (push) Blocked by required conditions
CI / fuzz parser (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / check playground (push) Blocked by required conditions
CI / benchmarks (push) Blocked by required conditions
[Knot Playground] Release / publish (push) Waiting to run

Summary
--

Detects duplicate type parameter names in function definitions, class
definitions, and type alias statements.

I also boxed the `type_params` field on `StmtTypeAlias` to make it
easier to
`match` with functions and classes. (That's the reason for the red-knot
code
owner review requests, sorry!)

Test Plan
--

New `ruff_python_syntax_errors` unit tests.

Fixes #11119.
This commit is contained in:
Brent Westbrook 2025-03-21 15:06:22 -04:00 committed by GitHub
parent 2baaedda6c
commit e4f5fe8cf7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 1043 additions and 9 deletions

View file

@ -957,7 +957,7 @@ where
self.with_type_params(
NodeWithScopeRef::TypeAliasTypeParameters(type_alias),
type_alias.type_params.as_ref(),
type_alias.type_params.as_deref(),
|builder| {
builder.push_scope(NodeWithScopeRef::TypeAlias(type_alias));
builder.visit_expr(&type_alias.value);

View file

@ -548,11 +548,13 @@ impl SemanticSyntaxContext for Checker<'_> {
}
}
SemanticSyntaxErrorKind::ReboundComprehensionVariable
| SemanticSyntaxErrorKind::DuplicateTypeParameter
if self.settings.preview.is_enabled() =>
{
self.semantic_errors.borrow_mut().push(error);
}
SemanticSyntaxErrorKind::ReboundComprehensionVariable => {}
SemanticSyntaxErrorKind::ReboundComprehensionVariable
| SemanticSyntaxErrorKind::DuplicateTypeParameter => {}
}
}
}

View file

@ -104,7 +104,7 @@ fields = [{ name = "targets", type = "Expr*" }]
doc = "See also [TypeAlias](https://docs.python.org/3/library/ast.html#ast.TypeAlias)"
fields = [
{ name = "name", type = "Expr" },
{ name = "type_params", type = "TypeParams?" },
{ name = "type_params", type = "Box<crate::TypeParams>?" },
{ name = "value", type = "Expr" },
]

View file

@ -6494,7 +6494,7 @@ pub struct StmtDelete {
pub struct StmtTypeAlias {
pub range: ruff_text_size::TextRange,
pub name: Box<Expr>,
pub type_params: Option<crate::TypeParams>,
pub type_params: Option<Box<crate::TypeParams>>,
pub value: Box<Expr>,
}

View file

@ -1622,10 +1622,10 @@ mod tests {
});
let type_alias = Stmt::TypeAlias(StmtTypeAlias {
name: Box::new(name.clone()),
type_params: Some(TypeParams {
type_params: Some(Box::new(TypeParams {
type_params: vec![type_var_one, type_var_two],
range: TextRange::default(),
}),
})),
value: Box::new(constant_three.clone()),
range: TextRange::default(),
});

View file

@ -0,0 +1,7 @@
type Alias[T, T] = ...
def f[T, T](t: T): ...
class C[T, T]: ...
type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
def f[T, T, T](): ... # two errors
def f[T, *T](): ... # star is still duplicate
def f[T, **T](): ... # as is double star

View file

@ -0,0 +1,5 @@
type Alias[T] = list[T]
def f[T](t: T): ...
class C[T]: ...
class C[T, U, V]: ...
type Alias[T, U: str, V: (str, bytes), *Ts, **P, D = default] = ...

View file

@ -974,7 +974,7 @@ impl<'src> Parser<'src> {
ast::StmtTypeAlias {
name: Box::new(name),
type_params,
type_params: type_params.map(Box::new),
value: Box::new(value.expr),
range: self.node_range(start),
}

View file

@ -11,7 +11,7 @@ use ruff_python_ast::{
visitor::{walk_expr, Visitor},
Expr, PythonVersion, Stmt, StmtExpr, StmtImportFrom,
};
use ruff_text_size::TextRange;
use ruff_text_size::{Ranged, TextRange};
#[derive(Debug)]
pub struct SemanticSyntaxChecker {
@ -53,12 +53,60 @@ impl SemanticSyntaxChecker {
});
}
fn check_stmt<Ctx: SemanticSyntaxContext>(&self, stmt: &ast::Stmt, ctx: &Ctx) {
fn check_stmt<Ctx: SemanticSyntaxContext>(&mut self, stmt: &ast::Stmt, ctx: &Ctx) {
if let Stmt::ImportFrom(StmtImportFrom { range, module, .. }) = stmt {
if self.seen_futures_boundary && matches!(module.as_deref(), Some("__future__")) {
Self::add_error(ctx, SemanticSyntaxErrorKind::LateFutureImport, *range);
}
}
Self::duplicate_type_parameter_name(stmt, ctx);
}
fn duplicate_type_parameter_name<Ctx: SemanticSyntaxContext>(stmt: &ast::Stmt, ctx: &Ctx) {
let (Stmt::FunctionDef(ast::StmtFunctionDef { type_params, .. })
| Stmt::ClassDef(ast::StmtClassDef { type_params, .. })
| Stmt::TypeAlias(ast::StmtTypeAlias { type_params, .. })) = stmt
else {
return;
};
let Some(type_params) = type_params else {
return;
};
if type_params.len() < 2 {
return;
}
for (i, type_param) in type_params.iter().enumerate() {
if type_params
.iter()
.take(i)
.any(|t| t.name().id == type_param.name().id)
{
// test_ok non_duplicate_type_parameter_names
// type Alias[T] = list[T]
// def f[T](t: T): ...
// class C[T]: ...
// class C[T, U, V]: ...
// type Alias[T, U: str, V: (str, bytes), *Ts, **P, D = default] = ...
// test_err duplicate_type_parameter_names
// type Alias[T, T] = ...
// def f[T, T](t: T): ...
// class C[T, T]: ...
// type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
// def f[T, T, T](): ... # two errors
// def f[T, *T](): ... # star is still duplicate
// def f[T, **T](): ... # as is double star
Self::add_error(
ctx,
SemanticSyntaxErrorKind::DuplicateTypeParameter,
type_param.range(),
);
}
}
}
pub fn visit_stmt<Ctx: SemanticSyntaxContext>(&mut self, stmt: &ast::Stmt, ctx: &Ctx) {
@ -168,6 +216,9 @@ impl Display for SemanticSyntaxError {
SemanticSyntaxErrorKind::ReboundComprehensionVariable => {
f.write_str("assignment expression cannot rebind comprehension variable")
}
SemanticSyntaxErrorKind::DuplicateTypeParameter => {
f.write_str("duplicate type parameter")
}
}
}
}
@ -202,6 +253,18 @@ pub enum SemanticSyntaxErrorKind {
/// ((a := 0) for a in range(0))
/// ```
ReboundComprehensionVariable,
/// Represents a duplicate type parameter name in a function definition, class definition, or
/// type alias statement.
///
/// ## Examples
///
/// ```python
/// type Alias[T, T] = ...
/// def f[T, T](t: T): ...
/// class C[T, T]: ...
/// ```
DuplicateTypeParameter,
}
/// Searches for the first named expression (`x := y`) rebinding one of the `iteration_variables` in

View file

@ -0,0 +1,589 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/err/duplicate_type_parameter_names.py
---
## AST
```
Module(
ModModule {
range: 0..261,
body: [
TypeAlias(
StmtTypeAlias {
range: 0..22,
name: Name(
ExprName {
range: 5..10,
id: Name("Alias"),
ctx: Store,
},
),
type_params: Some(
TypeParams {
range: 10..16,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 11..12,
name: Identifier {
id: Name("T"),
range: 11..12,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 14..15,
name: Identifier {
id: Name("T"),
range: 14..15,
},
bound: None,
default: None,
},
),
],
},
),
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 19..22,
},
),
},
),
FunctionDef(
StmtFunctionDef {
range: 23..45,
is_async: false,
decorator_list: [],
name: Identifier {
id: Name("f"),
range: 27..28,
},
type_params: Some(
TypeParams {
range: 28..34,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 29..30,
name: Identifier {
id: Name("T"),
range: 29..30,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 32..33,
name: Identifier {
id: Name("T"),
range: 32..33,
},
bound: None,
default: None,
},
),
],
},
),
parameters: Parameters {
range: 34..40,
posonlyargs: [],
args: [
ParameterWithDefault {
range: 35..39,
parameter: Parameter {
range: 35..39,
name: Identifier {
id: Name("t"),
range: 35..36,
},
annotation: Some(
Name(
ExprName {
range: 38..39,
id: Name("T"),
ctx: Load,
},
),
),
},
default: None,
},
],
vararg: None,
kwonlyargs: [],
kwarg: None,
},
returns: None,
body: [
Expr(
StmtExpr {
range: 42..45,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 42..45,
},
),
},
),
],
},
),
ClassDef(
StmtClassDef {
range: 46..64,
decorator_list: [],
name: Identifier {
id: Name("C"),
range: 52..53,
},
type_params: Some(
TypeParams {
range: 53..59,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 54..55,
name: Identifier {
id: Name("T"),
range: 54..55,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 57..58,
name: Identifier {
id: Name("T"),
range: 57..58,
},
bound: None,
default: None,
},
),
],
},
),
arguments: None,
body: [
Expr(
StmtExpr {
range: 61..64,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 61..64,
},
),
},
),
],
},
),
TypeAlias(
StmtTypeAlias {
range: 65..132,
name: Name(
ExprName {
range: 70..75,
id: Name("Alias"),
ctx: Store,
},
),
type_params: Some(
TypeParams {
range: 75..126,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 76..77,
name: Identifier {
id: Name("T"),
range: 76..77,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 79..85,
name: Identifier {
id: Name("U"),
range: 79..80,
},
bound: Some(
Name(
ExprName {
range: 82..85,
id: Name("str"),
ctx: Load,
},
),
),
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 87..102,
name: Identifier {
id: Name("V"),
range: 87..88,
},
bound: Some(
Tuple(
ExprTuple {
range: 90..102,
elts: [
Name(
ExprName {
range: 91..94,
id: Name("str"),
ctx: Load,
},
),
Name(
ExprName {
range: 96..101,
id: Name("bytes"),
ctx: Load,
},
),
],
ctx: Load,
parenthesized: true,
},
),
),
default: None,
},
),
TypeVarTuple(
TypeParamTypeVarTuple {
range: 104..107,
name: Identifier {
id: Name("Ts"),
range: 105..107,
},
default: None,
},
),
ParamSpec(
TypeParamParamSpec {
range: 109..112,
name: Identifier {
id: Name("P"),
range: 111..112,
},
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 114..125,
name: Identifier {
id: Name("T"),
range: 114..115,
},
bound: None,
default: Some(
Name(
ExprName {
range: 118..125,
id: Name("default"),
ctx: Load,
},
),
),
},
),
],
},
),
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 129..132,
},
),
},
),
FunctionDef(
StmtFunctionDef {
range: 133..154,
is_async: false,
decorator_list: [],
name: Identifier {
id: Name("f"),
range: 137..138,
},
type_params: Some(
TypeParams {
range: 138..147,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 139..140,
name: Identifier {
id: Name("T"),
range: 139..140,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 142..143,
name: Identifier {
id: Name("T"),
range: 142..143,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 145..146,
name: Identifier {
id: Name("T"),
range: 145..146,
},
bound: None,
default: None,
},
),
],
},
),
parameters: Parameters {
range: 147..149,
posonlyargs: [],
args: [],
vararg: None,
kwonlyargs: [],
kwarg: None,
},
returns: None,
body: [
Expr(
StmtExpr {
range: 151..154,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 151..154,
},
),
},
),
],
},
),
FunctionDef(
StmtFunctionDef {
range: 169..188,
is_async: false,
decorator_list: [],
name: Identifier {
id: Name("f"),
range: 173..174,
},
type_params: Some(
TypeParams {
range: 174..181,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 175..176,
name: Identifier {
id: Name("T"),
range: 175..176,
},
bound: None,
default: None,
},
),
TypeVarTuple(
TypeParamTypeVarTuple {
range: 178..180,
name: Identifier {
id: Name("T"),
range: 179..180,
},
default: None,
},
),
],
},
),
parameters: Parameters {
range: 181..183,
posonlyargs: [],
args: [],
vararg: None,
kwonlyargs: [],
kwarg: None,
},
returns: None,
body: [
Expr(
StmtExpr {
range: 185..188,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 185..188,
},
),
},
),
],
},
),
FunctionDef(
StmtFunctionDef {
range: 218..238,
is_async: false,
decorator_list: [],
name: Identifier {
id: Name("f"),
range: 222..223,
},
type_params: Some(
TypeParams {
range: 223..231,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 224..225,
name: Identifier {
id: Name("T"),
range: 224..225,
},
bound: None,
default: None,
},
),
ParamSpec(
TypeParamParamSpec {
range: 227..230,
name: Identifier {
id: Name("T"),
range: 229..230,
},
default: None,
},
),
],
},
),
parameters: Parameters {
range: 231..233,
posonlyargs: [],
args: [],
vararg: None,
kwonlyargs: [],
kwarg: None,
},
returns: None,
body: [
Expr(
StmtExpr {
range: 235..238,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 235..238,
},
),
},
),
],
},
),
],
},
)
```
## Semantic Syntax Errors
|
1 | type Alias[T, T] = ...
| ^ Syntax Error: duplicate type parameter
2 | def f[T, T](t: T): ...
3 | class C[T, T]: ...
|
|
1 | type Alias[T, T] = ...
2 | def f[T, T](t: T): ...
| ^ Syntax Error: duplicate type parameter
3 | class C[T, T]: ...
4 | type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
|
|
1 | type Alias[T, T] = ...
2 | def f[T, T](t: T): ...
3 | class C[T, T]: ...
| ^ Syntax Error: duplicate type parameter
4 | type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
5 | def f[T, T, T](): ... # two errors
|
|
2 | def f[T, T](t: T): ...
3 | class C[T, T]: ...
4 | type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
| ^^^^^^^^^^^ Syntax Error: duplicate type parameter
5 | def f[T, T, T](): ... # two errors
6 | def f[T, *T](): ... # star is still duplicate
|
|
3 | class C[T, T]: ...
4 | type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
5 | def f[T, T, T](): ... # two errors
| ^ Syntax Error: duplicate type parameter
6 | def f[T, *T](): ... # star is still duplicate
7 | def f[T, **T](): ... # as is double star
|
|
3 | class C[T, T]: ...
4 | type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
5 | def f[T, T, T](): ... # two errors
| ^ Syntax Error: duplicate type parameter
6 | def f[T, *T](): ... # star is still duplicate
7 | def f[T, **T](): ... # as is double star
|
|
4 | type Alias[T, U: str, V: (str, bytes), *Ts, **P, T = default] = ...
5 | def f[T, T, T](): ... # two errors
6 | def f[T, *T](): ... # star is still duplicate
| ^^ Syntax Error: duplicate type parameter
7 | def f[T, **T](): ... # as is double star
|
|
5 | def f[T, T, T](): ... # two errors
6 | def f[T, *T](): ... # star is still duplicate
7 | def f[T, **T](): ... # as is double star
| ^^^ Syntax Error: duplicate type parameter
|

View file

@ -0,0 +1,368 @@
---
source: crates/ruff_python_parser/tests/fixtures.rs
input_file: crates/ruff_python_parser/resources/inline/ok/non_duplicate_type_parameter_names.py
---
## AST
```
Module(
ModModule {
range: 0..150,
body: [
TypeAlias(
StmtTypeAlias {
range: 0..23,
name: Name(
ExprName {
range: 5..10,
id: Name("Alias"),
ctx: Store,
},
),
type_params: Some(
TypeParams {
range: 10..13,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 11..12,
name: Identifier {
id: Name("T"),
range: 11..12,
},
bound: None,
default: None,
},
),
],
},
),
value: Subscript(
ExprSubscript {
range: 16..23,
value: Name(
ExprName {
range: 16..20,
id: Name("list"),
ctx: Load,
},
),
slice: Name(
ExprName {
range: 21..22,
id: Name("T"),
ctx: Load,
},
),
ctx: Load,
},
),
},
),
FunctionDef(
StmtFunctionDef {
range: 24..43,
is_async: false,
decorator_list: [],
name: Identifier {
id: Name("f"),
range: 28..29,
},
type_params: Some(
TypeParams {
range: 29..32,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 30..31,
name: Identifier {
id: Name("T"),
range: 30..31,
},
bound: None,
default: None,
},
),
],
},
),
parameters: Parameters {
range: 32..38,
posonlyargs: [],
args: [
ParameterWithDefault {
range: 33..37,
parameter: Parameter {
range: 33..37,
name: Identifier {
id: Name("t"),
range: 33..34,
},
annotation: Some(
Name(
ExprName {
range: 36..37,
id: Name("T"),
ctx: Load,
},
),
),
},
default: None,
},
],
vararg: None,
kwonlyargs: [],
kwarg: None,
},
returns: None,
body: [
Expr(
StmtExpr {
range: 40..43,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 40..43,
},
),
},
),
],
},
),
ClassDef(
StmtClassDef {
range: 44..59,
decorator_list: [],
name: Identifier {
id: Name("C"),
range: 50..51,
},
type_params: Some(
TypeParams {
range: 51..54,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 52..53,
name: Identifier {
id: Name("T"),
range: 52..53,
},
bound: None,
default: None,
},
),
],
},
),
arguments: None,
body: [
Expr(
StmtExpr {
range: 56..59,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 56..59,
},
),
},
),
],
},
),
ClassDef(
StmtClassDef {
range: 60..81,
decorator_list: [],
name: Identifier {
id: Name("C"),
range: 66..67,
},
type_params: Some(
TypeParams {
range: 67..76,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 68..69,
name: Identifier {
id: Name("T"),
range: 68..69,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 71..72,
name: Identifier {
id: Name("U"),
range: 71..72,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 74..75,
name: Identifier {
id: Name("V"),
range: 74..75,
},
bound: None,
default: None,
},
),
],
},
),
arguments: None,
body: [
Expr(
StmtExpr {
range: 78..81,
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 78..81,
},
),
},
),
],
},
),
TypeAlias(
StmtTypeAlias {
range: 82..149,
name: Name(
ExprName {
range: 87..92,
id: Name("Alias"),
ctx: Store,
},
),
type_params: Some(
TypeParams {
range: 92..143,
type_params: [
TypeVar(
TypeParamTypeVar {
range: 93..94,
name: Identifier {
id: Name("T"),
range: 93..94,
},
bound: None,
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 96..102,
name: Identifier {
id: Name("U"),
range: 96..97,
},
bound: Some(
Name(
ExprName {
range: 99..102,
id: Name("str"),
ctx: Load,
},
),
),
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 104..119,
name: Identifier {
id: Name("V"),
range: 104..105,
},
bound: Some(
Tuple(
ExprTuple {
range: 107..119,
elts: [
Name(
ExprName {
range: 108..111,
id: Name("str"),
ctx: Load,
},
),
Name(
ExprName {
range: 113..118,
id: Name("bytes"),
ctx: Load,
},
),
],
ctx: Load,
parenthesized: true,
},
),
),
default: None,
},
),
TypeVarTuple(
TypeParamTypeVarTuple {
range: 121..124,
name: Identifier {
id: Name("Ts"),
range: 122..124,
},
default: None,
},
),
ParamSpec(
TypeParamParamSpec {
range: 126..129,
name: Identifier {
id: Name("P"),
range: 128..129,
},
default: None,
},
),
TypeVar(
TypeParamTypeVar {
range: 131..142,
name: Identifier {
id: Name("D"),
range: 131..132,
},
bound: None,
default: Some(
Name(
ExprName {
range: 135..142,
id: Name("default"),
ctx: Load,
},
),
),
},
),
],
},
),
value: EllipsisLiteral(
ExprEllipsisLiteral {
range: 146..149,
},
),
},
),
],
},
)
```