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
23 changes: 23 additions & 0 deletions crates/ty_python_semantic/resources/mdtest/mro.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,29 @@ reveal_type(E.__mro__) # revealed: tuple[<class 'E'>, <class 'B'>, <class 'C'>,
reveal_type(F.__mro__)
```

## Inheritance with intersections that include `Unknown`

An intersection that includes `Unknown` or `Any` is permitted as long as the intersection is not
disjoint from `type`.

```py
from does_not_exist import DoesNotExist # error: [unresolved-import]

reveal_type(DoesNotExist) # revealed: Unknown

if hasattr(DoesNotExist, "__mro__"):
reveal_type(DoesNotExist) # revealed: Unknown & <Protocol with members '__mro__'>

class Foo(DoesNotExist): ... # no error!
reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]

if not isinstance(DoesNotExist, type):
reveal_type(DoesNotExist) # revealed: Unknown & ~type

class Foo(DoesNotExist): ... # error: [invalid-base]
reveal_type(Foo.__mro__) # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
```

## `__bases__` lists that cause errors at runtime

If the class's `__bases__` cause an exception to be raised at runtime and therefore the class
Expand Down
7 changes: 7 additions & 0 deletions crates/ty_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,13 @@ impl<'db> Type<'db> {
Self::Dynamic(DynamicType::Unknown)
}

pub(crate) fn into_dynamic(self) -> Option<DynamicType> {
match self {
Type::Dynamic(dynamic_type) => Some(dynamic_type),
_ => None,
}
}

pub fn object(db: &'db dyn Db) -> Self {
KnownClass::Object.to_instance(db)
}
Expand Down
13 changes: 12 additions & 1 deletion crates/ty_python_semantic/src/types/class_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,19 @@ impl<'db> ClassBase<'db> {
{
Self::try_from_type(db, todo_type!("GenericAlias instance"))
}
Type::Intersection(inter) => {
let dynamic_element = inter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks good, but is there any reason it should be specific to a dynamic element? It seems like as long as the intersection is not disjoint from type, and we can find at least one element in it that is a valid base, we should be able to safely consider it that type and construct the base accordingly.

(Not saying that change needs to happen in this PR -- but the thought could maybe go into a comment, to help us out if it comes up in future.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's... an interesting thought. I'm not sure it has any practical consequences right now, because the only other "atomic" types we support as class bases currently are class-literal types and dynamic types. <dynamic type> & whatever is handled here. <class literal type> & whatever should always either simplify to <class literal type> or Never, since class-literal types only have a single inhabitant.

(We probably should allow users to inherit from objects of type[Any], though, due to it actually being an intersection. And if we did that, then yes, this would have more practical consequences. I can do that in a followup. And I'll tackle this at the same time!)

.positive(db)
.iter()
.find_map(|elem| elem.into_dynamic())?;

if ty.is_disjoint_from(db, KnownClass::Type.to_instance(db)) {
None
} else {
Some(ClassBase::Dynamic(dynamic_element))
}
}
Type::Union(_) => None, // TODO -- forces consideration of multiple possible MROs?
Type::Intersection(_) => None, // TODO -- probably incorrect?
Type::NominalInstance(_) => None, // TODO -- handle `__mro_entries__`?
Type::PropertyInstance(_) => None,
Type::Never
Expand Down
Loading