Skip to content

Commit c36cf36

Browse files
migeed-zfacebook-github-bot
authored andcommitted
Skip inapplicable parent overloads during override checking (#2880)
Summary: When binding an overloaded method to an object, filter out overloads whose `self` parameter type is incompatible with the object type. This fixes false positive `bad-override` errors when subclassing `str` (and similar classes with `LiteralString` overloads) and adding optional parameters to overridden methods. For example, `str.upper()` has two overloads: 1. `(self: LiteralString) -> LiteralString` 2. `(self) -> str` When `MyString(str)` overrides `upper()`, the override check previously required the child to satisfy ALL parent overloads, including the `LiteralString` one. Since `MyString` is not `LiteralString`, the check failed with a false positive. Now, inapplicable overloads are filtered out during method binding. Fixes #2309 Differential Revision: D97805178
1 parent e4e54a9 commit c36cf36

File tree

2 files changed

+68
-2
lines changed

2 files changed

+68
-2
lines changed

pyrefly/lib/alt/class/class_field.rs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3165,6 +3165,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
31653165
}
31663166
attr
31673167
};
3168+
let want_attribute = self.filter_overloads_for_override(want_attribute, cls);
31683169
if got_attribute.is_none() {
31693170
// Optimisation: Only compute the `got_attr` once, and only if we actually need it.
31703171
got_attribute = Some(self.as_instance_attribute(
@@ -4239,6 +4240,72 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
42394240
}
42404241
}
42414242

4243+
/// Filter out overloads from a parent's attribute whose `self` parameter type is
4244+
/// incompatible with the child class. This prevents false positive `bad-override` errors
4245+
/// when the parent has overloads specialized on `self` types (e.g., `LiteralString`) that
4246+
/// don't apply to the child class.
4247+
fn filter_overloads_for_override(
4248+
&self,
4249+
attr: ClassAttribute,
4250+
child_cls: &Class,
4251+
) -> ClassAttribute {
4252+
let child_type = self
4253+
.heap
4254+
.mk_class_type(self.as_class_type_unchecked(child_cls));
4255+
let filter_type = |ty: Type| -> Type {
4256+
match ty {
4257+
Type::BoundMethod(box BoundMethod {
4258+
obj,
4259+
func: BoundMethodType::Overload(overload),
4260+
}) => {
4261+
let dominated: Vec<usize> = overload
4262+
.signatures
4263+
.iter()
4264+
.enumerate()
4265+
.filter(|(_, sig)| {
4266+
matches!(sig, OverloadType::Function(f)
4267+
if f.signature.get_first_param()
4268+
.is_some_and(|param| !self.is_subset_eq(&child_type, &param)))
4269+
})
4270+
.map(|(i, _)| i)
4271+
.collect();
4272+
if dominated.is_empty() || dominated.len() == overload.signatures.len() {
4273+
BoundMethod {
4274+
obj,
4275+
func: BoundMethodType::Overload(overload),
4276+
}
4277+
.as_type()
4278+
} else {
4279+
let signatures = overload
4280+
.signatures
4281+
.into_iter()
4282+
.enumerate()
4283+
.filter(|(i, _)| !dominated.contains(i))
4284+
.map(|(_, sig)| sig)
4285+
.collect::<Vec<_>>();
4286+
BoundMethod {
4287+
obj,
4288+
func: BoundMethodType::Overload(Overload {
4289+
signatures: vec1::Vec1::try_from_vec(signatures)
4290+
.expect("at least one overload remains after filtering"),
4291+
metadata: overload.metadata,
4292+
}),
4293+
}
4294+
.as_type()
4295+
}
4296+
}
4297+
other => other,
4298+
}
4299+
};
4300+
match attr {
4301+
ClassAttribute::ReadWrite(ty) => ClassAttribute::ReadWrite(filter_type(ty)),
4302+
ClassAttribute::ReadOnly(ty, reason) => {
4303+
ClassAttribute::ReadOnly(filter_type(ty), reason)
4304+
}
4305+
other => other,
4306+
}
4307+
}
4308+
42424309
pub fn is_class_attribute_subset(
42434310
&self,
42444311
got: &ClassAttribute,

pyrefly/lib/test/class_overrides.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,11 +1298,10 @@ class B(A):
12981298
);
12991299

13001300
testcase!(
1301-
bug = "False positive bad-override when subclassing str with LiteralString overloads",
13021301
test_override_str_subclass_with_optional_param,
13031302
r#"
13041303
class MyString(str):
1305-
def upper(self, locale: str = "en") -> str: # E: Class member `MyString.upper` overrides parent class `str` in an inconsistent manner
1304+
def upper(self, locale: str = "en") -> str:
13061305
return super().upper()
13071306
"#,
13081307
);

0 commit comments

Comments
 (0)