mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-04 18:58:04 +00:00
[red-knot] fix inheritance-cycle detection for generic classes (#17620)
## Summary The `ClassLiteralType::inheritance_cycle` method is intended to detect inheritance cycles that would result in cyclic MROs, emit a diagnostic, and skip actually trying to create the cyclic MRO, falling back to an "error" MRO instead with just `Unknown` and `object`. This method didn't work properly for generic classes. It used `fully_static_explicit_bases`, which filter-maps `explicit_bases` over `Type::into_class_type`, which returns `None` for an unspecialized generic class literal. So in a case like `class C[T](C): ...`, because the explicit base is an unspecialized generic, we just skipped it, and failed to detect the class as cyclically defined. Instead, iterate directly over all `explicit_bases`, and explicitly handle both the specialized (`GenericAlias`) and unspecialized (`ClassLiteral`) cases, so that we check all bases and correctly detect cyclic inheritance. ## Test Plan Added mdtests.
This commit is contained in:
parent
6d3b1d13d6
commit
4c3f389598
2 changed files with 32 additions and 11 deletions
|
@ -275,14 +275,16 @@ c: C[int] = C[int]()
|
|||
reveal_type(c.method("string")) # revealed: Literal["string"]
|
||||
```
|
||||
|
||||
## Cyclic class definition
|
||||
## Cyclic class definitions
|
||||
|
||||
### F-bounded quantification
|
||||
|
||||
A class can use itself as the type parameter of one of its superclasses. (This is also known as the
|
||||
[curiously recurring template pattern][crtp] or [F-bounded quantification][f-bound].)
|
||||
|
||||
Here, `Sub` is not a generic class, since it fills its superclass's type parameter (with itself).
|
||||
#### In a stub file
|
||||
|
||||
`stub.pyi`:
|
||||
Here, `Sub` is not a generic class, since it fills its superclass's type parameter (with itself).
|
||||
|
||||
```pyi
|
||||
class Base[T]: ...
|
||||
|
@ -291,9 +293,9 @@ class Sub(Base[Sub]): ...
|
|||
reveal_type(Sub) # revealed: Literal[Sub]
|
||||
```
|
||||
|
||||
A similar case can work in a non-stub file, if forward references are stringified:
|
||||
#### With string forward references
|
||||
|
||||
`string_annotation.py`:
|
||||
A similar case can work in a non-stub file, if forward references are stringified:
|
||||
|
||||
```py
|
||||
class Base[T]: ...
|
||||
|
@ -302,9 +304,9 @@ class Sub(Base["Sub"]): ...
|
|||
reveal_type(Sub) # revealed: Literal[Sub]
|
||||
```
|
||||
|
||||
In a non-stub file, without stringified forward references, this raises a `NameError`:
|
||||
#### Without string forward references
|
||||
|
||||
`bare_annotation.py`:
|
||||
In a non-stub file, without stringified forward references, this raises a `NameError`:
|
||||
|
||||
```py
|
||||
class Base[T]: ...
|
||||
|
@ -313,11 +315,23 @@ class Base[T]: ...
|
|||
class Sub(Base[Sub]): ...
|
||||
```
|
||||
|
||||
## Another cyclic case
|
||||
### Cyclic inheritance as a generic parameter
|
||||
|
||||
```pyi
|
||||
class Derived[T](list[Derived[T]]): ...
|
||||
```
|
||||
|
||||
### Direct cyclic inheritance
|
||||
|
||||
Inheritance that would result in a cyclic MRO is detected as an error.
|
||||
|
||||
```py
|
||||
# error: [cyclic-class-definition]
|
||||
class C[T](C): ...
|
||||
|
||||
# error: [cyclic-class-definition]
|
||||
class D[T](D[int]): ...
|
||||
```
|
||||
|
||||
[crtp]: https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern
|
||||
[f-bound]: https://en.wikipedia.org/wiki/Bounded_quantification#F-bounded_quantification
|
||||
|
|
|
@ -174,6 +174,10 @@ impl<'db> GenericAlias<'db> {
|
|||
pub(crate) fn definition(self, db: &'db dyn Db) -> Definition<'db> {
|
||||
self.origin(db).class(db).definition(db)
|
||||
}
|
||||
|
||||
pub(crate) fn class_literal(self, db: &'db dyn Db) -> ClassLiteralType<'db> {
|
||||
ClassLiteralType::Generic(self.origin(db))
|
||||
}
|
||||
}
|
||||
|
||||
impl<'db> From<GenericAlias<'db>> for Type<'db> {
|
||||
|
@ -1690,8 +1694,12 @@ impl<'db> ClassLiteralType<'db> {
|
|||
visited_classes: &mut IndexSet<ClassLiteralType<'db>>,
|
||||
) -> bool {
|
||||
let mut result = false;
|
||||
for explicit_base_class in class.fully_static_explicit_bases(db) {
|
||||
let (explicit_base_class_literal, _) = explicit_base_class.class_literal(db);
|
||||
for explicit_base in class.explicit_bases(db) {
|
||||
let explicit_base_class_literal = match explicit_base {
|
||||
Type::ClassLiteral(class_literal) => *class_literal,
|
||||
Type::GenericAlias(generic_alias) => generic_alias.class_literal(db),
|
||||
_ => continue,
|
||||
};
|
||||
if !classes_on_stack.insert(explicit_base_class_literal) {
|
||||
return true;
|
||||
}
|
||||
|
@ -1705,7 +1713,6 @@ impl<'db> ClassLiteralType<'db> {
|
|||
visited_classes,
|
||||
);
|
||||
}
|
||||
|
||||
classes_on_stack.pop();
|
||||
}
|
||||
result
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue