mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-22 08:12:17 +00:00
[ty] Treat Callable
s as bound-method descriptors in special cases (#20802)
## Summary Treat `Callable`s as bound-method descriptors if `Callable` is the return type of a decorator that is applied to a function definition. See the [rendered version of the new test file](https://github.com/astral-sh/ruff/blob/david/callables-as-descriptors/crates/ty_python_semantic/resources/mdtest/call/callables_as_descriptors.md) for the full description of this new heuristic. I could imagine that we want to treat `Callable`s as bound-method descriptors in other cases as well, but this seems like a step in the right direction. I am planning to add other "use cases" from https://github.com/astral-sh/ty/issues/491 to this test suite. partially addresses https://github.com/astral-sh/ty/issues/491 closes https://github.com/astral-sh/ty/issues/1333 ## Ecosystem impact All positive * 2961 removed `unsupported-operator` diagnostics on `sympy`, which was one of the main motivations for implementing this change * 37 removed `missing-argument` diagnostics, and no added call-error diagnostics, which is an indicator that this heuristic shouldn't cause many false positives * A few removed `possibly-missing-attribute` diagnostics when accessing attributes like `__name__` on decorated functions. The two added `unused-ignore-comment` diagnostics are also cases of this. * One new `invalid-assignment` diagnostic on `dd-trace-py`, which looks suspicious, but only because our `invalid-assignment` diagnostics are not great. This is actually a "Implicit shadowing of function" diagnostic that hides behind the `invalid-assignment` diagnostic, because a module-global function is being patched through a `module.func` attribute assignment. ## Test Plan New Markdown tests.
This commit is contained in:
parent
d912f13661
commit
4b8e278a88
3 changed files with 217 additions and 1 deletions
|
@ -0,0 +1,190 @@
|
||||||
|
# Callables as descriptors?
|
||||||
|
|
||||||
|
```toml
|
||||||
|
[environment]
|
||||||
|
python-version = "3.14"
|
||||||
|
```
|
||||||
|
|
||||||
|
## Introduction
|
||||||
|
|
||||||
|
Some common callable objects (all functions, including lambdas) are also bound-method descriptors.
|
||||||
|
That is, they have a `__get__` method which returns a bound-method object that binds the receiver
|
||||||
|
instance to the first argument. The bound-method object therefore has a different signature, lacking
|
||||||
|
the first argument:
|
||||||
|
|
||||||
|
```py
|
||||||
|
from ty_extensions import CallableTypeOf
|
||||||
|
from typing import Callable
|
||||||
|
|
||||||
|
class C1:
|
||||||
|
def method(self: C1, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
|
||||||
|
def _(
|
||||||
|
accessed_on_class: CallableTypeOf[C1.method],
|
||||||
|
accessed_on_instance: CallableTypeOf[C1().method],
|
||||||
|
):
|
||||||
|
reveal_type(accessed_on_class) # revealed: (self: C1, x: int) -> str
|
||||||
|
reveal_type(accessed_on_instance) # revealed: (x: int) -> str
|
||||||
|
```
|
||||||
|
|
||||||
|
Other callable objects (`staticmethod` objects, instances of classes with a `__call__` method but no
|
||||||
|
dedicated `__get__` method) are *not* bound-method descriptors. If accessed as class attributes via
|
||||||
|
an instance, they are simply themselves:
|
||||||
|
|
||||||
|
```py
|
||||||
|
class NonDescriptorCallable2:
|
||||||
|
def __call__(self, c2: C2, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
|
||||||
|
class C2:
|
||||||
|
non_descriptor_callable: NonDescriptorCallable2 = NonDescriptorCallable2()
|
||||||
|
|
||||||
|
def _(
|
||||||
|
accessed_on_class: CallableTypeOf[C2.non_descriptor_callable],
|
||||||
|
accessed_on_instance: CallableTypeOf[C2().non_descriptor_callable],
|
||||||
|
):
|
||||||
|
reveal_type(accessed_on_class) # revealed: (c2: C2, x: int) -> str
|
||||||
|
reveal_type(accessed_on_instance) # revealed: (c2: C2, x: int) -> str
|
||||||
|
```
|
||||||
|
|
||||||
|
Both kinds of objects can inhabit the same `Callable` type:
|
||||||
|
|
||||||
|
```py
|
||||||
|
class NonDescriptorCallable3:
|
||||||
|
def __call__(self, c3: C3, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
|
||||||
|
class C3:
|
||||||
|
def method(self: C3, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
non_descriptor_callable: NonDescriptorCallable3 = NonDescriptorCallable3()
|
||||||
|
|
||||||
|
callable_m: Callable[[C3, int], str] = method
|
||||||
|
callable_n: Callable[[C3, int], str] = non_descriptor_callable
|
||||||
|
```
|
||||||
|
|
||||||
|
However, when they are accessed on instances of `C3`, they have different signatures:
|
||||||
|
|
||||||
|
```py
|
||||||
|
def _(
|
||||||
|
method_accessed_on_instance: CallableTypeOf[C3().method],
|
||||||
|
callable_accessed_on_instance: CallableTypeOf[C3().non_descriptor_callable],
|
||||||
|
):
|
||||||
|
reveal_type(method_accessed_on_instance) # revealed: (x: int) -> str
|
||||||
|
reveal_type(callable_accessed_on_instance) # revealed: (c3: C3, x: int) -> str
|
||||||
|
```
|
||||||
|
|
||||||
|
This leaves the question how the `callable_m` and `callable_n` attributes should be treated when
|
||||||
|
accessed on instances of `C3`. If we treat `Callable` as being equivalent to a protocol that defines
|
||||||
|
a `__call__` method (and no `__get__` method), then they should show no bound-method behavior. This
|
||||||
|
is what we currently do:
|
||||||
|
|
||||||
|
```py
|
||||||
|
reveal_type(C3().callable_m) # revealed: (C3, int, /) -> str
|
||||||
|
reveal_type(C3().callable_n) # revealed: (C3, int, /) -> str
|
||||||
|
```
|
||||||
|
|
||||||
|
However, this leads to unsoundness: `C3().callable_m` is actually `C3.method` which *is* a
|
||||||
|
bound-method descriptor. We currently allow the following call, which will fail at runtime:
|
||||||
|
|
||||||
|
```py
|
||||||
|
C3().callable_m(C3(), 1) # runtime error! ("takes 2 positional arguments but 3 were given")
|
||||||
|
```
|
||||||
|
|
||||||
|
If we were to treat `Callable`s as bound-method descriptors, then the signatures of `callable_m` and
|
||||||
|
`callable_n` when accessed on instances would bind the `self` argument:
|
||||||
|
|
||||||
|
- `C3().callable_m`: `(x: int) -> str`
|
||||||
|
- `C3().callable_n`: `(x: int) -> str`
|
||||||
|
|
||||||
|
This would be equally unsound, because now we would allow a call to `C3().callable_n(1)` which would
|
||||||
|
also fail at runtime.
|
||||||
|
|
||||||
|
There is no perfect solution here, but we can use some heuristics to improve the situation for
|
||||||
|
certain use cases (at the cost of purity and simplicity).
|
||||||
|
|
||||||
|
## Use case: Decorating a method with a `Callable`-typed decorator
|
||||||
|
|
||||||
|
A commonly used pattern in the ecosystem is to use a `Callable`-typed decorator on a method with the
|
||||||
|
intention that it shouldn't influence the method's descriptor behavior. For example, we treat
|
||||||
|
`method_decorated` below as a bound method, even though its type is `Callable[[C1, int], str]`:
|
||||||
|
|
||||||
|
```py
|
||||||
|
from typing import Callable
|
||||||
|
|
||||||
|
# TODO: this could use a generic signature, but we don't support
|
||||||
|
# `ParamSpec` and solving of typevars inside `Callable` types yet.
|
||||||
|
def memoize(f: Callable[[C1, int], str]) -> Callable[[C1, int], str]:
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
class C1:
|
||||||
|
def method(self, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
|
||||||
|
@memoize
|
||||||
|
def method_decorated(self, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
|
||||||
|
C1().method(1)
|
||||||
|
|
||||||
|
C1().method_decorated(1)
|
||||||
|
```
|
||||||
|
|
||||||
|
This also works with an argumentless `Callable` annotation:
|
||||||
|
|
||||||
|
```py
|
||||||
|
def memoize2(f: Callable) -> Callable:
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
class C2:
|
||||||
|
@memoize2
|
||||||
|
def method_decorated(self, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
|
||||||
|
C2().method_decorated(1)
|
||||||
|
```
|
||||||
|
|
||||||
|
Note that we currently only apply this heuristic when calling a function such as `memoize` via the
|
||||||
|
decorator syntax. This is inconsistent, because the above *should* be equivalent to the following,
|
||||||
|
but here we emit errors:
|
||||||
|
|
||||||
|
```py
|
||||||
|
def memoize3(f: Callable[[C3, int], str]) -> Callable[[C3, int], str]:
|
||||||
|
raise NotImplementedError
|
||||||
|
|
||||||
|
class C3:
|
||||||
|
def method(self, x: int) -> str:
|
||||||
|
return str(x)
|
||||||
|
method_decorated = memoize3(method)
|
||||||
|
|
||||||
|
# error: [missing-argument]
|
||||||
|
# error: [invalid-argument-type]
|
||||||
|
C3().method_decorated(1)
|
||||||
|
```
|
||||||
|
|
||||||
|
The reason for this is that the heuristic is problematic. We don't *know* that the `Callable` in the
|
||||||
|
return type of `memoize` is actually related to the method that we pass in. But when `memoize` is
|
||||||
|
applied as a decorator, it is reasonable to assume so.
|
||||||
|
|
||||||
|
In general, a function call might however return a `Callable` that is unrelated to the argument
|
||||||
|
passed in. And here, it seems more reasonable and safe to treat the `Callable` as a non-descriptor.
|
||||||
|
This allows correct programs like the following to pass type checking (that are currently rejected
|
||||||
|
by pyright and mypy with a heuristic that apparently applies in a wider range of situations):
|
||||||
|
|
||||||
|
```py
|
||||||
|
class SquareCalculator:
|
||||||
|
def __init__(self, post_process: Callable[[float], int]):
|
||||||
|
self.post_process = post_process
|
||||||
|
|
||||||
|
def __call__(self, x: float) -> int:
|
||||||
|
return self.post_process(x * x)
|
||||||
|
|
||||||
|
def square_then(c: Callable[[float], int]) -> Callable[[float], int]:
|
||||||
|
return SquareCalculator(c)
|
||||||
|
|
||||||
|
class Calculator:
|
||||||
|
square_then_round = square_then(round)
|
||||||
|
|
||||||
|
reveal_type(Calculator().square_then_round(3.14)) # revealed: Unknown | int
|
||||||
|
```
|
|
@ -1014,6 +1014,13 @@ impl<'db> Type<'db> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(crate) const fn unwrap_as_callable_type(self) -> Option<CallableType<'db>> {
|
||||||
|
match self {
|
||||||
|
Type::Callable(callable_type) => Some(callable_type),
|
||||||
|
_ => None,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) const fn expect_dynamic(self) -> DynamicType<'db> {
|
pub(crate) const fn expect_dynamic(self) -> DynamicType<'db> {
|
||||||
self.into_dynamic()
|
self.into_dynamic()
|
||||||
.expect("Expected a Type::Dynamic variant")
|
.expect("Expected a Type::Dynamic variant")
|
||||||
|
|
|
@ -2175,7 +2175,26 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
|
||||||
.try_call(self.db(), &CallArguments::positional([inferred_ty]))
|
.try_call(self.db(), &CallArguments::positional([inferred_ty]))
|
||||||
.map(|bindings| bindings.return_type(self.db()))
|
.map(|bindings| bindings.return_type(self.db()))
|
||||||
{
|
{
|
||||||
Ok(return_ty) => return_ty,
|
Ok(return_ty) => {
|
||||||
|
let is_input_function_like = inferred_ty
|
||||||
|
.into_callable(self.db())
|
||||||
|
.and_then(Type::unwrap_as_callable_type)
|
||||||
|
.is_some_and(|callable| callable.is_function_like(self.db()));
|
||||||
|
if is_input_function_like
|
||||||
|
&& let Some(callable_type) = return_ty.unwrap_as_callable_type()
|
||||||
|
{
|
||||||
|
// When a method on a class is decorated with a function that returns a `Callable`, assume that
|
||||||
|
// the returned callable is also function-like. See "Decorating a method with a `Callable`-typed
|
||||||
|
// decorator" in `callables_as_descriptors.md` for the extended explanation.
|
||||||
|
Type::Callable(CallableType::new(
|
||||||
|
self.db(),
|
||||||
|
callable_type.signatures(self.db()),
|
||||||
|
true,
|
||||||
|
))
|
||||||
|
} else {
|
||||||
|
return_ty
|
||||||
|
}
|
||||||
|
}
|
||||||
Err(CallError(_, bindings)) => {
|
Err(CallError(_, bindings)) => {
|
||||||
bindings.report_diagnostics(&self.context, (*decorator_node).into());
|
bindings.report_diagnostics(&self.context, (*decorator_node).into());
|
||||||
bindings.return_type(self.db())
|
bindings.return_type(self.db())
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue