diff --git a/conformance/third_party/conformance.exp b/conformance/third_party/conformance.exp index fdd2405a5..2051f9704 100644 --- a/conformance/third_party/conformance.exp +++ b/conformance/third_party/conformance.exp @@ -9981,6 +9981,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": [ diff --git a/conformance/third_party/conformance.result b/conformance/third_party/conformance.result index e3cd3078d..848d61b2a 100644 --- a/conformance/third_party/conformance.result +++ b/conformance/third_party/conformance.result @@ -345,9 +345,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": [], diff --git a/conformance/third_party/results.json b/conformance/third_party/results.json index 8acc3ab4e..a23de3feb 100644 --- a/conformance/third_party/results.json +++ b/conformance/third_party/results.json @@ -1,9 +1,9 @@ { "total": 138, - "pass": 94, - "fail": 44, - "pass_rate": 0.68, - "differences": 182, + "pass": 95, + "fail": 43, + "pass_rate": 0.69, + "differences": 181, "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 }, diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index 6530212c8..981ad1cd9 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/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 form 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. diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index 7aec49163..586bf3b0c 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -1957,6 +1957,62 @@ 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 = class_and_types.iter().map(|(_, ty)| ty.clone()).collect(); + if types + .iter() + .any(|ty| matches!(ty, Type::BoundMethod(..) | Type::Function(..))) + { + // TODO(fangyizhou): Handle bound methods and funtions 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, diff --git a/pyrefly/lib/alt/narrow.rs b/pyrefly/lib/alt/narrow.rs index aff9b5d00..f4841d0bb 100644 --- a/pyrefly/lib/alt/narrow.rs +++ b/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(), diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 508dad773..af0ce44f5 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/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) } diff --git a/pyrefly/lib/alt/types/class_bases.rs b/pyrefly/lib/alt/types/class_bases.rs index 0165b28fa..5092fa504 100644 --- a/pyrefly/lib/alt/types/class_bases.rs +++ b/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 { diff --git a/pyrefly/lib/alt/types/class_metadata.rs b/pyrefly/lib/alt/types/class_metadata.rs index 82d6ccb2d..935ba0e57 100644 --- a/pyrefly/lib/alt/types/class_metadata.rs +++ b/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), diff --git a/pyrefly/lib/test/class_subtyping.rs b/pyrefly/lib/test/class_subtyping.rs index cb0916ddd..7c4d87ebd 100644 --- a/pyrefly/lib/test/class_subtyping.rs +++ b/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 + ... +"#, +); diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 71eda1312..592ec82c9 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -381,6 +381,19 @@ An error related to the import mechanism, such as when a module cannot be found. The error message will include which paths were searched, such as the site package paths. You may be missing a dependency, or you may need to inform Pyrefly where the module lives. See [Configuration](configuration.mdx) for further information. +## inconsistent-inheritance + +When a class inherits from multiple base classes, the inherited fields must be consistent. + +Example: +```python +class A: + f: str +class B: + f: int +class C(A, B): ... # error, the field `f` is inconsistent +``` + ## inconsistent-overload The signature of a function overload is inconsistent with the implementation.