From 7593cce08859c6f94adb4ba40103a8cc8ff35122 Mon Sep 17 00:00:00 2001 From: Fangyi Zhou Date: Mon, 8 Dec 2025 00:23:20 +0000 Subject: [PATCH 1/4] Consider an uninitialised ClassVar as abstract As per title, this diff addresses the last remaining issue in the conformance test `protocols_explicit.py` by requiring declared classvar members to be initialised for protocols. --- conformance/third_party/conformance.exp | 11 +++++++++++ conformance/third_party/conformance.result | 4 +--- conformance/third_party/results.json | 8 ++++---- pyrefly/lib/alt/class/class_field.rs | 21 +++++++++++++++++++++ pyrefly/lib/alt/class/class_metadata.rs | 2 +- pyrefly/lib/test/protocol.rs | 15 +++++++++++++++ 6 files changed, 53 insertions(+), 8 deletions(-) diff --git a/conformance/third_party/conformance.exp b/conformance/third_party/conformance.exp index dcfebb6b8..332a045b6 100644 --- a/conformance/third_party/conformance.exp +++ b/conformance/third_party/conformance.exp @@ -8778,6 +8778,17 @@ "stop_column": 20, "stop_line": 60 }, + { + "code": -2, + "column": 15, + "concise_description": "Cannot instantiate `Concrete1` because the following members are abstract: `cm1`", + "description": "Cannot instantiate `Concrete1` because the following members are abstract: `cm1`", + "line": 89, + "name": "bad-instantiation", + "severity": "error", + "stop_column": 17, + "stop_line": 89 + }, { "code": -2, "column": 15, diff --git a/conformance/third_party/conformance.result b/conformance/third_party/conformance.result index 24dead98f..bb39a001b 100644 --- a/conformance/third_party/conformance.result +++ b/conformance/third_party/conformance.result @@ -242,9 +242,7 @@ "Line 117: Expected 1 errors", "Line 79: Unexpected errors ['`Concrete` is not assignable to `Template`\\n Protocol `Template` requires attribute `temp`']" ], - "protocols_explicit.py": [ - "Line 89: Expected 1 errors" - ], + "protocols_explicit.py": [], "protocols_generic.py": [], "protocols_merging.py": [], "protocols_modules.py": [ diff --git a/conformance/third_party/results.json b/conformance/third_party/results.json index c0226f8da..bba035a28 100644 --- a/conformance/third_party/results.json +++ b/conformance/third_party/results.json @@ -1,9 +1,9 @@ { "total": 138, - "pass": 103, - "fail": 35, + "pass": 104, + "fail": 34, "pass_rate": 0.75, - "differences": 146, + "differences": 145, "passing": [ "aliases_explicit.py", "aliases_newtype.py", @@ -81,6 +81,7 @@ "overloads_consistency.py", "overloads_definitions.py", "overloads_evaluation.py", + "protocols_explicit.py", "protocols_generic.py", "protocols_merging.py", "protocols_recursive.py", @@ -138,7 +139,6 @@ "literals_interactions.py": 2, "protocols_class_objects.py": 4, "protocols_definition.py": 3, - "protocols_explicit.py": 1, "protocols_modules.py": 1, "protocols_variance.py": 7, "qualifiers_annotated.py": 6, diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index 7f42164a1..9b2ed2373 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -852,6 +852,27 @@ impl ClassField { } } + pub fn is_uninit_class_var(&self) -> bool { + match &self.0 { + ClassFieldInner::Property { .. } => false, + ClassFieldInner::Descriptor { .. } => false, + ClassFieldInner::Method { .. } => false, + ClassFieldInner::NestedClass { .. } => false, + ClassFieldInner::ClassAttribute { + is_classvar, + initialization, + .. + } => { + *is_classvar + && matches!( + initialization, + ClassFieldInitialization::Uninitialized | ClassFieldInitialization::Magic + ) + } + ClassFieldInner::InstanceAttribute { .. } => false, + } + } + pub fn is_init_var(&self) -> bool { match &self.0 { ClassFieldInner::Property { .. } => false, diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index 83f9b951c..5092dc91e 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -1212,7 +1212,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { let mut abstract_members = SmallSet::new(); for field_name in fields_to_check { if let Some(field) = self.get_non_synthesized_class_member(cls, &field_name) - && field.is_abstract() + && (field.is_abstract() || field.is_uninit_class_var()) { abstract_members.insert(field_name.clone()); } diff --git a/pyrefly/lib/test/protocol.rs b/pyrefly/lib/test/protocol.rs index aa942ba5c..85f0f9283 100644 --- a/pyrefly/lib/test/protocol.rs +++ b/pyrefly/lib/test/protocol.rs @@ -752,3 +752,18 @@ isinstance(Impl(), GenericProtocol) # E: Runtime checkable protocol `GenericPro issubclass(Impl, GenericProtocol) # E: Runtime checkable protocol `GenericProtocol` has an unsafe overlap with type `Impl` "#, ); + +testcase!( + test_protocol_with_uninit_classvar, + r#" +from typing import Protocol, ClassVar, final +class P(Protocol): + x: ClassVar[int] + +@final +class C(P): # E: Final class `C` cannot have unimplemented abstract members: `x` + pass + +c = C() # E: Cannot instantiate `C` because the following members are abstract: `x` +"#, +); From c1434d40fe5351df2a2ee2491e62862df868722b Mon Sep 17 00:00:00 2001 From: Fangyi Zhou Date: Mon, 8 Dec 2025 22:26:24 +0000 Subject: [PATCH 2/4] Add a test case for abstract class with class var --- pyrefly/lib/test/abstract_methods.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pyrefly/lib/test/abstract_methods.rs b/pyrefly/lib/test/abstract_methods.rs index 3ce3b8e26..ef52133dc 100644 --- a/pyrefly/lib/test/abstract_methods.rs +++ b/pyrefly/lib/test/abstract_methods.rs @@ -417,3 +417,16 @@ class D(C): # E: Class `D` has unimplemented abstract members: `bar` yield 1 "#, ); + + +testcase!( + test_uninit_classvar_abc, + r#" +from abc import ABC +from typing import ClassVar, final +@final +class A(ABC): # E: Final class `A` cannot have unimplemented abstract members: `x` + x: ClassVar[int] +a = A() # E: Cannot instantiate `A` because the following members are abstract: `x` +"#, +); From 212887d9ae73210aae007c889013154f2df3d37c Mon Sep 17 00:00:00 2001 From: Fangyi Zhou Date: Tue, 9 Dec 2025 22:41:43 +0000 Subject: [PATCH 3/4] Special case __hash__ --- pyrefly/lib/alt/class/class_metadata.rs | 7 +++++++ pyrefly/lib/test/abstract_methods.rs | 1 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index 5092dc91e..7b329e862 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -1209,6 +1209,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .cloned(), ); } + // skip __hash__, but we might need to find a better way to handle it in the future. + // Context: + // https://github.com/facebook/pyrefly/pull/1797#issuecomment-3629910569 + // and + // https://github.com/python/typeshed/issues/2148 + fields_to_check.shift_remove(&Name::new_static("__hash__")); + let mut abstract_members = SmallSet::new(); for field_name in fields_to_check { if let Some(field) = self.get_non_synthesized_class_member(cls, &field_name) diff --git a/pyrefly/lib/test/abstract_methods.rs b/pyrefly/lib/test/abstract_methods.rs index ef52133dc..0b8ca01cd 100644 --- a/pyrefly/lib/test/abstract_methods.rs +++ b/pyrefly/lib/test/abstract_methods.rs @@ -418,7 +418,6 @@ class D(C): # E: Class `D` has unimplemented abstract members: `bar` "#, ); - testcase!( test_uninit_classvar_abc, r#" From 4a54d806f857a5fbc56cab380678de1e787e2b4d Mon Sep 17 00:00:00 2001 From: Fangyi Zhou Date: Tue, 9 Dec 2025 23:35:38 +0000 Subject: [PATCH 4/4] Don't consider class vars in interface files as abstract --- pyrefly/lib/alt/class/class_metadata.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index 7b329e862..475544a07 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -1209,17 +1209,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .cloned(), ); } - // skip __hash__, but we might need to find a better way to handle it in the future. - // Context: - // https://github.com/facebook/pyrefly/pull/1797#issuecomment-3629910569 - // and - // https://github.com/python/typeshed/issues/2148 - fields_to_check.shift_remove(&Name::new_static("__hash__")); let mut abstract_members = SmallSet::new(); for field_name in fields_to_check { if let Some(field) = self.get_non_synthesized_class_member(cls, &field_name) - && (field.is_abstract() || field.is_uninit_class_var()) + && (field.is_abstract() || + // If a class variable is declared on the interface file, don't consider as abstract + !cls.module().path().is_interface() && field.is_uninit_class_var()) { abstract_members.insert(field_name.clone()); }