Skip to content

Commit 8ad0e50

Browse files
committed
fix disjointness checks on @final classes
1 parent abaa49f commit 8ad0e50

File tree

4 files changed

+132
-21
lines changed

4 files changed

+132
-21
lines changed

crates/ty_python_semantic/resources/mdtest/type_properties/is_disjoint_from.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ python-version = "3.12"
9595
```
9696

9797
```py
98-
from typing import final
98+
from typing import Any, final
9999
from ty_extensions import static_assert, is_disjoint_from
100100

101101
@final
@@ -106,9 +106,12 @@ class Foo[T]:
106106
class A: ...
107107
class B: ...
108108

109+
static_assert(not is_disjoint_from(A, B))
109110
static_assert(not is_disjoint_from(Foo[A], Foo[B]))
111+
static_assert(not is_disjoint_from(Foo[A], Foo[Any]))
112+
static_assert(not is_disjoint_from(Foo[Any], Foo[B]))
110113

111-
# TODO: `int` and `str` are disjoint bases, so these should be disjoint.
114+
# `Foo[Never]` is a subtype of both `Foo[int]` and `Foo[str]`.
112115
static_assert(not is_disjoint_from(Foo[int], Foo[str]))
113116
```
114117

crates/ty_python_semantic/src/types/class.rs

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,31 @@ impl<'db> ClassType<'db> {
706706
.find_map(|base| base.as_disjoint_base(db))
707707
}
708708

709+
/// Return `true` if this class could exist in the MRO of `other`.
710+
pub(super) fn could_exist_in_mro_of(self, db: &'db dyn Db, other: Self) -> bool {
711+
other
712+
.iter_mro(db)
713+
.filter_map(ClassBase::into_class)
714+
.any(|class| match (self, class) {
715+
(ClassType::NonGeneric(this_class), ClassType::NonGeneric(other_class)) => {
716+
this_class == other_class
717+
}
718+
(ClassType::Generic(this_alias), ClassType::Generic(other_alias)) => {
719+
this_alias.origin(db) == other_alias.origin(db)
720+
&& !this_alias
721+
.specialization(db)
722+
.is_disjoint_from(
723+
db,
724+
other_alias.specialization(db),
725+
InferableTypeVars::None,
726+
)
727+
.is_always_satisfied(db)
728+
}
729+
(ClassType::NonGeneric(_), ClassType::Generic(_))
730+
| (ClassType::Generic(_), ClassType::NonGeneric(_)) => false,
731+
})
732+
}
733+
709734
/// Return `true` if this class could coexist in an MRO with `other`.
710735
///
711736
/// For two given classes `A` and `B`, it is often possible to say for sure
@@ -717,16 +742,11 @@ impl<'db> ClassType<'db> {
717742
}
718743

719744
if self.is_final(db) {
720-
return self
721-
.iter_mro(db)
722-
.filter_map(ClassBase::into_class)
723-
.any(|class| class.class_literal(db).0 == other.class_literal(db).0);
745+
return other.could_exist_in_mro_of(db, self);
724746
}
747+
725748
if other.is_final(db) {
726-
return other
727-
.iter_mro(db)
728-
.filter_map(ClassBase::into_class)
729-
.any(|class| class.class_literal(db).0 == self.class_literal(db).0);
749+
return self.could_exist_in_mro_of(db, other);
730750
}
731751

732752
// Two disjoint bases can only coexist in an MRO if one is a subclass of the other.

crates/ty_python_semantic/src/types/generics.rs

Lines changed: 82 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind, ScopeId};
1111
use crate::semantic_index::{SemanticIndex, semantic_index};
1212
use crate::types::class::ClassType;
1313
use crate::types::class_base::ClassBase;
14-
use crate::types::constraints::ConstraintSet;
14+
use crate::types::constraints::{ConstraintSet, IteratorConstraintsExtension};
1515
use crate::types::instance::{Protocol, ProtocolInstanceType};
1616
use crate::types::signatures::Parameters;
1717
use crate::types::tuple::{TupleSpec, TupleType, walk_tuple_type};
@@ -1166,18 +1166,20 @@ impl<'db> Specialization<'db> {
11661166
let self_materialization_kind = self.materialization_kind(db);
11671167
let other_materialization_kind = other.materialization_kind(db);
11681168

1169-
let mut result = ConstraintSet::from(true);
1170-
for ((bound_typevar, self_type), other_type) in (generic_context.variables(db))
1171-
.zip(self.types(db))
1172-
.zip(other.types(db))
1173-
{
1169+
let types = itertools::izip!(
1170+
generic_context.variables(db),
1171+
self.types(db),
1172+
other.types(db)
1173+
);
1174+
1175+
types.when_all(db, |(bound_typevar, self_type, other_type)| {
11741176
// Subtyping/assignability of each type in the specialization depends on the variance
11751177
// of the corresponding typevar:
11761178
// - covariant: verify that self_type <: other_type
11771179
// - contravariant: verify that other_type <: self_type
11781180
// - invariant: verify that self_type <: other_type AND other_type <: self_type
11791181
// - bivariant: skip, can't make subtyping/assignability false
1180-
let compatible = match bound_typevar.variance(db) {
1182+
match bound_typevar.variance(db) {
11811183
TypeVarVariance::Invariant => has_relation_in_invariant_position(
11821184
db,
11831185
self_type,
@@ -1206,13 +1208,82 @@ impl<'db> Specialization<'db> {
12061208
disjointness_visitor,
12071209
),
12081210
TypeVarVariance::Bivariant => ConstraintSet::from(true),
1209-
};
1210-
if result.intersect(db, compatible).is_never_satisfied(db) {
1211-
return result;
12121211
}
1212+
})
1213+
}
1214+
1215+
pub(crate) fn is_disjoint_from(
1216+
self,
1217+
db: &'db dyn Db,
1218+
other: Self,
1219+
inferable: InferableTypeVars<'_, 'db>,
1220+
) -> ConstraintSet<'db> {
1221+
self.is_disjoint_from_impl(
1222+
db,
1223+
other,
1224+
inferable,
1225+
&IsDisjointVisitor::default(),
1226+
&HasRelationToVisitor::default(),
1227+
)
1228+
}
1229+
1230+
pub(crate) fn is_disjoint_from_impl(
1231+
self,
1232+
db: &'db dyn Db,
1233+
other: Self,
1234+
inferable: InferableTypeVars<'_, 'db>,
1235+
disjointness_visitor: &IsDisjointVisitor<'db>,
1236+
relation_visitor: &HasRelationToVisitor<'db>,
1237+
) -> ConstraintSet<'db> {
1238+
let generic_context = self.generic_context(db);
1239+
if generic_context != other.generic_context(db) {
1240+
return ConstraintSet::from(true);
12131241
}
12141242

1215-
result
1243+
if let (Some(self_tuple), Some(other_tuple)) = (self.tuple_inner(db), other.tuple_inner(db))
1244+
{
1245+
return self_tuple.is_disjoint_from_impl(
1246+
db,
1247+
other_tuple,
1248+
inferable,
1249+
disjointness_visitor,
1250+
relation_visitor,
1251+
);
1252+
}
1253+
1254+
let types = itertools::izip!(
1255+
generic_context.variables(db),
1256+
self.types(db),
1257+
other.types(db)
1258+
);
1259+
1260+
types.when_all(
1261+
db,
1262+
|(bound_typevar, self_type, other_type)| match bound_typevar.variance(db) {
1263+
// TODO: This check can lead to false negatives.
1264+
//
1265+
// For example, `Foo[int]` and `Foo[bool]` are disjoint, even though `bool` is a subtype
1266+
// of `int`. However, given two non-inferable type variables `T` and `U`, `Foo[T]` and
1267+
// `Foo[U]` should not be considered disjoint, as `T` and `U` could be specialized to the
1268+
// same type. We don't currently have a good typing relationship to represent this.
1269+
TypeVarVariance::Invariant => self_type.is_disjoint_from_impl(
1270+
db,
1271+
*other_type,
1272+
inferable,
1273+
disjointness_visitor,
1274+
relation_visitor,
1275+
),
1276+
1277+
// If `Foo[T]` is covariant in `T`, `Foo[Never]` is a subtype of `Foo[A]` and `Foo[B]`
1278+
TypeVarVariance::Covariant => ConstraintSet::from(false),
1279+
1280+
// If `Foo[T]` is contravariant in `T`, `Foo[A | B]` is a subtype of `Foo[A]` and `Foo[B]`
1281+
TypeVarVariance::Contravariant => ConstraintSet::from(false),
1282+
1283+
// If `Foo[T]` is bivariant in `T`, `Foo[A]` and `Foo[B]` are mutual subtypes.
1284+
TypeVarVariance::Bivariant => ConstraintSet::from(false),
1285+
},
1286+
)
12161287
}
12171288

12181289
pub(crate) fn is_equivalent_to_impl(

crates/ty_python_semantic/src/types/tuple.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,23 @@ impl<'db> TupleType<'db> {
287287
)
288288
}
289289

290+
pub(crate) fn is_disjoint_from_impl(
291+
self,
292+
db: &'db dyn Db,
293+
other: Self,
294+
inferable: InferableTypeVars<'_, 'db>,
295+
disjointness_visitor: &IsDisjointVisitor<'db>,
296+
relation_visitor: &HasRelationToVisitor<'db>,
297+
) -> ConstraintSet<'db> {
298+
self.tuple(db).is_disjoint_from_impl(
299+
db,
300+
other.tuple(db),
301+
inferable,
302+
disjointness_visitor,
303+
relation_visitor,
304+
)
305+
}
306+
290307
pub(crate) fn is_equivalent_to_impl(
291308
self,
292309
db: &'db dyn Db,

0 commit comments

Comments
 (0)