diff --git a/crates/ty_python_semantic/resources/mdtest/mro.md b/crates/ty_python_semantic/resources/mdtest/mro.md index 1e08923946cb80..a2d2c26e906b6d 100644 --- a/crates/ty_python_semantic/resources/mdtest/mro.md +++ b/crates/ty_python_semantic/resources/mdtest/mro.md @@ -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[, Unknown, ] ``` +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[, Unknown, ] +``` + ## Unrelated objects inferred as `Any`/`Unknown` do not have special `__mro__` attributes ```py diff --git a/crates/ty_python_semantic/src/types/mro.rs b/crates/ty_python_semantic/src/types/mro.rs index cd73fc7967c3e0..83dac9841aab6f 100644 --- a/crates/ty_python_semantic/src/types/mro.rs +++ b/crates/ty_python_semantic/src/types/mro.rs @@ -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, Vec> = - 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> = { + let mut base_to_indices: FxHashMap, Vec> = + 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(), + )) + } } } }