Check for compatibility when inheriting from multiple classes for fields (#1196)

Summary:
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 field types is `never`. If so, we raise an error.

Next step: Handle method overrides / overloads.

Pull Request resolved: #1196

Reviewed By: stroxler

Differential Revision: D84155735

Pulled By: yangdanny97

fbshipit-source-id: 1b1e526951ae3ed79034d188fe2cb413d9027b26

Author: fangyi-zhou

conformance/third_party/conformance.exp

@@ -9961,6 +9961,16 @@
       "name": "bad-override",
       "stop_column": 5,
       "stop_line": 55
+    },
+    {
+      "code": -2,
+      "column": 7,
+      "concise_description": "Field `x` has inconsistent types inherited from multiple base classes",
+      "description": "Field `x` has inconsistent types inherited from multiple base classes\n  Inherited types include:\n    `int` from `X2`\n    `str` from `Y2`",
+      "line": 65,
+      "name": "inconsistent-inheritance",
+      "stop_column": 11,
+      "stop_line": 65
     }
   ],
   "typeddicts_operations.py": [

conformance/third_party/conformance.result

@@ -343,9 +343,7 @@
   "typeddicts_class_syntax.py": [],
   "typeddicts_extra_items.py": [],
   "typeddicts_final.py": [],
-  "typeddicts_inheritance.py": [
-    "Line 65: Expected 1 errors"
-  ],
+  "typeddicts_inheritance.py": [],
   "typeddicts_operations.py": [],
   "typeddicts_readonly.py": [],
   "typeddicts_readonly_consistency.py": [],

conformance/third_party/results.json

@@ -1,9 +1,9 @@
 {
   "total": 138,
-  "pass": 94,
-  "fail": 44,
-  "pass_rate": 0.68,
-  "differences": 180,
+  "pass": 95,
+  "fail": 43,
+  "pass_rate": 0.69,
+  "differences": 179,
   "passing": [
     "aliases_explicit.py",
     "aliases_newtype.py",
@@ -92,6 +92,7 @@
     "typeddicts_class_syntax.py",
     "typeddicts_extra_items.py",
     "typeddicts_final.py",
+    "typeddicts_inheritance.py",
     "typeddicts_operations.py",
     "typeddicts_readonly.py",
     "typeddicts_readonly_consistency.py",
@@ -142,7 +143,6 @@
     "qualifiers_final_annotation.py": 12,
     "specialtypes_never.py": 1,
     "specialtypes_type.py": 8,
-    "typeddicts_inheritance.py": 1,
     "typeddicts_readonly_inheritance.py": 4,
     "typeddicts_required.py": 1
   },

crates/pyrefly_config/src/error_kind.rs

@@ -140,6 +140,8 @@ pub enum ErrorKind {
     /// An error related to the import machinery.
     /// e.g. failed to import a module.
     ImportError,
+    /// An inconsistency between inherited fields or methods from multiple base classes.
+    InconsistentInheritance,
     /// An inconsistency between the signature of a function overload and the implementation.
     InconsistentOverload,
     /// Attempting to access a container with an incorrect index.

pyrefly/lib/alt/class/class_field.rs

@@ -1964,6 +1964,67 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
         }
     }
 
+    /// For classes with multiple inheritance, check that fields inherited from multiple base classes are consistent.
+    pub fn check_consistent_multiple_inheritance(&self, cls: &Class, errors: &ErrorCollector) {
+        // Maps field from inherited class
+        let mro = self.get_mro_for_class(cls);
+        let mut inherited_fields: SmallMap<&Name, Vec<(&Name, Type)>> = SmallMap::new();
+
+        for parent_cls in mro.ancestors_no_object().iter() {
+            let class_fields = parent_cls.class_object().fields();
+            for field in class_fields {
+                let key = KeyClassField(parent_cls.class_object().index(), field.clone());
+                let field_entry = self.get_from_class(cls, &key);
+                if let Some(field_entry) = field_entry.as_ref() {
+                    inherited_fields
+                        .entry(field)
+                        .or_default()
+                        .push((parent_cls.name(), field_entry.ty()));
+                }
+            }
+        }
+
+        for (field_name, class_and_types) in inherited_fields.iter() {
+            if class_and_types.len() > 1 {
+                let types: Vec<Type> = class_and_types.iter().map(|(_, ty)| ty.clone()).collect();
+                if types.iter().any(|ty| {
+                    matches!(
+                        ty,
+                        Type::BoundMethod(..)
+                            | Type::Function(..)
+                            | Type::Forall(_)
+                            | Type::Overload(_)
+                    )
+                }) {
+                    // TODO(fangyizhou): Handle bound methods and functions properly.
+                    // This is a leftover from https://github.com/facebook/pyrefly/pull/1196
+                    continue;
+                }
+                let intersect = self.intersects(&types);
+                if matches!(intersect, Type::Never(_)) {
+                    let mut error_msg = vec1![
+                        format!(
+                            "Field `{field_name}` has inconsistent types inherited from multiple base classes"
+                        ),
+                        "Inherited types include:".to_owned()
+                    ];
+                    for (cls, ty) in class_and_types.iter() {
+                        error_msg.push(format!(
+                            "  `{}` from `{}`",
+                            self.for_display(ty.clone()),
+                            cls
+                        ));
+                    }
+                    errors.add(
+                        cls.range(),
+                        ErrorInfo::Kind(ErrorKind::InconsistentInheritance),
+                        error_msg,
+                    );
+                }
+            }
+        }
+    }
+
     fn get_non_synthesized_field_from_current_class_only(
         &self,
         cls: &Class,

pyrefly/lib/alt/narrow.rs

@@ -100,7 +100,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
         self.intersect_with_fallback(left, right, Type::never)
     }
 
-    fn intersects(&self, ts: &[Type]) -> Type {
+    /// Calculate the intersection of a number of types
+    pub fn intersects(&self, ts: &[Type]) -> Type {
         match ts {
             [] => Type::ClassType(self.stdlib.object().clone()),
             [ty] => ty.clone(),

pyrefly/lib/alt/solve.rs

@@ -1557,6 +1557,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
                     errors,
                 );
             }
+
+            // If we are inheriting from multiple base types, we should
+            // check whether the multiple inheritance is consistent
+            if class_bases.as_ref().base_type_count() > 1 {
+                self.check_consistent_multiple_inheritance(cls, errors);
+            }
         }
         Arc::new(EmptyAnswer)
     }

