mirror of https://github.com/astral-sh/ruff
[red-knot] Fix Stack overflow in Type::bool (#15843)
## Summary This PR adds `Type::call_bound` method for calls that should follow descriptor protocol calling convention. The PR is intentionally shallow in scope and only fixes #15672 Couple of obvious things that weren't done: * Switch to `call_bound` everywhere it should be used * Address the fact, that red_knot resolves `__bool__ = bool` as a Union, which includes `Type::Dynamic` and hence fails to infer that the truthiness is always false for such a class (I've added a todo comment in mdtests) * Doesn't try to invent a new type for descriptors, although I have a gut feeling it may be more convenient in the end, instead of doing method lookup each time like I did in `call_bound` ## Test Plan * extended mdtests with 2 examples from the issue * cargo neatest run
This commit is contained in:
parent
444b055cec
commit
e15419396c
|
|
@ -1,5 +1,7 @@
|
||||||
# Truthiness
|
# Truthiness
|
||||||
|
|
||||||
|
## Literals
|
||||||
|
|
||||||
```py
|
```py
|
||||||
from typing_extensions import Literal, LiteralString
|
from typing_extensions import Literal, LiteralString
|
||||||
from knot_extensions import AlwaysFalsy, AlwaysTruthy
|
from knot_extensions import AlwaysFalsy, AlwaysTruthy
|
||||||
|
|
@ -45,3 +47,31 @@ def _(
|
||||||
reveal_type(bool(c)) # revealed: bool
|
reveal_type(bool(c)) # revealed: bool
|
||||||
reveal_type(bool(d)) # revealed: bool
|
reveal_type(bool(d)) # revealed: bool
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Instances
|
||||||
|
|
||||||
|
Checks that we don't get into a cycle if someone sets their `__bool__` method to the `bool` builtin:
|
||||||
|
|
||||||
|
### __bool__ is bool
|
||||||
|
|
||||||
|
```py
|
||||||
|
class BoolIsBool:
|
||||||
|
__bool__ = bool
|
||||||
|
|
||||||
|
reveal_type(bool(BoolIsBool())) # revealed: bool
|
||||||
|
```
|
||||||
|
|
||||||
|
### Conditional __bool__ method
|
||||||
|
|
||||||
|
```py
|
||||||
|
def flag() -> bool:
|
||||||
|
return True
|
||||||
|
|
||||||
|
class Boom:
|
||||||
|
if flag():
|
||||||
|
__bool__ = bool
|
||||||
|
else:
|
||||||
|
__bool__ = int
|
||||||
|
|
||||||
|
reveal_type(bool(Boom())) # revealed: bool
|
||||||
|
```
|
||||||
|
|
|
||||||
|
|
@ -141,15 +141,6 @@ class AlwaysFalse:
|
||||||
# revealed: Literal[True]
|
# revealed: Literal[True]
|
||||||
reveal_type(not AlwaysFalse())
|
reveal_type(not AlwaysFalse())
|
||||||
|
|
||||||
# We don't get into a cycle if someone sets their `__bool__` method to the `bool` builtin:
|
|
||||||
class BoolIsBool:
|
|
||||||
# TODO: The `type[bool]` declaration here is a workaround to avoid running into
|
|
||||||
# https://github.com/astral-sh/ruff/issues/15672
|
|
||||||
__bool__: type[bool] = bool
|
|
||||||
|
|
||||||
# revealed: bool
|
|
||||||
reveal_type(not BoolIsBool())
|
|
||||||
|
|
||||||
# At runtime, no `__bool__` and no `__len__` means truthy, but we can't rely on that, because
|
# At runtime, no `__bool__` and no `__len__` means truthy, but we can't rely on that, because
|
||||||
# a subclass could add a `__bool__` method.
|
# a subclass could add a `__bool__` method.
|
||||||
class NoBoolMethod: ...
|
class NoBoolMethod: ...
|
||||||
|
|
|
||||||
|
|
@ -670,6 +670,10 @@ impl<'db> Type<'db> {
|
||||||
matches!(self, Type::ClassLiteral(..))
|
matches!(self, Type::ClassLiteral(..))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub const fn is_instance(&self) -> bool {
|
||||||
|
matches!(self, Type::Instance(..))
|
||||||
|
}
|
||||||
|
|
||||||
pub fn module_literal(db: &'db dyn Db, importing_file: File, submodule: Module) -> Self {
|
pub fn module_literal(db: &'db dyn Db, importing_file: File, submodule: Module) -> Self {
|
||||||
Self::ModuleLiteral(ModuleLiteralType::new(db, importing_file, submodule))
|
Self::ModuleLiteral(ModuleLiteralType::new(db, importing_file, submodule))
|
||||||
}
|
}
|
||||||
|
|
@ -1843,19 +1847,8 @@ impl<'db> Type<'db> {
|
||||||
return Truthiness::Ambiguous;
|
return Truthiness::Ambiguous;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Check if the class has `__bool__ = bool` and avoid infinite recursion, since
|
|
||||||
// `Type::call` on `bool` will call `Type::bool` on the argument.
|
|
||||||
if bool_method
|
|
||||||
.into_class_literal()
|
|
||||||
.is_some_and(|ClassLiteralType { class }| {
|
|
||||||
class.is_known(db, KnownClass::Bool)
|
|
||||||
})
|
|
||||||
{
|
|
||||||
return Truthiness::Ambiguous;
|
|
||||||
}
|
|
||||||
|
|
||||||
if let Some(Type::BooleanLiteral(bool_val)) = bool_method
|
if let Some(Type::BooleanLiteral(bool_val)) = bool_method
|
||||||
.call(db, &CallArguments::positional([*instance_ty]))
|
.call_bound(db, instance_ty, &CallArguments::positional([]))
|
||||||
.return_type(db)
|
.return_type(db)
|
||||||
{
|
{
|
||||||
bool_val.into()
|
bool_val.into()
|
||||||
|
|
@ -2148,6 +2141,52 @@ impl<'db> Type<'db> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Return the outcome of calling an class/instance attribute of this type
|
||||||
|
/// using descriptor protocol.
|
||||||
|
///
|
||||||
|
/// `receiver_ty` must be `Type::Instance(_)` or `Type::ClassLiteral`.
|
||||||
|
///
|
||||||
|
/// TODO: handle `super()` objects properly
|
||||||
|
#[must_use]
|
||||||
|
fn call_bound(
|
||||||
|
self,
|
||||||
|
db: &'db dyn Db,
|
||||||
|
receiver_ty: &Type<'db>,
|
||||||
|
arguments: &CallArguments<'_, 'db>,
|
||||||
|
) -> CallOutcome<'db> {
|
||||||
|
debug_assert!(receiver_ty.is_instance() || receiver_ty.is_class_literal());
|
||||||
|
|
||||||
|
match self {
|
||||||
|
Type::FunctionLiteral(..) => {
|
||||||
|
// Functions are always descriptors, so this would effectively call
|
||||||
|
// the function with the instance as the first argument
|
||||||
|
self.call(db, &arguments.with_self(*receiver_ty))
|
||||||
|
}
|
||||||
|
|
||||||
|
Type::Instance(_) | Type::ClassLiteral(_) => {
|
||||||
|
// TODO descriptor protocol. For now, assume non-descriptor and call without `self` argument.
|
||||||
|
self.call(db, arguments)
|
||||||
|
}
|
||||||
|
|
||||||
|
Type::Union(union) => CallOutcome::union(
|
||||||
|
self,
|
||||||
|
union
|
||||||
|
.elements(db)
|
||||||
|
.iter()
|
||||||
|
.map(|elem| elem.call_bound(db, receiver_ty, arguments)),
|
||||||
|
),
|
||||||
|
|
||||||
|
Type::Intersection(_) => CallOutcome::callable(CallBinding::from_return_type(
|
||||||
|
todo_type!("Type::Intersection.call_bound()"),
|
||||||
|
)),
|
||||||
|
|
||||||
|
// Cases that duplicate, and thus must be kept in sync with, `Type::call()`
|
||||||
|
Type::Dynamic(_) => CallOutcome::callable(CallBinding::from_return_type(self)),
|
||||||
|
|
||||||
|
_ => CallOutcome::not_callable(self),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Look up a dunder method on the meta type of `self` and call it.
|
/// Look up a dunder method on the meta type of `self` and call it.
|
||||||
fn call_dunder(
|
fn call_dunder(
|
||||||
self,
|
self,
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue