mirror of https://github.com/astral-sh/ruff
[ty] Improve check enforcing that an overloaded function must have an implementation (#21978)
## Summary - Treat `if TYPE_CHECKING` blocks the same as stub files (the feature requested in https://github.com/astral-sh/ty/issues/1216) - We currently only allow `@abstractmethod`-decorated methods to omit the implementation if they're methods in classes that have _exactly_ `ABCMeta` as their metaclass. That seems wrong -- `@abstractmethod` has the same semantics if a class has a subclass of `ABCMeta` as its metaclass. This PR fixes that too. (I'm actually not _totally_ sure we should care what the class's metaclass is at all -- see discussion in https://github.com/astral-sh/ty/issues/1877#issue-3725937441... but the change this PR is making seems less wrong than what we have currently, anyway.) Fixes https://github.com/astral-sh/ty/issues/1216 ## Test Plan Mdtests and snapshots
This commit is contained in:
parent
9838f81baf
commit
0b918ae4d5
|
|
@ -418,6 +418,18 @@ Using the `@abstractmethod` decorator requires that the class's metaclass is `AB
|
|||
from it.
|
||||
|
||||
```py
|
||||
from abc import ABCMeta
|
||||
|
||||
class CustomAbstractMetaclass(ABCMeta): ...
|
||||
|
||||
class Fine(metaclass=CustomAbstractMetaclass):
|
||||
@overload
|
||||
@abstractmethod
|
||||
def f(self, x: int) -> int: ...
|
||||
@overload
|
||||
@abstractmethod
|
||||
def f(self, x: str) -> str: ...
|
||||
|
||||
class Foo:
|
||||
@overload
|
||||
@abstractmethod
|
||||
|
|
@ -448,6 +460,52 @@ class PartialFoo(ABC):
|
|||
def f(self, x: str) -> str: ...
|
||||
```
|
||||
|
||||
#### `TYPE_CHECKING` blocks
|
||||
|
||||
As in other areas of ty, we treat `TYPE_CHECKING` blocks the same as "inline stub files", so we
|
||||
permit overloaded functions to exist without an implementation if all overloads are defined inside
|
||||
an `if TYPE_CHECKING` block:
|
||||
|
||||
```py
|
||||
from typing import overload, TYPE_CHECKING
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@overload
|
||||
def a() -> str: ...
|
||||
@overload
|
||||
def a(x: int) -> int: ...
|
||||
|
||||
class F:
|
||||
@overload
|
||||
def method(self) -> None: ...
|
||||
@overload
|
||||
def method(self, x: int) -> int: ...
|
||||
|
||||
class G:
|
||||
if TYPE_CHECKING:
|
||||
@overload
|
||||
def method(self) -> None: ...
|
||||
@overload
|
||||
def method(self, x: int) -> int: ...
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@overload
|
||||
def b() -> str: ...
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@overload
|
||||
def b(x: int) -> int: ...
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@overload
|
||||
def c() -> None: ...
|
||||
|
||||
# not all overloads are in a `TYPE_CHECKING` block, so this is an error
|
||||
@overload
|
||||
# error: [invalid-overload]
|
||||
def c(x: int) -> int: ...
|
||||
```
|
||||
|
||||
### `@overload`-decorated functions with non-stub bodies
|
||||
|
||||
<!-- snapshot-diagnostics -->
|
||||
|
|
|
|||
|
|
@ -42,7 +42,11 @@ error[invalid-overload]: Overloads for function `func` must be followed by a non
|
|||
9 | class Foo:
|
||||
|
|
||||
info: Attempting to call `func` will raise `TypeError` at runtime
|
||||
info: Overloaded functions without implementations are only permitted in stub files, on protocols, or for abstract methods
|
||||
info: Overloaded functions without implementations are only permitted:
|
||||
info: - in stub files
|
||||
info: - in `if TYPE_CHECKING` blocks
|
||||
info: - as methods on protocol classes
|
||||
info: - or as `@abstractmethod`-decorated methods on abstract classes
|
||||
info: See https://docs.python.org/3/library/typing.html#typing.overload for more details
|
||||
info: rule `invalid-overload` is enabled by default
|
||||
|
||||
|
|
@ -58,7 +62,11 @@ error[invalid-overload]: Overloads for function `method` must be followed by a n
|
|||
| ^^^^^^
|
||||
|
|
||||
info: Attempting to call `method` will raise `TypeError` at runtime
|
||||
info: Overloaded functions without implementations are only permitted in stub files, on protocols, or for abstract methods
|
||||
info: Overloaded functions without implementations are only permitted:
|
||||
info: - in stub files
|
||||
info: - in `if TYPE_CHECKING` blocks
|
||||
info: - as methods on protocol classes
|
||||
info: - or as `@abstractmethod`-decorated methods on abstract classes
|
||||
info: See https://docs.python.org/3/library/typing.html#typing.overload for more details
|
||||
info: rule `invalid-overload` is enabled by default
|
||||
|
||||
|
|
|
|||
|
|
@ -1815,13 +1815,6 @@ impl<'db> ClassLiteral<'db> {
|
|||
})
|
||||
}
|
||||
|
||||
/// Determine if this is an abstract class.
|
||||
pub(super) fn is_abstract(self, db: &'db dyn Db) -> bool {
|
||||
self.metaclass(db)
|
||||
.as_class_literal()
|
||||
.is_some_and(|metaclass| metaclass.is_known(db, KnownClass::ABCMeta))
|
||||
}
|
||||
|
||||
/// Return the types of the decorators on this class
|
||||
#[salsa::tracked(returns(deref), cycle_initial=decorators_cycle_initial, heap_size=ruff_memory_usage::heap_size)]
|
||||
fn decorators(self, db: &'db dyn Db) -> Box<[Type<'db>]> {
|
||||
|
|
|
|||
|
|
@ -1143,7 +1143,16 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
if implementation.is_none() && !self.in_stub() {
|
||||
let mut implementation_required = true;
|
||||
|
||||
if let NodeWithScopeKind::Class(class_node_ref) = scope {
|
||||
if function
|
||||
.iter_overloads_and_implementation(self.db())
|
||||
.all(|f| {
|
||||
f.body_scope(self.db())
|
||||
.scope(self.db())
|
||||
.in_type_checking_block()
|
||||
})
|
||||
{
|
||||
implementation_required = false;
|
||||
} else if let NodeWithScopeKind::Class(class_node_ref) = scope {
|
||||
let class = binding_type(
|
||||
self.db(),
|
||||
self.index
|
||||
|
|
@ -1152,7 +1161,8 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
.expect_class_literal();
|
||||
|
||||
if class.is_protocol(self.db())
|
||||
|| (class.is_abstract(self.db())
|
||||
|| (Type::ClassLiteral(class)
|
||||
.is_subtype_of(self.db(), KnownClass::ABCMeta.to_instance(self.db()))
|
||||
&& overloads.iter().all(|overload| {
|
||||
overload.has_known_decorator(
|
||||
self.db(),
|
||||
|
|
@ -1179,8 +1189,13 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
|||
&function_node.name
|
||||
));
|
||||
diagnostic.info(
|
||||
"Overloaded functions without implementations are only permitted \
|
||||
in stub files, on protocols, or for abstract methods",
|
||||
"Overloaded functions without implementations are only permitted:",
|
||||
);
|
||||
diagnostic.info(" - in stub files");
|
||||
diagnostic.info(" - in `if TYPE_CHECKING` blocks");
|
||||
diagnostic.info(" - as methods on protocol classes");
|
||||
diagnostic.info(
|
||||
" - or as `@abstractmethod`-decorated methods on abstract classes",
|
||||
);
|
||||
diagnostic.info(
|
||||
"See https://docs.python.org/3/library/typing.html#typing.overload \
|
||||
|
|
|
|||
Loading…
Reference in New Issue