Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[red-knot] Lookup of `__new__` (#17733) 

## Summary

Model the lookup of `__new__` without going through
`Type::try_call_dunder`. The `__new__` method is only looked up on the
constructed type itself, not on the meta-type.

This now removes ~930 false positives across the ecosystem (vs 255 for
https://github.com/astral-sh/ruff/pull/17662). It introduces 30 new
false positives related to the construction of enums via something like
`Color = enum.Enum("Color", ["RED", "GREEN"])`. This is expected,
because we don't handle custom metaclass `__call__` methods. The fact
that we previously didn't emit diagnostics there was a coincidence (we
incorrectly called `EnumMeta.__new__`, and since we don't fully
understand its signature, that happened to work with `str`, `list`
arguments).

closes #17462

## Test Plan

Regression test
This commit is contained in:
David Peter 2025-04-30 17:27:09 +02:00 committed by GitHub
parent 7568eeb7a5
commit 18bac94226
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 163 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

68
crates/red_knot_python_semantic/resources/mdtest/call/constructor.md
View File

@ -1,25 +1,25 @@
# Constructor # Constructor
When classes are instantiated, Python calls the meta-class `__call__` method, which can either be When classes are instantiated, Python calls the metaclass's `__call__` method. The metaclass of most
customized by the user or `type.__call__` is used. Python classes is the class `builtins.type`.
The latter calls the `__new__` method of the class, which is responsible for creating the instance `type.__call__` calls the `__new__` method of the class, which is responsible for creating the
and then calls the `__init__` method on the resulting instance to initialize it with the same instance. `__init__` is then called on the constructed instance with the same arguments that were
arguments. passed to `__new__`.
Both `__new__` and `__init__` are looked up using full descriptor protocol, but `__new__` is then Both `__new__` and `__init__` are looked up using the descriptor protocol, i.e., `__get__` is called
called as an implicit static, rather than bound method with `cls` passed as the first argument. if these attributes are descriptors. `__new__` is always treated as a static method, i.e., `cls` is
`__init__` has no special handling, it is fetched as bound method and is called just like any other passed as the first argument. `__init__` has no special handling; it is fetched as a bound method
dunder method. and called just like any other dunder method.
`type.__call__` does other things too, but this is not yet handled by us. `type.__call__` does other things too, but this is not yet handled by us.
Since every class has `object` in it's MRO, the default implementations are `object.__new__` and Since every class has `object` in it's MRO, the default implementations are `object.__new__` and
`object.__init__`. They have some special behavior, namely: `object.__init__`. They have some special behavior, namely:
- If neither `__new__` nor `__init__` are defined anywhere in the MRO of class (except for `object`) - If neither `__new__` nor `__init__` are defined anywhere in the MRO of class (except for
\- no arguments are accepted and `TypeError` is raised if any are passed. `object`), no arguments are accepted and `TypeError` is raised if any are passed.
- If `__new__` is defined, but `__init__` is not - `object.__init__` will allow arbitrary arguments! - If `__new__` is defined but `__init__` is not, `object.__init__` will allow arbitrary arguments!
As of today there are a number of behaviors that we do not support: As of today there are a number of behaviors that we do not support:
@ -146,6 +146,25 @@ reveal_type(Foo()) # revealed: Foo
### Possibly Unbound ### Possibly Unbound
#### Possibly unbound `__new__` method
```py
def _(flag: bool) -> None:
class Foo:
if flag:
def __new__(cls):
return object.__new__(cls)
# error: [call-possibly-unbound-method]
reveal_type(Foo()) # revealed: Foo
# error: [call-possibly-unbound-method]
# error: [too-many-positional-arguments]
reveal_type(Foo(1)) # revealed: Foo
```
#### Possibly unbound `__call__` on `__new__` callable
```py ```py
def _(flag: bool) -> None: def _(flag: bool) -> None:
class Callable: class Callable:
@ -323,3 +342,28 @@ reveal_type(Foo(1)) # revealed: Foo
# error: [too-many-positional-arguments] "Too many positional arguments to bound method `__init__`: expected 1, got 2" # error: [too-many-positional-arguments] "Too many positional arguments to bound method `__init__`: expected 1, got 2"
reveal_type(Foo(1, 2)) # revealed: Foo reveal_type(Foo(1, 2)) # revealed: Foo
``` ```
### Lookup of `__new__`
The `__new__` method is always invoked on the class itself, never on the metaclass. This is
different from how other dunder methods like `__lt__` are implicitly called (always on the
meta-type, never on the type itself).
```py
from typing_extensions import Literal
class Meta(type):
def __new__(mcls, name, bases, namespace, /, **kwargs):
return super().__new__(mcls, name, bases, namespace)
def __lt__(cls, other) -> Literal[True]:
return True
class C(metaclass=Meta): ...
# No error is raised here, since we don't implicitly call `Meta.__new__`
reveal_type(C()) # revealed: C
# Meta.__lt__ is implicitly called here:
reveal_type(C < C) # revealed: Literal[True]
```

28
crates/red_knot_python_semantic/src/symbol.rs
View File

@ -107,6 +107,34 @@ impl<'db> Symbol<'db> {
qualifiers, qualifiers,
} }
} }
/// Try to call `__get__(None, owner)` on the type of this symbol (not on the meta type).
/// If it succeeds, return the `__get__` return type. Otherwise, returns the original symbol.
/// This is used to resolve (potential) descriptor attributes.
pub(crate) fn try_call_dunder_get(self, db: &'db dyn Db, owner: Type<'db>) -> Symbol<'db> {
match self {
Symbol::Type(Type::Union(union), boundness) => union.map_with_boundness(db, |elem| {
Symbol::Type(*elem, boundness).try_call_dunder_get(db, owner)
}),
Symbol::Type(Type::Intersection(intersection), boundness) => intersection
.map_with_boundness(db, |elem| {
Symbol::Type(*elem, boundness).try_call_dunder_get(db, owner)
}),
Symbol::Type(self_ty, boundness) => {
if let Some((dunder_get_return_ty, _)) =
self_ty.try_call_dunder_get(db, Type::none(db), owner)
{
Symbol::Type(dunder_get_return_ty, boundness)
} else {
self
}
}
Symbol::Unbound => Symbol::Unbound,
}
}
} }
impl<'db> From<LookupResult<'db>> for SymbolAndQualifiers<'db> { impl<'db> From<LookupResult<'db>> for SymbolAndQualifiers<'db> {

77
crates/red_knot_python_semantic/src/types.rs
View File

@ -155,7 +155,7 @@ fn definition_expression_type<'db>(
/// method or a `__delete__` method. This enum is used to categorize attributes into two /// method or a `__delete__` method. This enum is used to categorize attributes into two
/// groups: (1) data descriptors and (2) normal attributes or non-data descriptors. /// groups: (1) data descriptors and (2) normal attributes or non-data descriptors.
#[derive(Clone, Debug, Copy, PartialEq, Eq, Hash, salsa::Update)] #[derive(Clone, Debug, Copy, PartialEq, Eq, Hash, salsa::Update)]
enum AttributeKind { pub(crate) enum AttributeKind {
DataDescriptor, DataDescriptor,
NormalOrNonDataDescriptor, NormalOrNonDataDescriptor,
} }
@ -2627,7 +2627,7 @@ impl<'db> Type<'db> {
/// ///
/// If `__get__` is not defined on the meta-type, this method returns `None`. /// If `__get__` is not defined on the meta-type, this method returns `None`.
#[salsa::tracked] #[salsa::tracked]
fn try_call_dunder_get( pub(crate) fn try_call_dunder_get(
self, self,
db: &'db dyn Db, db: &'db dyn Db,
instance: Type<'db>, instance: Type<'db>,
@ -2643,7 +2643,10 @@ impl<'db> Type<'db> {
if let Symbol::Type(descr_get, descr_get_boundness) = descr_get { if let Symbol::Type(descr_get, descr_get_boundness) = descr_get {
let return_ty = descr_get let return_ty = descr_get
.try_call(db, CallArgumentTypes::positional([self, instance, owner])) .try_call(
db,
&mut CallArgumentTypes::positional([self, instance, owner]),
)
.map(|bindings| { .map(|bindings| {
if descr_get_boundness == Boundness::Bound { if descr_get_boundness == Boundness::Bound {
bindings.return_type(db) bindings.return_type(db)
@ -4198,11 +4201,10 @@ impl<'db> Type<'db> {
fn try_call( fn try_call(
self, self,
db: &'db dyn Db, db: &'db dyn Db,
mut argument_types: CallArgumentTypes<'_, 'db>, argument_types: &mut CallArgumentTypes<'_, 'db>,
) -> Result<Bindings<'db>, CallError<'db>> { ) -> Result<Bindings<'db>, CallError<'db>> {
let signatures = self.signatures(db); let signatures = self.signatures(db);
Bindings::match_parameters(signatures, &mut argument_types) Bindings::match_parameters(signatures, argument_types).check_types(db, argument_types)
.check_types(db, &mut argument_types)
} }
/// 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.
@ -4466,16 +4468,27 @@ impl<'db> Type<'db> {
// easy to check if that's the one we found? // easy to check if that's the one we found?
// Note that `__new__` is a static method, so we must inject the `cls` argument. // Note that `__new__` is a static method, so we must inject the `cls` argument.
let new_call_outcome = argument_types.with_self(Some(self_type), |argument_types| { let new_call_outcome = argument_types.with_self(Some(self_type), |argument_types| {
let result = self_type.try_call_dunder_with_policy( let new_method = self_type
.find_name_in_mro_with_policy(
db, db,
"__new__", "__new__",
argument_types,
MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
| MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK, | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
); )?
match result { .symbol
Err(CallDunderError::MethodNotAvailable) => None, .try_call_dunder_get(db, self_type);
_ => Some(result),
match new_method {
Symbol::Type(new_method, boundness) => {
let result = new_method.try_call(db, argument_types);
if boundness == Boundness::PossiblyUnbound {
return Some(Err(DunderNewCallError::PossiblyUnbound(result.err())));
}
Some(result.map_err(DunderNewCallError::CallError))
}
Symbol::Unbound => None,
} }
}); });
@ -6265,12 +6278,23 @@ impl<'db> BoolError<'db> {
} }
} }
/// Represents possibly failure modes of implicit `__new__` calls.
#[derive(Debug)]
enum DunderNewCallError<'db> {
/// The call to `__new__` failed.
CallError(CallError<'db>),
/// The `__new__` method could be unbound. If the call to the
/// method has also failed, this variant also includes the
/// corresponding `CallError`.
PossiblyUnbound(Option<CallError<'db>>),
}
/// Error returned if a class instantiation call failed /// Error returned if a class instantiation call failed
#[derive(Debug)] #[derive(Debug)]
enum ConstructorCallError<'db> { enum ConstructorCallError<'db> {
Init(Type<'db>, CallDunderError<'db>), Init(Type<'db>, CallDunderError<'db>),
New(Type<'db>, CallDunderError<'db>), New(Type<'db>, DunderNewCallError<'db>),
NewAndInit(Type<'db>, CallDunderError<'db>, CallDunderError<'db>), NewAndInit(Type<'db>, DunderNewCallError<'db>, CallDunderError<'db>),
} }
impl<'db> ConstructorCallError<'db> { impl<'db> ConstructorCallError<'db> {
@ -6320,13 +6344,8 @@ impl<'db> ConstructorCallError<'db> {
} }
}; };
let report_new_error = |call_dunder_error: &CallDunderError<'db>| match call_dunder_error { let report_new_error = |error: &DunderNewCallError<'db>| match error {
CallDunderError::MethodNotAvailable => { DunderNewCallError::PossiblyUnbound(call_error) => {
// We are explicitly checking for `__new__` before attempting to call it,
// so this should never happen.
unreachable!("`__new__` method may not be called if missing");
}
CallDunderError::PossiblyUnbound(bindings) => {
if let Some(builder) = if let Some(builder) =
context.report_lint(&CALL_POSSIBLY_UNBOUND_METHOD, context_expression_node) context.report_lint(&CALL_POSSIBLY_UNBOUND_METHOD, context_expression_node)
{ {
@ -6336,22 +6355,24 @@ impl<'db> ConstructorCallError<'db> {
)); ));
} }
if let Some(CallError(_kind, bindings)) = call_error {
bindings.report_diagnostics(context, context_expression_node); bindings.report_diagnostics(context, context_expression_node);
} }
CallDunderError::CallError(_, bindings) => { }
DunderNewCallError::CallError(CallError(_kind, bindings)) => {
bindings.report_diagnostics(context, context_expression_node); bindings.report_diagnostics(context, context_expression_node);
} }
}; };
match self { match self {
Self::Init(_, call_dunder_error) => { Self::Init(_, init_call_dunder_error) => {
report_init_error(call_dunder_error); report_init_error(init_call_dunder_error);
} }
Self::New(_, call_dunder_error) => { Self::New(_, new_call_error) => {
report_new_error(call_dunder_error); report_new_error(new_call_error);
} }
Self::NewAndInit(_, new_call_dunder_error, init_call_dunder_error) => { Self::NewAndInit(_, new_call_error, init_call_dunder_error) => {
report_new_error(new_call_dunder_error); report_new_error(new_call_error);
report_init_error(init_call_dunder_error); report_init_error(init_call_dunder_error);
} }
} }

18
crates/red_knot_python_semantic/src/types/call/bind.rs
View File

@ -346,7 +346,7 @@ impl<'db> Bindings<'db> {
[Some(Type::PropertyInstance(property)), Some(instance), ..] => { [Some(Type::PropertyInstance(property)), Some(instance), ..] => {
if let Some(getter) = property.getter(db) { if let Some(getter) = property.getter(db) {
if let Ok(return_ty) = getter if let Ok(return_ty) = getter
.try_call(db, CallArgumentTypes::positional([*instance])) .try_call(db, &mut CallArgumentTypes::positional([*instance]))
.map(|binding| binding.return_type(db)) .map(|binding| binding.return_type(db))
{ {
overload.set_return_type(return_ty); overload.set_return_type(return_ty);
@ -374,7 +374,7 @@ impl<'db> Bindings<'db> {
[Some(instance), ..] => { [Some(instance), ..] => {
if let Some(getter) = property.getter(db) { if let Some(getter) = property.getter(db) {
if let Ok(return_ty) = getter if let Ok(return_ty) = getter
.try_call(db, CallArgumentTypes::positional([*instance])) .try_call(db, &mut CallArgumentTypes::positional([*instance]))
.map(|binding| binding.return_type(db)) .map(|binding| binding.return_type(db))
{ {
overload.set_return_type(return_ty); overload.set_return_type(return_ty);
@ -400,9 +400,10 @@ impl<'db> Bindings<'db> {
overload.parameter_types() overload.parameter_types()
{ {
if let Some(setter) = property.setter(db) { if let Some(setter) = property.setter(db) {
if let Err(_call_error) = setter if let Err(_call_error) = setter.try_call(
.try_call(db, CallArgumentTypes::positional([*instance, *value])) db,
{ &mut CallArgumentTypes::positional([*instance, *value]),
) {
overload.errors.push(BindingError::InternalCallError( overload.errors.push(BindingError::InternalCallError(
"calling the setter failed", "calling the setter failed",
)); ));
@ -418,9 +419,10 @@ impl<'db> Bindings<'db> {
Type::MethodWrapper(MethodWrapperKind::PropertyDunderSet(property)) => { Type::MethodWrapper(MethodWrapperKind::PropertyDunderSet(property)) => {
if let [Some(instance), Some(value), ..] = overload.parameter_types() { if let [Some(instance), Some(value), ..] = overload.parameter_types() {
if let Some(setter) = property.setter(db) { if let Some(setter) = property.setter(db) {
if let Err(_call_error) = setter if let Err(_call_error) = setter.try_call(
.try_call(db, CallArgumentTypes::positional([*instance, *value])) db,
{ &mut CallArgumentTypes::positional([*instance, *value]),
) {
overload.errors.push(BindingError::InternalCallError( overload.errors.push(BindingError::InternalCallError(
"calling the setter failed", "calling the setter failed",
)); ));

28
crates/red_knot_python_semantic/src/types/class.rs
View File

@ -732,9 +732,9 @@ impl<'db> ClassLiteral<'db> {
let namespace = KnownClass::Dict.to_instance(db); let namespace = KnownClass::Dict.to_instance(db);
// TODO: Other keyword arguments? // TODO: Other keyword arguments?
let arguments = CallArgumentTypes::positional([name, bases, namespace]); let mut arguments = CallArgumentTypes::positional([name, bases, namespace]);
let return_ty_result = match metaclass.try_call(db, arguments) { let return_ty_result = match metaclass.try_call(db, &mut arguments) {
Ok(bindings) => Ok(bindings.return_type(db)), Ok(bindings) => Ok(bindings.return_type(db)),
Err(CallError(CallErrorKind::NotCallable, bindings)) => Err(MetaclassError { Err(CallError(CallErrorKind::NotCallable, bindings)) => Err(MetaclassError {
@ -817,17 +817,14 @@ impl<'db> ClassLiteral<'db> {
return Some(metaclass_call_function.into_callable_type(db)); return Some(metaclass_call_function.into_callable_type(db));
} }
let new_function_symbol = self_ty let dunder_new_method = self_ty
.member_lookup_with_policy( .find_name_in_mro(db, "__new__")
db, .expect("find_name_in_mro always succeeds for class literals")
"__new__".into(), .symbol
MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK .try_call_dunder_get(db, self_ty);
| MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
)
.symbol;
if let Symbol::Type(Type::FunctionLiteral(new_function), _) = new_function_symbol { if let Symbol::Type(Type::FunctionLiteral(dunder_new_method), _) = dunder_new_method {
return Some(new_function.into_bound_method_type(db, self.into())); return Some(dunder_new_method.into_bound_method_type(db, self.into()));
} }
// TODO handle `__init__` also // TODO handle `__init__` also
None None
@ -905,12 +902,7 @@ impl<'db> ClassLiteral<'db> {
continue; continue;
} }
// HACK: we should implement some more general logic here that supports arbitrary custom if class.is_known(db, KnownClass::Type) && policy.meta_class_no_type_fallback()
// metaclasses, not just `type` and `ABCMeta`.
if matches!(
class.known(db),
Some(KnownClass::Type | KnownClass::ABCMeta)
) && policy.meta_class_no_type_fallback()
{ {
continue; continue;
} }

10
crates/red_knot_python_semantic/src/types/infer.rs
View File

@ -1815,7 +1815,7 @@ impl<'db> TypeInferenceBuilder<'db> {
for (decorator_ty, decorator_node) in decorator_types_and_nodes.iter().rev() { for (decorator_ty, decorator_node) in decorator_types_and_nodes.iter().rev() {
inferred_ty = match decorator_ty inferred_ty = match decorator_ty
.try_call(self.db(), CallArgumentTypes::positional([inferred_ty])) .try_call(self.db(), &mut CallArgumentTypes::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) => return_ty,
@ -2832,7 +2832,7 @@ impl<'db> TypeInferenceBuilder<'db> {
let successful_call = meta_dunder_set let successful_call = meta_dunder_set
.try_call( .try_call(
db, db,
CallArgumentTypes::positional([ &mut CallArgumentTypes::positional([
meta_attr_ty, meta_attr_ty,
object_ty, object_ty,
value_ty, value_ty,
@ -2973,7 +2973,7 @@ impl<'db> TypeInferenceBuilder<'db> {
let successful_call = meta_dunder_set let successful_call = meta_dunder_set
.try_call( .try_call(
db, db,
CallArgumentTypes::positional([ &mut CallArgumentTypes::positional([
meta_attr_ty, meta_attr_ty,
object_ty, object_ty,
value_ty, value_ty,
@ -6454,7 +6454,7 @@ impl<'db> TypeInferenceBuilder<'db> {
Symbol::Type(contains_dunder, Boundness::Bound) => { Symbol::Type(contains_dunder, Boundness::Bound) => {
// If `__contains__` is available, it is used directly for the membership test. // If `__contains__` is available, it is used directly for the membership test.
contains_dunder contains_dunder
.try_call(db, CallArgumentTypes::positional([right, left])) .try_call(db, &mut CallArgumentTypes::positional([right, left]))
.map(|bindings| bindings.return_type(db)) .map(|bindings| bindings.return_type(db))
.ok() .ok()
} }
@ -6860,7 +6860,7 @@ impl<'db> TypeInferenceBuilder<'db> {
match ty.try_call( match ty.try_call(
self.db(), self.db(),
CallArgumentTypes::positional([value_ty, slice_ty]), &mut CallArgumentTypes::positional([value_ty, slice_ty]),
) { ) {
Ok(bindings) => return bindings.return_type(self.db()), Ok(bindings) => return bindings.return_type(self.db()),
Err(CallError(_, bindings)) => { Err(CallError(_, bindings)) => {