Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ class FileLoader:
path: str
def __init__(self, fullname: str, path: str) -> None: ...
def get_data(self, path: str) -> bytes: ...
def get_filename(self, name: str | None = None) -> str: ...
def load_module(self, name: str | None = None) -> types.ModuleType: ...
def get_filename(self, fullname: str | None = None) -> str: ...
def load_module(self, fullname: str | None = None) -> types.ModuleType: ...
if sys.version_info >= (3, 10):
def get_resource_reader(self, name: str | None = None) -> importlib.readers.FileReader: ...
else:
Expand All @@ -135,7 +135,7 @@ class SourcelessFileLoader(importlib.abc.FileLoader, FileLoader, _LoaderBasics):

class ExtensionFileLoader(FileLoader, _LoaderBasics, importlib.abc.ExecutionLoader):
def __init__(self, name: str, path: str) -> None: ...
def get_filename(self, name: str | None = None) -> str: ...
def get_filename(self, fullname: str | None = None) -> str: ...
def get_source(self, fullname: str) -> None: ...
def create_module(self, spec: ModuleSpec) -> types.ModuleType: ...
def exec_module(self, module: types.ModuleType) -> None: ...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ class FileLoader(_bootstrap_external.FileLoader, ResourceLoader, ExecutionLoader
path: str
def __init__(self, fullname: str, path: str) -> None: ...
def get_data(self, path: str) -> bytes: ...
def get_filename(self, name: str | None = None) -> str: ...
def load_module(self, name: str | None = None) -> types.ModuleType: ...
def get_filename(self, fullname: str | None = None) -> str: ...
def load_module(self, fullname: str | None = None) -> types.ModuleType: ...

if sys.version_info < (3, 11):
class ResourceReader(metaclass=ABCMeta):
Expand Down
57 changes: 38 additions & 19 deletions pyrefly/lib/alt/class/class_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1754,30 +1754,18 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
.is_some_and(|metadata| metadata.fields.contains_key(field_name))
}

pub fn check_consistent_override_for_field(
&self,
cls: &Class,
field_name: &Name,
class_field: &ClassField,
bases: &ClassBases,
errors: &ErrorCollector,
) {
let is_override = class_field.is_override();
if matches!(class_field.1, IsInherited::No) && !is_override {
return;
}

fn should_check_field_for_override_consistency(&self, field_name: &Name) -> bool {
// Object construction (`__new__`, `__init__`, `__init_subclass__`) should not participate in override checks
if field_name == &dunder::NEW
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dataclasses' post_init as well

|| field_name == &dunder::INIT
|| field_name == &dunder::INIT_SUBCLASS
{
return;
return false;
}

// `__hash__` is often overridden to `None` to signal hashability
if field_name == &dunder::HASH {
return;
return false;
}

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

// Private attributes should not participate in overrload checks
// Private attributes should not participate in override checks
if field_name.starts_with("__") && !field_name.ends_with("__") {
return;
return false;
}

// TODO: This should only be ignored when `cls` is a dataclass
if field_name == &dunder::POST_INIT {
return;
return false;
}

// TODO: This should only be ignored when `_ignore_` is defined on enums
if field_name.as_str() == "_ignore_" {
return false;
}

true
}

pub fn check_consistent_override_for_field(
&self,
cls: &Class,
field_name: &Name,
class_field: &ClassField,
bases: &ClassBases,
errors: &ErrorCollector,
) {
let is_override = class_field.is_override();
if matches!(class_field.1, IsInherited::No) && !is_override {
return;
}

if !self.should_check_field_for_override_consistency(field_name) {
return;
}

Expand Down Expand Up @@ -1969,13 +1977,24 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
// Maps field from inherited class
let mro = self.get_mro_for_class(cls);
let mut inherited_fields: SmallMap<&Name, Vec<(&Name, Type)>> = SmallMap::new();
let current_class_fields: SmallSet<_> = cls.fields().collect();

for parent_cls in mro.ancestors_no_object().iter() {
let class_fields = parent_cls.class_object().fields();
for field in class_fields {
// Skip fields that are not relevant for override consistency checks
if !self.should_check_field_for_override_consistency(field) {
continue;
}
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() {
// Skip fields that are overridden in the current class,
// they will have been checked by
// `check_consistent_override_for_field` already
if current_class_fields.contains(field) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we can lift this check up to the beginning of the loop body, it's a bit buried nested in here

continue;
}
inherited_fields
.entry(field)
.or_default()
Expand Down
15 changes: 14 additions & 1 deletion pyrefly/lib/test/class_subtyping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ class A:
x: int
class B:
x: str
class C(A, B): # E: Field `x` has inconsistent types inherited from multiple base classes
class C(A, B):
x: int # E: Class member `C.x` overrides parent class `B` in an inconsistent manner
class D:
x: int
Expand All @@ -324,3 +324,16 @@ class Both(Foo, Bar): # Expect error here
...
"#,
);

testcase!(
test_multiple_inheritance_special_methods,
r#"
class Foo:
def __init__(self, x: int) -> None: ...
class Bar:
def __init__(self, x: str) -> None: ...

class Both(Foo, Bar): # No error here, because __init__ is special
...
"#,
);
Loading