language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-27 04:19:18 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

[ty] Respect the gradual guarantee when reporting errors in resolving MROs (#17962)

Browse source
This commit is contained in:
Alex Waygood 2025-05-08 22:57:39 +01:00 • committed by GitHub
parent 981bd70d39
commit 4d81a41107
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 73 additions and 46 deletions
Download patch file Download diff file Expand all files Collapse all files

37
crates/ty_python_semantic/resources/mdtest/mro.md
View file

@ -391,28 +391,35 @@ class E(
## `__bases__` lists with duplicate `Unknown` bases ## `__bases__` lists with duplicate `Unknown` bases
We do not emit errors on classes where multiple bases are inferred as `Unknown`, `Todo` or `Any`.
Usually having duplicate bases in a bases list like this would cause us to emit a diagnostic;
however, for gradual types this would break the
[gradual guarantee](https://typing.python.org/en/latest/spec/concepts.html#the-gradual-guarantee):
the dynamic base can usually be materialised to a type that would lead to a resolvable MRO.
```py ```py
# error: [unresolved-import] from unresolvable_module import UnknownBase1, UnknownBase2 # error: [unresolved-import]
from does_not_exist import unknown_object_1, unknown_object_2
reveal_type(unknown_object_1) # revealed: Unknown reveal_type(UnknownBase1) # revealed: Unknown
reveal_type(unknown_object_2) # revealed: Unknown reveal_type(UnknownBase2) # revealed: Unknown
# We *should* emit an error here to warn the user that we have no idea # no error here -- we respect the gradual guarantee:
# what the MRO of this class should really be. class Foo(UnknownBase1, UnknownBase2): ...
# However, we don't complain about "duplicate base classes" here,
# even though two classes are both inferred as being `Unknown`.
#
# (TODO: should we revisit this? Does it violate the gradual guarantee?
# Should we just silently infer `[Foo, Unknown, object]` as the MRO here
# without emitting any error at all? Not sure...)
#
# error: [inconsistent-mro] "Cannot create a consistent method resolution order (MRO) for class `Foo` with bases list `[Unknown, Unknown]`"
class Foo(unknown_object_1, unknown_object_2): ...
reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>] reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
``` ```
However, if there are duplicate class elements, we do emit an error, even if there are also multiple
dynamic members. The following class definition will definitely fail, no matter what the dynamic
bases materialize to:
```py
# error: [duplicate-base] "Duplicate base class `Foo`"
class Bar(UnknownBase1, Foo, UnknownBase2, Foo): ...
reveal_type(Bar.__mro__) # revealed: tuple[<class 'Bar'>, Unknown, <class 'object'>]
```
## Unrelated objects inferred as `Any`/`Unknown` do not have special `__mro__` attributes ## Unrelated objects inferred as `Any`/`Unknown` do not have special `__mro__` attributes
```py ```py

82
crates/ty_python_semantic/src/types/mro.rs
View file

@ -153,43 +153,63 @@ impl<'db> Mro<'db> {
.collect(), .collect(),
); );
c3_merge(seqs).ok_or_else(|| { if let Some(mro) = c3_merge(seqs) {
let duplicate_bases: Box<[DuplicateBaseError<'db>]> = { return Ok(mro);
let mut base_to_indices: FxHashMap<ClassType<'db>, Vec<usize>> = }
FxHashMap::default();
for (index, base) in valid_bases let mut duplicate_dynamic_bases = false;
.iter()
.enumerate() let duplicate_bases: Vec<DuplicateBaseError<'db>> = {
.filter_map(|(index, base)| Some((index, base.into_class()?))) let mut base_to_indices: FxHashMap<ClassBase<'db>, Vec<usize>> =
{ FxHashMap::default();
base_to_indices.entry(base).or_default().push(index);
for (index, base) in valid_bases.iter().enumerate() {
base_to_indices.entry(*base).or_default().push(index);
}
let mut errors = vec![];
for (base, indices) in base_to_indices {
let Some((first_index, later_indices)) = indices.split_first() else {
continue;
};
if later_indices.is_empty() {
continue;
} }
match base {
base_to_indices ClassBase::Class(class) => {
.iter() errors.push(DuplicateBaseError {
.filter_map(|(base, indices)| { duplicate_base: class.class_literal(db).0,
let (first_index, later_indices) = indices.split_first()?;
if later_indices.is_empty() {
return None;
}
Some(DuplicateBaseError {
duplicate_base: base.class_literal(db).0,
first_index: *first_index, first_index: *first_index,
later_indices: later_indices.iter().copied().collect(), later_indices: later_indices.iter().copied().collect(),
}) });
}) }
.collect() // TODO these should also be reported as duplicate bases
}; // rather than using the less specific `inconsistent-mro` error
ClassBase::Generic(_) | ClassBase::Protocol => continue,
if duplicate_bases.is_empty() { ClassBase::Dynamic(_) => duplicate_dynamic_bases = true,
MroErrorKind::UnresolvableMro {
bases_list: valid_bases.into_boxed_slice(),
} }
} else {
MroErrorKind::DuplicateBases(duplicate_bases)
} }
})
errors
};
if duplicate_bases.is_empty() {
if duplicate_dynamic_bases {
Ok(Mro::from_error(
db,
class.apply_optional_specialization(db, specialization),
))
} else {
Err(MroErrorKind::UnresolvableMro {
bases_list: valid_bases.into_boxed_slice(),
})
}
} else {
Err(MroErrorKind::DuplicateBases(
duplicate_bases.into_boxed_slice(),
))
}
} }
} }
} }