Skip to content

Commit fdf5de4

Browse files
asukaminato0721meta-codesync[bot]
authored andcommitted
fix Error when calling abstract method on super() #1358 (#1363)
Summary: fix #1358 Pull Request resolved: #1363 Reviewed By: stroxler Differential Revision: D85364483 Pulled By: yangdanny97 fbshipit-source-id: 53e9478a88eddf1aaf9ad0b39487b382ed195a65
1 parent 2a47c60 commit fdf5de4

File tree

13 files changed

+156
-9
lines changed

13 files changed

+156
-9
lines changed

conformance/third_party/conformance.exp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8768,6 +8768,17 @@
87688768
}
87698769
],
87708770
"protocols_explicit.py": [
8771+
{
8772+
"code": -2,
8773+
"column": 16,
8774+
"concise_description": "Method `draw` inherited from class `PColor` has no implementation and cannot be accessed via `super()`",
8775+
"description": "Method `draw` inherited from class `PColor` has no implementation and cannot be accessed via `super()`",
8776+
"line": 27,
8777+
"name": "missing-attribute",
8778+
"severity": "error",
8779+
"stop_column": 28,
8780+
"stop_line": 27
8781+
},
87718782
{
87728783
"code": -2,
87738784
"column": 20,

conformance/third_party/conformance.result

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,6 @@
252252
"Line 79: Unexpected errors ['`Concrete` is not assignable to `Template`\\n Protocol `Template` requires attribute `temp`']"
253253
],
254254
"protocols_explicit.py": [
255-
"Line 27: Expected 1 errors",
256255
"Line 89: Expected 1 errors"
257256
],
258257
"protocols_generic.py": [],

conformance/third_party/results.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"pass": 99,
44
"fail": 39,
55
"pass_rate": 0.72,
6-
"differences": 158,
6+
"differences": 157,
77
"passing": [
88
"aliases_explicit.py",
99
"aliases_newtype.py",
@@ -136,7 +136,7 @@
136136
"literals_interactions.py": 2,
137137
"protocols_class_objects.py": 7,
138138
"protocols_definition.py": 3,
139-
"protocols_explicit.py": 2,
139+
"protocols_explicit.py": 1,
140140
"protocols_modules.py": 1,
141141
"protocols_runtime_checkable.py": 3,
142142
"protocols_variance.py": 7,

crates/pyrefly_types/src/callable.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,10 @@ pub struct FuncFlags {
344344
pub has_final_decoration: bool,
345345
/// A function decorated with `@abc.abstractmethod`
346346
pub is_abstract_method: bool,
347+
/// Function body is treated as a stub (e.g. body is `...` or absent in a stub file)
348+
pub lacks_implementation: bool,
349+
/// Is the function definition in a `.pyi` file
350+
pub defined_in_stub_file: bool,
347351
/// A function decorated with `typing.dataclass_transform(...)`, turning it into a
348352
/// `dataclasses.dataclass`-like decorator. Stores the keyword values passed to the
349353
/// `dataclass_transform` call. See

crates/pyrefly_types/src/types.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,13 @@ impl Type {
11061106
self.check_toplevel_func_metadata(&|meta| meta.flags.dataclass_transform_metadata.clone())
11071107
}
11081108

1109+
/// If a Protocol method lacks an implementation and does not come from a `.pyi` file, then it cannot be called
1110+
pub fn is_non_callable_protocol_method(&self) -> bool {
1111+
self.check_toplevel_func_metadata(&|meta| {
1112+
meta.flags.lacks_implementation && !meta.flags.defined_in_stub_file
1113+
})
1114+
}
1115+
11091116
/// Transforms this type's function metadata, if it is a function. Note that we do *not*
11101117
/// recurse into the type to find nested function types.
11111118
pub fn transform_toplevel_func_metadata(&mut self, mut f: impl FnMut(&mut FuncMetadata)) {

pyrefly/lib/alt/attr.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,8 @@ pub enum NoAccessReason {
239239
/// We do not allow class-level mutation of descriptors (this is conservative,
240240
/// it is unspecified whether monkey-patching descriptors should be permitted).
241241
SettingDescriptorOnClass(Class),
242+
/// Calling a method via `super()` when no implementation is available (e.g. abstract protocol or abstract base method).
243+
SuperMethodNeedsImplementation(Class),
242244
}
243245

244246
#[derive(Debug)]
@@ -294,6 +296,12 @@ impl NoAccessReason {
294296
"Attribute `{attr_name}` of class `{class_name}` is a read-only descriptor with no `__set__` and cannot be set"
295297
)
296298
}
299+
NoAccessReason::SuperMethodNeedsImplementation(class) => {
300+
let class_name = class.name();
301+
format!(
302+
"Method `{attr_name}` inherited from class `{class_name}` has no implementation and cannot be accessed via `super()`"
303+
)
304+
}
297305
}
298306
}
299307
}

