Skip to content

Commit 0c3384d

Browse files
committed
Check for compatibility when inherting from multiple classes
This diff is an attempt to address the issue in #943. When a class inherits from multiple base classes, we check whether the intersection of the types of the fields is never. If that's the case, we raise an error. Note 1: There are some errors when type checking the builtins (which is reflected by errors in scrut tests). They seem to be caused by subtyping checks of self types in functions and generic function types. Note 2: Should this be a new error category? I'm currently reusing the closest error category for inconsistent overloads, but that looks very specific.
1 parent 255aa20 commit 0c3384d

File tree

5 files changed

+68
-2
lines changed

5 files changed

+68
-2
lines changed

pyrefly/lib/alt/narrow.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
101101
self.intersect_with_fallback(left, right, Type::never)
102102
}
103103

104-
fn intersects(&self, ts: &[Type]) -> Type {
104+
/// Calculate the intersection of a number of types
105+
pub fn intersects(&self, ts: &[Type]) -> Type {
105106
match ts {
106107
[] => Type::ClassType(self.stdlib.object().clone()),
107108
[ty] => ty.clone(),

pyrefly/lib/alt/solve.rs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use std::iter;
99
use std::sync::Arc;
1010

1111
use dupe::Dupe;
12+
use itertools::Itertools;
1213
use pyrefly_python::ast::Ast;
1314
use pyrefly_python::dunder;
1415
use pyrefly_python::short_identifier::ShortIdentifier;
@@ -72,6 +73,7 @@ use crate::binding::binding::FunctionParameter;
7273
use crate::binding::binding::FunctionStubOrImpl;
7374
use crate::binding::binding::IsAsync;
7475
use crate::binding::binding::Key;
76+
use crate::binding::binding::KeyClassField;
7577
use crate::binding::binding::KeyExport;
7678
use crate::binding::binding::KeyUndecoratedFunction;
7779
use crate::binding::binding::LastStmt;
@@ -1533,6 +1535,52 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
15331535
errors,
15341536
);
15351537
}
1538+
1539+
// If we are inheriting from multiple base types, we should
1540+
// check whether the multiple inheritance is consistent
1541+
if class_bases.as_ref().base_type_count() > 1 {
1542+
// Maps field from inherited class
1543+
let mro = self.get_mro_for_class(cls);
1544+
let mut inherited_fields: SmallMap<&Name, Vec<(&Name, Type)>> = SmallMap::new();
1545+
1546+
for parent_cls in mro.ancestors_no_object().iter() {
1547+
let class_fields = parent_cls.class_object().fields();
1548+
for field in class_fields {
1549+
let key = KeyClassField(parent_cls.class_object().index(), field.clone());
1550+
let field_entry = self.get_from_class(cls, &key);
1551+
if let Some(field_entry) = field_entry.as_ref() {
1552+
inherited_fields
1553+
.entry(field)
1554+
.or_default()
1555+
.push((parent_cls.name(), field_entry.ty()));
1556+
}
1557+
}
1558+
}
1559+
1560+
for (field_name, class_and_types) in inherited_fields.iter() {
1561+
if class_and_types.len() > 1 {
1562+
let types: Vec<Type> =
1563+
class_and_types.iter().map(|(_, ty)| ty.clone()).collect();
1564+
let intersect = self.intersects(&types);
1565+
if matches!(intersect, Type::Never(_)) {
1566+
let class_and_types_str = class_and_types
1567+
.iter()
1568+
.map(|(cls, ty)| {
1569+
format!("`{}` from `{}`", self.for_display(ty.clone()), cls)
1570+
})
1571+
.join(", ");
1572+
self.error(
1573+
errors,
1574+
cls.range(),
1575+
ErrorInfo::Kind(ErrorKind::InconsistentOverload),
1576+
format!(
1577+
"Inconsistent types for field `{field_name}` inherited from multiple base classes: {class_and_types_str}",
1578+
),
1579+
);
1580+
}
1581+
}
1582+
}
1583+
}
15361584
}
15371585
Arc::new(EmptyAnswer)
15381586
}

pyrefly/lib/alt/types/class_bases.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ impl ClassBases {
5858
pub fn iter(&self) -> impl Iterator<Item = &ClassType> {
5959
self.base_types.iter()
6060
}
61+
62+
pub fn base_type_count(&self) -> usize {
63+
self.base_types.len()
64+
}
6165
}
6266

6367
impl fmt::Display for ClassBases {

pyrefly/lib/alt/types/class_metadata.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ pub struct TotalOrderingMetadata {
401401
/// If a class is present in multiple places of the inheritance tree (and is
402402
/// linearizable using C3 linearization), it is possible it appears with
403403
/// different type arguments. The type arguments computed here will always be
404-
/// those coming from the instance that was selected during lineariation.
404+
/// those coming from the instance that was selected during linearization.
405405
#[derive(Clone, Debug, VisitMut, TypeEq, PartialEq, Eq)]
406406
pub enum ClassMro {
407407
Resolved(Vec<ClassType>),

pyrefly/lib/test/class_subtyping.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,3 +261,16 @@ class B(type(A)): # E: Invalid expression form for base class: `type(A)`
261261
assert_type(B.x, Any)
262262
"#,
263263
);
264+
265+
testcase!(
266+
test_multiple_inheritance_incompatible_field,
267+
r#"
268+
class Foo:
269+
p: int
270+
class Bar:
271+
p: str
272+
273+
class Both(Foo, Bar): # E: Inconsistent types for field `p` inherited from multiple base classes: `int` from `Foo`, `str` from `Bar`
274+
...
275+
"#,
276+
);

0 commit comments

Comments
 (0)