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/class/class_metadata.rs b/pyrefly/lib/alt/class/class_metadata.rs index c7de376bb..7aee42c56 100644 --- a/pyrefly/lib/alt/class/class_metadata.rs +++ b/pyrefly/lib/alt/class/class_metadata.rs @@ -284,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() { @@ -971,6 +971,9 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { ErrorInfo::Kind(ErrorKind::InvalidArgument), "Second argument to NewType cannot be a protocol".to_owned(), ); + return None; + } else { + return Some((class_object, metadata)); } } else if metadata.is_new_type() { self.error( @@ -1155,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 cdb565328..43acc9f1f 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,24 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { members ), ); + } else if !metadata.is_protocol() && !metadata.is_new_type() { + 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 !extends_abc && !defines_abstract_member { + self.error( + errors, + cls.range(), + ErrorInfo::Kind(ErrorKind::ImplicitAbstractClass), + format!( + "Class `{}` must implement abstract members: {}", + cls.name(), + members + ), + ); + } } } Arc::new(abstract_members) diff --git a/pyrefly/lib/test/abstract_methods.rs b/pyrefly/lib/test/abstract_methods.rs index 7d5b72021..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 @@ -93,7 +95,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 @@ -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 @@ -132,7 +135,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") @@ -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 @@ -215,7 +219,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__() @@ -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 @@ -250,7 +255,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 "#, 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.