pyrefly/lib/alt/class/class_field.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,12 @@ impl ClassField {
564564
}
565565
}
566566

567+
fn is_non_callable_protocol_method(&self) -> bool {
568+
match &self.0 {
569+
ClassFieldInner::Simple { ty, .. } => ty.is_non_callable_protocol_method(),
570+
}
571+
}
572+
567573
pub fn is_foreign_key(&self) -> bool {
568574
match &self.0 {
569575
ClassFieldInner::Simple { is_foreign_key, .. } => *is_foreign_key,
@@ -2557,16 +2563,43 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
25572563
SuperObj::Instance(obj) => self
25582564
.get_super_class_member(obj.class_object(), Some(start_lookup_cls), name)
25592565
.map(|member| {
2560-
self.as_instance_attribute(&member.value, &Instance::of_self_type(obj))
2566+
if let Some(reason) = self.super_method_needs_impl_reason(&member) {
2567+
ClassAttribute::no_access(reason)
2568+
} else {
2569+
self.as_instance_attribute(&member.value, &Instance::of_self_type(obj))
2570+
}
25612571
}),
25622572
SuperObj::Class(obj) => self
25632573
.get_super_class_member(obj.class_object(), Some(start_lookup_cls), name)
25642574
.map(|member| {
2565-
self.as_class_attribute(&member.value, &ClassBase::SelfType(obj.clone()))
2575+
if let Some(reason) = self.super_method_needs_impl_reason(&member) {
2576+
ClassAttribute::no_access(reason)
2577+
} else {
2578+
self.as_class_attribute(&member.value, &ClassBase::SelfType(obj.clone()))
2579+
}
25662580
}),
25672581
}
25682582
}
25692583

2584+
fn super_method_needs_impl_reason(
2585+
&self,
2586+
member: &WithDefiningClass<Arc<ClassField>>,
2587+
) -> Option<NoAccessReason> {
2588+
if member.value.is_abstract() {
2589+
return Some(NoAccessReason::SuperMethodNeedsImplementation(
2590+
member.defining_class.dupe(),
2591+
));
2592+
}
2593+
let metadata = self.get_metadata_for_class(&member.defining_class);
2594+
if metadata.is_protocol() && member.value.is_non_callable_protocol_method() {
2595+
Some(NoAccessReason::SuperMethodNeedsImplementation(
2596+
member.defining_class.dupe(),
2597+
))
2598+
} else {
2599+
None
2600+
}
2601+
}
2602+
25702603
/// Gets an attribute from a class definition.
25712604
///
25722605
/// Returns `None` if there is no such attribute, otherwise an `Attribute` object

pyrefly/lib/alt/function.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
218218
class_key: Option<&Idx<KeyClass>>,
219219
decorators: &[(Idx<Key>, TextRange)],
220220
legacy_tparams: &[Idx<KeyLegacyTypeParam>],
221+
module_style: ModuleStyle,
221222
errors: &ErrorCollector,
222223
) -> Arc<UndecoratedFunction> {
223224
let defining_cls = class_key.and_then(|k| self.get_idx(*k).0.dupe());
@@ -263,6 +264,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
263264
.map(|(idx, range)| (self.get_idx(*idx).arc_clone_ty(), *range)),
264265
);
265266

267+
if stub_or_impl == FunctionStubOrImpl::Stub {
268+
flags.lacks_implementation = true;
269+
}
270+
if module_style == ModuleStyle::Interface {
271+
flags.defined_in_stub_file = true;
272+
}
273+
266274
// Look for a @classmethod or @staticmethod decorator and change the "self" type
267275
// accordingly. This is not totally correct, since it doesn't account for chaining
268276
// decorators, or weird cases like both decorators existing at the same time.

pyrefly/lib/alt/solve.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3414,6 +3414,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
34143414
x.class_key.as_ref(),
34153415
&x.decorators,
34163416
&x.legacy_tparams,
3417+
x.module_style,
34173418
errors,
34183419
)
34193420
}

pyrefly/lib/binding/binding.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use pyrefly_derive::TypeEq;
1515
use pyrefly_derive::VisitMut;
1616
use pyrefly_python::dunder;
1717
use pyrefly_python::module_name::ModuleName;
18+
use pyrefly_python::module_path::ModuleStyle;
1819
use pyrefly_python::nesting_context::NestingContext;
1920
use pyrefly_python::short_identifier::ShortIdentifier;
2021
use pyrefly_python::symbol_kind::SymbolKind;
@@ -1009,6 +1010,7 @@ pub struct BindingUndecoratedFunction {
10091010
pub class_key: Option<Idx<KeyClass>>,
10101011
pub legacy_tparams: Box<[Idx<KeyLegacyTypeParam>]>,
10111012
pub decorators: Box<[(Idx<Key>, TextRange)]>,
1013+
pub module_style: ModuleStyle,
10121014
}
10131015

10141016
impl DisplayWith<Bindings> for BindingUndecoratedFunction {

0 commit comments

Comments
 (0)