From 558e5391022991a5b20222f61d0fe0cb72eda227 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 17 Oct 2025 03:20:21 +0900 Subject: [PATCH 1/7] fix --- pyrefly/lib/alt/class/class_metadata.rs | 2 ++ pyrefly/lib/alt/solve.rs | 45 ++++++++++++++++++++----- pyrefly/lib/alt/types/class_metadata.rs | 22 +++++++++--- pyrefly/lib/test/abstract_methods.rs | 8 ++--- 4 files changed, 61 insertions(+), 16 deletions(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index c7de376bb..710dc802b 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -146,6 +146,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { "metaclass" => Either::Left(x), _ => Either::Right((n.clone(), self.expr_class_keyword(x, errors))), }); + let has_explicit_metaclass = !metaclasses.is_empty(); let base_metaclasses = bases_with_metadata .iter() @@ -313,6 +314,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ClassMetadata::new( bases, metaclass, + has_explicit_metaclass, keywords, typed_dict_metadata, named_tuple_metadata, diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index cdb565328..a5861fad9 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -318,14 +318,14 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ) -> Arc { let metadata = self.get_metadata_for_class(cls); let abstract_members = self.calculate_abstract_members(cls); - if metadata.is_final() { - let unimplemented = abstract_members.unimplemented_abstract_methods(); - if !unimplemented.is_empty() { - let members = unimplemented - .iter() - .map(|member| format!("`{member}`")) - .collect::>() - .join(", "); + let unimplemented = abstract_members.unimplemented_abstract_methods(); + if !unimplemented.is_empty() { + let members = unimplemented + .iter() + .map(|member| format!("`{member}`")) + .collect::>() + .join(", "); + if metadata.is_final() { self.error( errors, cls.range(), @@ -336,6 +336,35 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { members ), ); + } else if !metadata.is_protocol() { + let has_direct_abc_base = metadata + .base_class_objects() + .iter() + .any(|base| base.has_toplevel_qname("abc", "ABC")); + let has_explicit_abc_metaclass = metadata.has_explicit_metaclass() + && metadata.custom_metaclass().is_some_and(|meta| { + meta.class_object() + .has_toplevel_qname("abc", "ABCMeta") + }); + let defines_abstract_member = cls.fields().any(|name| { + self.get_field_from_current_class_only(cls, name) + .is_some_and(|field| field.is_abstract()) + }); + if !has_direct_abc_base + && !has_explicit_abc_metaclass + && !defines_abstract_member + { + self.error( + errors, + cls.range(), + ErrorInfo::Kind(ErrorKind::BadClassDefinition), + format!( + "Class `{}` must implement abstract members: {}", + cls.name(), + members + ), + ); + } } } Arc::new(abstract_members) diff --git a/pyrefly/lib/alt/types/class_metadata.rs b/pyrefly/lib/alt/types/class_metadata.rs index b8a64a22a..fc9742e0b 100644 --- a/pyrefly/lib/alt/types/class_metadata.rs +++ b/pyrefly/lib/alt/types/class_metadata.rs @@ -79,6 +79,7 @@ impl ClassMetadata { pub fn new( bases: Vec, metaclass: Option, + has_explicit_metaclass: bool, keywords: Vec<(Name, Type)>, typed_dict_metadata: Option, named_tuple_metadata: Option, @@ -97,7 +98,7 @@ impl ClassMetadata { is_django_model: bool, ) -> ClassMetadata { ClassMetadata { - metaclass: Metaclass(metaclass), + metaclass: Metaclass::new(metaclass, has_explicit_metaclass), keywords: Keywords(keywords), typed_dict_metadata, named_tuple_metadata, @@ -143,7 +144,7 @@ impl ClassMetadata { /// The class's custom (non-`type`) metaclass, if it has one. pub fn custom_metaclass(&self) -> Option<&ClassType> { - self.metaclass.0.as_ref() + self.metaclass.value.as_ref() } /// The class's metaclass. @@ -152,6 +153,10 @@ impl ClassMetadata { .unwrap_or_else(|| stdlib.builtins_type()) } + pub fn has_explicit_metaclass(&self) -> bool { + self.metaclass.explicit + } + #[allow(dead_code)] // This is used in tests now, and will be needed later in production. pub fn keywords(&self) -> &[(Name, Type)] { &self.keywords.0 @@ -331,11 +336,20 @@ impl Display for ClassSynthesizedFields { /// A struct representing a class's metaclass. A value of `None` indicates /// no explicit metaclass, in which case the default metaclass is `type`. #[derive(Clone, Debug, TypeEq, PartialEq, Eq, Default)] -struct Metaclass(Option); +struct Metaclass { + value: Option, + explicit: bool, +} + +impl Metaclass { + fn new(value: Option, explicit: bool) -> Self { + Self { value, explicit } + } +} impl Display for Metaclass { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match &self.0 { + match &self.value { Some(metaclass) => write!(f, "{metaclass}"), None => write!(f, "type"), } diff --git a/pyrefly/lib/test/abstract_methods.rs b/pyrefly/lib/test/abstract_methods.rs index 7d5b72021..5a6f6f6bd 100644 --- a/pyrefly/lib/test/abstract_methods.rs +++ b/pyrefly/lib/test/abstract_methods.rs @@ -93,7 +93,7 @@ class Base(ABC): def method(self) -> None: pass -class Child(Base): +class Child(Base): # E: Class `Child` must implement abstract members: `method` # Child doesn't implement method, so it's still abstract pass @@ -132,7 +132,7 @@ class Base(ABC): def method2(self) -> None: pass -class Partial(Base): +class Partial(Base): # E: Class `Partial` must implement abstract members: `method2` def method1(self) -> None: print("implemented") @@ -215,7 +215,7 @@ class Base(ABC): @abstractmethod def processor(self) -> bool: pass -class Child(Base): +class Child(Base): # E: Class `Child` must implement abstract members: `processor` def __init__(self) -> None: super().__init__() @@ -250,7 +250,7 @@ class C(ABC): async def bar(self) -> Any: pass -class D(C): +class D(C): # E: Class `D` must implement abstract members: `bar` async def foo(self) -> AsyncIterator[int]: yield 1 "#, From f9ba9b5ac4cc7f50234d0b042fb295c039851e79 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 17 Oct 2025 03:21:24 +0900 Subject: [PATCH 2/7] fmt --- pyrefly/lib/alt/solve.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index a5861fad9..3e80c9521 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -343,17 +343,13 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .any(|base| base.has_toplevel_qname("abc", "ABC")); let has_explicit_abc_metaclass = metadata.has_explicit_metaclass() && metadata.custom_metaclass().is_some_and(|meta| { - meta.class_object() - .has_toplevel_qname("abc", "ABCMeta") + meta.class_object().has_toplevel_qname("abc", "ABCMeta") }); let defines_abstract_member = cls.fields().any(|name| { self.get_field_from_current_class_only(cls, name) .is_some_and(|field| field.is_abstract()) }); - if !has_direct_abc_base - && !has_explicit_abc_metaclass - && !defines_abstract_member - { + if !has_direct_abc_base && !has_explicit_abc_metaclass && !defines_abstract_member { self.error( errors, cls.range(), From 3e5aaafdd3dcd51d4a003e35b004aab03653836f Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 17 Oct 2025 03:36:41 +0900 Subject: [PATCH 3/7] fix --- pyrefly/lib/alt/class/class_metadata.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index 710dc802b..068da6ca2 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -973,16 +973,21 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ErrorInfo::Kind(ErrorKind::InvalidArgument), "Second argument to NewType cannot be a protocol".to_owned(), ); + None + } else { + Some((class_object, metadata)) } - } else if metadata.is_new_type() { - self.error( - errors, - range, - ErrorInfo::Kind(ErrorKind::InvalidInheritance), - "Subclassing a NewType not allowed".to_owned(), - ); + } else { + if metadata.is_new_type() { + self.error( + errors, + range, + ErrorInfo::Kind(ErrorKind::InvalidInheritance), + "Subclassing a NewType not allowed".to_owned(), + ); + } + Some((class_object, metadata)) } - Some((class_object, metadata)) } }) .collect::>() From 075cb064b33e5ceac8fa318f897e684bf9a9d8f5 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Fri, 17 Oct 2025 15:34:21 +0900 Subject: [PATCH 4/7] add new error type --- crates/pyrefly_config/src/error_kind.rs | 4 ++++ pyrefly/lib/alt/solve.rs | 4 ++-- pyrefly/lib/test/abstract_methods.rs | 5 +++++ pyrefly/lib/test/util.rs | 10 ++++++++++ website/docs/error-kinds.mdx | 6 ++++++ 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index b26c51a12..b817d3cc7 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -131,6 +131,9 @@ pub enum ErrorKind { DeleteError, /// Calling a function marked with `@deprecated` Deprecated, + /// Raised when a class implicitly becomes abstract by defining abstract members without + /// inheriting from `abc.ABC` or using `abc.ABCMeta`. + ImplicitAbstractClass, /// This error is raised when Pyrefly infers an implicit `Any` ImplicitAny, /// Usage of a module that was not actually imported, but does exist. @@ -276,6 +279,7 @@ impl ErrorKind { ErrorKind::Deprecated => Severity::Warn, ErrorKind::RedundantCast => Severity::Warn, ErrorKind::ImplicitlyDefinedAttribute => Severity::Ignore, + ErrorKind::ImplicitAbstractClass => Severity::Ignore, ErrorKind::ImplicitAny => Severity::Ignore, _ => Severity::Error, } diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index 3e80c9521..e107ca678 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -336,7 +336,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { members ), ); - } else if !metadata.is_protocol() { + } else if !metadata.is_protocol() && !metadata.is_new_type() { let has_direct_abc_base = metadata .base_class_objects() .iter() @@ -353,7 +353,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { self.error( errors, cls.range(), - ErrorInfo::Kind(ErrorKind::BadClassDefinition), + ErrorInfo::Kind(ErrorKind::ImplicitAbstractClass), format!( "Class `{}` must implement abstract members: {}", cls.name(), diff --git a/pyrefly/lib/test/abstract_methods.rs b/pyrefly/lib/test/abstract_methods.rs index 5a6f6f6bd..f6c04ac33 100644 --- a/pyrefly/lib/test/abstract_methods.rs +++ b/pyrefly/lib/test/abstract_methods.rs @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +use crate::test::util::TestEnv; use crate::testcase; testcase!( @@ -85,6 +86,7 @@ drawable = Drawable() # E: Cannot instantiate `Drawable` testcase!( test_inherited_abstract_method, + TestEnv::new().enable_implicit_abstract_class_error(), r#" from abc import ABC, abstractmethod @@ -120,6 +122,7 @@ x = BadClass() # E: Cannot instantiate `BadClass` testcase!( test_partial_implementation, + TestEnv::new().enable_implicit_abstract_class_error(), r#" from abc import ABC, abstractmethod @@ -205,6 +208,7 @@ c = Child() testcase!( test_abstract_property, + TestEnv::new().enable_implicit_abstract_class_error(), r#" from typing import * from abc import ABC, abstractmethod @@ -225,6 +229,7 @@ x = Child() # E: Cannot instantiate `Child` testcase!( test_abstract_async_iterator, + TestEnv::new().enable_implicit_abstract_class_error(), r#" from abc import ABC, abstractmethod from collections.abc import AsyncIterator diff --git a/pyrefly/lib/test/util.rs b/pyrefly/lib/test/util.rs index a0817a87f..539e25e87 100644 --- a/pyrefly/lib/test/util.rs +++ b/pyrefly/lib/test/util.rs @@ -105,6 +105,7 @@ pub struct TestEnv { site_package_path: Vec, implicitly_defined_attribute_error: bool, implicit_any_error: bool, + implicit_abstract_class_error: bool, default_require_level: Require, } @@ -120,6 +121,7 @@ impl TestEnv { site_package_path: Vec::new(), implicitly_defined_attribute_error: false, implicit_any_error: false, + implicit_abstract_class_error: false, default_require_level: Require::Exports, } } @@ -158,6 +160,11 @@ impl TestEnv { self } + pub fn enable_implicit_abstract_class_error(mut self) -> Self { + self.implicit_abstract_class_error = true; + self + } + pub fn with_default_require_level(mut self, level: Require) -> Self { self.default_require_level = level; self @@ -231,6 +238,9 @@ impl TestEnv { if self.implicit_any_error { errors.set_error_severity(ErrorKind::ImplicitAny, Severity::Error); } + if self.implicit_abstract_class_error { + errors.set_error_severity(ErrorKind::ImplicitAbstractClass, Severity::Error); + } let mut sourcedb = MapDatabase::new(config.get_sys_info()); for (name, path, _) in self.modules.iter() { sourcedb.insert(*name, path.dupe()); diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index e03555da6..95fd1ce8d 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -347,6 +347,12 @@ def f(): ... f() # deprecated! ``` +## implicit-abstract-class + +Pyrefly emits this error when a class defines abstract members but is not declared abstract (for example, it does not inherit from `abc.ABC` or use `abc.ABCMeta`). Such classes cannot be instantiated because they have unimplemented abstract methods. Add `ABC` as a base class, adjust the metaclass, or provide concrete implementations to resolve the issue. + +This error defaults to `Ignore`, so opt in by enabling it in your configuration if you want to enforce explicit abstract base classes. + ## implicit-any This error is emitted when a Pyrefly infers an implicit `Any` type in your code. This is common in gradually-typed code, but reduces type safety so we provide this error for users that want to enforce fully-typed codebases. From f755ab7209c6cb641fad68689f744a7089a96c49 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Sat, 18 Oct 2025 05:00:50 +0900 Subject: [PATCH 5/7] collapsed --- pyrefly/lib/alt/class/class_metadata.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index 068da6ca2..a79f59838 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -977,17 +977,15 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } else { Some((class_object, metadata)) } - } else { - if metadata.is_new_type() { - self.error( - errors, - range, - ErrorInfo::Kind(ErrorKind::InvalidInheritance), - "Subclassing a NewType not allowed".to_owned(), - ); - } - Some((class_object, metadata)) + } else if metadata.is_new_type() { + self.error( + errors, + range, + ErrorInfo::Kind(ErrorKind::InvalidInheritance), + "Subclassing a NewType not allowed".to_owned(), + ); } + Some((class_object, metadata)) } }) .collect::>() From 61c0f5669eebe8e6c69c3e360b3c1ff026aeadc6 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Sat, 18 Oct 2025 05:07:50 +0900 Subject: [PATCH 6/7] fix --- pyrefly/lib/alt/class/class_metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index a79f59838..87057b7ca 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -973,9 +973,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ErrorInfo::Kind(ErrorKind::InvalidArgument), "Second argument to NewType cannot be a protocol".to_owned(), ); - None + return None; } else { - Some((class_object, metadata)) + return Some((class_object, metadata)); } } else if metadata.is_new_type() { self.error( From 6f6634dfbd17fbefa79d236b5f060dd5e8320129 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Sat, 18 Oct 2025 05:41:52 +0900 Subject: [PATCH 7/7] rely on extends_abc(err) --- pyrefly/lib/alt/class/class_metadata.rs | 10 ++++++---- pyrefly/lib/alt/solve.rs | 11 ++--------- pyrefly/lib/alt/types/class_metadata.rs | 22 ++++------------------ 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/pyrefly/lib/alt/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index 87057b7ca..7aee42c56 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -146,7 +146,6 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { "metaclass" => Either::Left(x), _ => Either::Right((n.clone(), self.expr_class_keyword(x, errors))), }); - let has_explicit_metaclass = !metaclasses.is_empty(); let base_metaclasses = bases_with_metadata .iter() @@ -285,7 +284,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { { self.validate_frozen_dataclass_inheritance(cls, dm, &bases_with_metadata, errors); } - let extends_abc = self.extends_abc(&bases_with_metadata); + let extends_abc = self.extends_abc(&bases_with_metadata, metaclass.as_ref()); // Compute final base class list. let bases = if is_typed_dict && bases_with_metadata.is_empty() { @@ -314,7 +313,6 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ClassMetadata::new( bases, metaclass, - has_explicit_metaclass, keywords, typed_dict_metadata, named_tuple_metadata, @@ -1160,7 +1158,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { AbstractClassMembers::new(abstract_members) } - fn extends_abc(&self, bases_with_metadata: &Vec<(Class, Arc)>) -> bool { + fn extends_abc( + &self, + bases_with_metadata: &Vec<(Class, Arc)>, + metaclass: Option<&ClassType>, + ) -> bool { for (base, base_metadata) in bases_with_metadata { if base.has_toplevel_qname("abc", "ABC") { return true; diff --git a/pyrefly/lib/alt/solve.rs b/pyrefly/lib/alt/solve.rs index e107ca678..43acc9f1f 100644 --- a/pyrefly/lib/alt/solve.rs +++ b/pyrefly/lib/alt/solve.rs @@ -337,19 +337,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ), ); } else if !metadata.is_protocol() && !metadata.is_new_type() { - let has_direct_abc_base = metadata - .base_class_objects() - .iter() - .any(|base| base.has_toplevel_qname("abc", "ABC")); - let has_explicit_abc_metaclass = metadata.has_explicit_metaclass() - && metadata.custom_metaclass().is_some_and(|meta| { - meta.class_object().has_toplevel_qname("abc", "ABCMeta") - }); + let extends_abc = metadata.extends_abc(); let defines_abstract_member = cls.fields().any(|name| { self.get_field_from_current_class_only(cls, name) .is_some_and(|field| field.is_abstract()) }); - if !has_direct_abc_base && !has_explicit_abc_metaclass && !defines_abstract_member { + if !extends_abc && !defines_abstract_member { self.error( errors, cls.range(), diff --git a/pyrefly/lib/alt/types/class_metadata.rs b/pyrefly/lib/alt/types/class_metadata.rs index fc9742e0b..b8a64a22a 100644 --- a/pyrefly/lib/alt/types/class_metadata.rs +++ b/pyrefly/lib/alt/types/class_metadata.rs @@ -79,7 +79,6 @@ impl ClassMetadata { pub fn new( bases: Vec, metaclass: Option, - has_explicit_metaclass: bool, keywords: Vec<(Name, Type)>, typed_dict_metadata: Option, named_tuple_metadata: Option, @@ -98,7 +97,7 @@ impl ClassMetadata { is_django_model: bool, ) -> ClassMetadata { ClassMetadata { - metaclass: Metaclass::new(metaclass, has_explicit_metaclass), + metaclass: Metaclass(metaclass), keywords: Keywords(keywords), typed_dict_metadata, named_tuple_metadata, @@ -144,7 +143,7 @@ impl ClassMetadata { /// The class's custom (non-`type`) metaclass, if it has one. pub fn custom_metaclass(&self) -> Option<&ClassType> { - self.metaclass.value.as_ref() + self.metaclass.0.as_ref() } /// The class's metaclass. @@ -153,10 +152,6 @@ impl ClassMetadata { .unwrap_or_else(|| stdlib.builtins_type()) } - pub fn has_explicit_metaclass(&self) -> bool { - self.metaclass.explicit - } - #[allow(dead_code)] // This is used in tests now, and will be needed later in production. pub fn keywords(&self) -> &[(Name, Type)] { &self.keywords.0 @@ -336,20 +331,11 @@ impl Display for ClassSynthesizedFields { /// A struct representing a class's metaclass. A value of `None` indicates /// no explicit metaclass, in which case the default metaclass is `type`. #[derive(Clone, Debug, TypeEq, PartialEq, Eq, Default)] -struct Metaclass { - value: Option, - explicit: bool, -} - -impl Metaclass { - fn new(value: Option, explicit: bool) -> Self { - Self { value, explicit } - } -} +struct Metaclass(Option); impl Display for Metaclass { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match &self.value { + match &self.0 { Some(metaclass) => write!(f, "{metaclass}"), None => write!(f, "type"), }