pyrefly/lib/alt/types/class_bases.rs

@@ -62,6 +62,10 @@ impl ClassBases {
     pub fn is_empty(&self) -> bool {
         self.base_types.is_empty()
     }
+
+    pub fn base_type_count(&self) -> usize {
+        self.base_types.len()
+    }
 }
 
 impl fmt::Display for ClassBases {

pyrefly/lib/alt/types/class_metadata.rs

@@ -438,7 +438,7 @@ pub struct TotalOrderingMetadata {
 /// If a class is present in multiple places of the inheritance tree (and is
 /// linearizable using C3 linearization), it is possible it appears with
 /// different type arguments. The type arguments computed here will always be
-/// those coming from the instance that was selected during lineariation.
+/// those coming from the instance that was selected during linearization.
 #[derive(Clone, Debug, VisitMut, TypeEq, PartialEq, Eq)]
 pub enum ClassMro {
     Resolved(Vec<ClassType>),

pyrefly/lib/test/class_subtyping.rs

@@ -261,3 +261,66 @@ class B(type(A)):  # E: Invalid expression form for base class: `type(A)`
 assert_type(B.x, Any)
     "#,
 );
+
+testcase!(
+    test_multiple_inheritance_incompatible_field,
+    r#"
+class Foo:
+    p: int
+class Bar:
+    p: str
+
+class Both(Foo, Bar): # E: Field `p` has inconsistent types inherited from multiple base classes
+    ...
+"#,
+);
+
+testcase!(
+    test_nested_multiple_inheritance_incompatible_field_without_override,
+    r#"
+class A:
+    x: int
+class B:
+    x: str
+class C(A, B): # E: Field `x` has inconsistent types inherited from multiple base classes
+    pass
+class D:
+    x: int
+
+# Here we repeat the error on E, despite the error already being reported in C.
+class E(C, D): # E: Field `x` has inconsistent types inherited from multiple base classes
+    pass
+"#,
+);
+
+testcase!(
+    test_nested_multiple_inheritance_incompatible_field_with_override,
+    r#"
+class A:
+    x: int
+class B:
+    x: str
+class C(A, B): # E: Field `x` has inconsistent types inherited from multiple base classes
+    x: int # E: Class member `C.x` overrides parent class `B` in an inconsistent manner
+class D:
+    x: int
+
+# Here we still report the error on E, despite the field being overridden in C.
+class E(C, D): # E: Field `x` has inconsistent types inherited from multiple base classes
+    pass
+"#,
+);
+
+testcase!(
+    bug = "This is currently not handled",
+    test_multiple_inheritance_incompatible_methods,
+    r#"
+class Foo:
+    def foo(self) -> int: ...
+class Bar:
+    def foo(self) -> str: ...
+
+class Both(Foo, Bar): # Expect error here
+    ...
+"#,
+);