Skip to content

Commit 12287d7

Browse files
fangyi-zhoumeta-codesync[bot]
authored andcommitted
Inconsistent inheritance check follow ups (#1298)
Summary: This PR is a series of minor diffs building up to the correct handling of multiple inheritance checks for methods, following up from #1196. Main changes: - [Skip multiple inheritance check for fields that are overridden](6fccf4a): this diff prevents repeated error reporting when a field gets overridden -- in those cases we don't report inconsistent multiple inheritance - [Skip multiple inheritance checks for "special" fields](b290fd1): this diff skips checks for multiple inheritances for some special fields like constructors. - Cherry picking fixes for some problematic function types in stdlib in vendored typeshed Next steps: We're close to handle multiple inheritance checks for methods. We still have some errors on stdlib due to handling of type variables (#1196 (comment) item 2), which I consider as a blocker. Pull Request resolved: #1298 Reviewed By: kinto0 Differential Revision: D85053799 Pulled By: yangdanny97 fbshipit-source-id: 2e6c22916886dc4c760931cf49522a057c7b42aa
1 parent 1946d0e commit 12287d7

File tree

2 files changed

+55
-22
lines changed

2 files changed

+55
-22
lines changed

pyrefly/lib/alt/class/class_field.rs

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1780,30 +1780,18 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
17801780
.is_some_and(|metadata| metadata.fields.contains_key(field_name))
17811781
}
17821782

1783-
pub fn check_consistent_override_for_field(
1784-
&self,
1785-
cls: &Class,
1786-
field_name: &Name,
1787-
class_field: &ClassField,
1788-
bases: &ClassBases,
1789-
errors: &ErrorCollector,
1790-
) {
1791-
let is_override = class_field.is_override();
1792-
if matches!(class_field.1, IsInherited::No) && !is_override {
1793-
return;
1794-
}
1795-
1783+
fn should_check_field_for_override_consistency(&self, field_name: &Name) -> bool {
17961784
// Object construction (`__new__`, `__init__`, `__init_subclass__`) should not participate in override checks
17971785
if field_name == &dunder::NEW
17981786
|| field_name == &dunder::INIT
17991787
|| field_name == &dunder::INIT_SUBCLASS
18001788
{
1801-
return;
1789+
return false;
18021790
}
18031791

18041792
// `__hash__` is often overridden to `None` to signal hashability
18051793
if field_name == &dunder::HASH {
1806-
return;
1794+
return false;
18071795
}
18081796

18091797
// TODO(grievejia): In principle we should not really skip `__call__`. But the reality is that
@@ -1815,21 +1803,46 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
18151803
// -- any overrider must be able to take ANY arguments which can't be practical. We need to either
18161804
// special-case typeshed or special-case callable subtyping to make `__call__` override check more usable.
18171805
if field_name == &dunder::CALL {
1818-
return;
1806+
return false;
18191807
}
18201808

1821-
// Private attributes should not participate in overrload checks
1809+
// Private attributes should not participate in override checks
18221810
if field_name.starts_with("__") && !field_name.ends_with("__") {
1823-
return;
1811+
return false;
18241812
}
18251813

18261814
// TODO: This should only be ignored when `cls` is a dataclass
18271815
if field_name == &dunder::POST_INIT {
1828-
return;
1816+
return false;
18291817
}
18301818

18311819
// TODO: This should only be ignored when `_ignore_` is defined on enums
18321820
if field_name.as_str() == "_ignore_" {
1821+
return false;
1822+
}
1823+
1824+
// TODO: skipping slots for now to unblock typeshed upgrade
1825+
if field_name == &dunder::SLOTS {
1826+
return false;
1827+
}
1828+
1829+
true
1830+
}
1831+
1832+
pub fn check_consistent_override_for_field(
1833+
&self,
1834+
cls: &Class,
1835+
field_name: &Name,
1836+
class_field: &ClassField,
1837+
bases: &ClassBases,
1838+
errors: &ErrorCollector,
1839+
) {
1840+
let is_override = class_field.is_override();
1841+
if matches!(class_field.1, IsInherited::No) && !is_override {
1842+
return;
1843+
}
1844+
1845+
if !self.should_check_field_for_override_consistency(field_name) {
18331846
return;
18341847
}
18351848

@@ -1995,17 +2008,24 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
19952008
// Maps field from inherited class
19962009
let mro = self.get_mro_for_class(cls);
19972010
let mut inherited_fields: SmallMap<&Name, Vec<(&Name, Type)>> = SmallMap::new();
2011+
let current_class_fields: SmallSet<_> = cls.fields().collect();
19982012

19992013
for parent_cls in mro.ancestors_no_object().iter() {
20002014
let class_fields = parent_cls.class_object().fields();
20012015
for field in class_fields {
2002-
// Skip `__slots__` for now
2003-
if field == &dunder::SLOTS {
2016+
// Skip fields that are not relevant for override consistency checks
2017+
if !self.should_check_field_for_override_consistency(field) {
20042018
continue;
20052019
}
20062020
let key = KeyClassField(parent_cls.class_object().index(), field.clone());
20072021
let field_entry = self.get_from_class(cls, &key);
20082022
if let Some(field_entry) = field_entry.as_ref() {
2023+
// Skip fields that are overridden in the current class,
2024+
// they will have been checked by
2025+
// `check_consistent_override_for_field` already
2026+
if current_class_fields.contains(field) {
2027+
continue;
2028+
}
20092029
inherited_fields
20102030
.entry(field)
20112031
.or_default()

pyrefly/lib/test/class_subtyping.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ class A:
300300
x: int
301301
class B:
302302
x: str
303-
class C(A, B): # E: Field `x` has inconsistent types inherited from multiple base classes
303+
class C(A, B):
304304
x: int # E: Class member `C.x` overrides parent class `B` in an inconsistent manner
305305
class D:
306306
x: int
@@ -324,3 +324,16 @@ class Both(Foo, Bar): # Expect error here
324324
...
325325
"#,
326326
);
327+
328+
testcase!(
329+
test_multiple_inheritance_special_methods,
330+
r#"
331+
class Foo:
332+
def __init__(self, x: int) -> None: ...
333+
class Bar:
334+
def __init__(self, x: str) -> None: ...
335+
336+
class Both(Foo, Bar): # No error here, because __init__ is special
337+
...
338+
"#,
339+
);

0 commit comments

Comments
 (0)