Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 24 additions & 17 deletions crates/ty_python_semantic/resources/mdtest/mro.md
Original file line number Diff line number Diff line change
Expand Up @@ -391,28 +391,35 @@ class E(

## `__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
# error: [unresolved-import]
from does_not_exist import unknown_object_1, unknown_object_2

reveal_type(unknown_object_1) # revealed: Unknown
reveal_type(unknown_object_2) # revealed: Unknown

# We *should* emit an error here to warn the user that we have no idea
# what the MRO of this class should really be.
# 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): ...
from unresolvable_module import UnknownBase1, UnknownBase2 # error: [unresolved-import]

reveal_type(UnknownBase1) # revealed: Unknown
reveal_type(UnknownBase2) # revealed: Unknown

# no error here -- we respect the gradual guarantee:
class Foo(UnknownBase1, UnknownBase2): ...

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

```py
Expand Down
82 changes: 51 additions & 31 deletions crates/ty_python_semantic/src/types/mro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,43 +153,63 @@ impl<'db> Mro<'db> {
.collect(),
);

c3_merge(seqs).ok_or_else(|| {
let duplicate_bases: Box<[DuplicateBaseError<'db>]> = {
let mut base_to_indices: FxHashMap<ClassType<'db>, Vec<usize>> =
FxHashMap::default();

for (index, base) in valid_bases
.iter()
.enumerate()
.filter_map(|(index, base)| Some((index, base.into_class()?)))
{
base_to_indices.entry(base).or_default().push(index);
}
if let Some(mro) = c3_merge(seqs) {
return Ok(mro);
}

let mut duplicate_dynamic_bases = false;

let duplicate_bases: Vec<DuplicateBaseError<'db>> = {
let mut base_to_indices: FxHashMap<ClassBase<'db>, Vec<usize>> =
FxHashMap::default();

base_to_indices
.iter()
.filter_map(|(base, indices)| {
let (first_index, later_indices) = indices.split_first()?;
if later_indices.is_empty() {
return None;
}
Some(DuplicateBaseError {
duplicate_base: base.class_literal(db).0,
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 {
ClassBase::Class(class) => {
errors.push(DuplicateBaseError {
duplicate_base: class.class_literal(db).0,
first_index: *first_index,
later_indices: later_indices.iter().copied().collect(),
})
})
.collect()
};

if duplicate_bases.is_empty() {
MroErrorKind::UnresolvableMro {
bases_list: valid_bases.into_boxed_slice(),
});
}
// TODO these should also be reported as duplicate bases
// rather than using the less specific `inconsistent-mro` error
ClassBase::Generic(_) | ClassBase::Protocol => continue,
ClassBase::Dynamic(_) => duplicate_dynamic_bases = true,
}
}

errors
};

if duplicate_bases.is_empty() {
if duplicate_dynamic_bases {
Ok(Mro::from_error(
db,
class.apply_optional_specialization(db, specialization),
))
} else {
MroErrorKind::DuplicateBases(duplicate_bases)
Err(MroErrorKind::UnresolvableMro {
bases_list: valid_bases.into_boxed_slice(),
})
}
})
} else {
Err(MroErrorKind::DuplicateBases(
duplicate_bases.into_boxed_slice(),
))
}
}
}
}
Expand Down
Loading