[red-knot] Lookup of __new__

crates/red_knot_python_semantic/resources/mdtest/call/constructor.md

@@ -1,25 +1,25 @@
 # Constructor
 
-When classes are instantiated, Python calls the meta-class `__call__` method, which can either be
-customized by the user or `type.__call__` is used.
+When classes are instantiated, Python calls the metaclass's `__call__` method. The metaclass of most
+Python classes is the class `builtins.type`.
 
-The latter calls the `__new__` method of the class, which is responsible for creating the instance
-and then calls the `__init__` method on the resulting instance to initialize it with the same
-arguments.
+`type.__call__` calls the `__new__` method of the class, which is responsible for creating the
+instance. `__init__` is then called on the constructed instance with the same arguments that were
+passed to `__new__`.
 
-Both `__new__` and `__init__` are looked up using full descriptor protocol, but `__new__` is then
-called as an implicit static, rather than bound method with `cls` passed as the first argument.
-`__init__` has no special handling, it is fetched as bound method and is called just like any other
-dunder method.
+Both `__new__` and `__init__` are looked up using the descriptor protocol, i.e., `__get__` is called
+if these attributes are descriptors. `__new__` is always treated as a static method, i.e., `cls` is
+passed as the first argument. `__init__` has no special handling; it is fetched as a bound method
+and called just like any other dunder method.
 
 `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
 `object.__init__`. They have some special behavior, namely:
 
-- If neither `__new__` nor `__init__` are defined anywhere in the MRO of class (except for `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 neither `__new__` nor `__init__` are defined anywhere in the MRO of class (except for
+    `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!
 
 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 `__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
 def _(flag: bool) -> None:
     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"
 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]
+```

crates/red_knot_python_semantic/src/symbol.rs

@@ -107,6 +107,34 @@ impl<'db> Symbol<'db> {
             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> {

crates/red_knot_python_semantic/src/types.rs

@@ -154,7 +154,7 @@ fn definition_expression_type<'db>(
 /// 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.
 #[derive(Clone, Debug, Copy, PartialEq, Eq, Hash, salsa::Update)]
-enum AttributeKind {
+pub(crate) enum AttributeKind {
     DataDescriptor,
     NormalOrNonDataDescriptor,
 }
@@ -2512,7 +2512,7 @@ impl<'db> Type<'db> {
     ///
     /// If `__get__` is not defined on the meta-type, this method returns `None`.
     #[salsa::tracked]
-    fn try_call_dunder_get(
+    pub(crate) fn try_call_dunder_get(
         self,
         db: &'db dyn Db,
         instance: Type<'db>,
@@ -2528,7 +2528,10 @@ impl<'db> Type<'db> {
 
         if let Symbol::Type(descr_get, descr_get_boundness) = 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| {
                     if descr_get_boundness == Boundness::Bound {
                         bindings.return_type(db)
@@ -4080,11 +4083,10 @@ impl<'db> Type<'db> {
     fn try_call(
         self,
         db: &'db dyn Db,
-        mut argument_types: CallArgumentTypes<'_, 'db>,
+        argument_types: &mut CallArgumentTypes<'_, 'db>,
     ) -> Result<Bindings<'db>, CallError<'db>> {
         let signatures = self.signatures(db);
-        Bindings::match_parameters(signatures, &mut argument_types)
-            .check_types(db, &mut argument_types)
+        Bindings::match_parameters(signatures, argument_types).check_types(db, argument_types)
     }
 
     /// Look up a dunder method on the meta-type of `self` and call it.
@@ -4348,16 +4350,27 @@ impl<'db> Type<'db> {
         // easy to check if that's the one we found?
         // 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 result = self_type.try_call_dunder_with_policy(
-                db,
-                "__new__",
-                argument_types,
-                MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
-                    | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
-            );
-            match result {
-                Err(CallDunderError::MethodNotAvailable) => None,
-                _ => Some(result),
+            let new_method = self_type
+                .find_name_in_mro_with_policy(
+                    db,
+                    "__new__",
+                    MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
+                        | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
+                )?
+                .symbol
+                .try_call_dunder_get(db, self_type);
+
+            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,
             }
         });
 
@@ -6133,12 +6146,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
 #[derive(Debug)]
 enum ConstructorCallError<'db> {
     Init(Type<'db>, CallDunderError<'db>),
-    New(Type<'db>, CallDunderError<'db>),
-    NewAndInit(Type<'db>, CallDunderError<'db>, CallDunderError<'db>),
+    New(Type<'db>, DunderNewCallError<'db>),
+    NewAndInit(Type<'db>, DunderNewCallError<'db>, CallDunderError<'db>),
 }
 
 impl<'db> ConstructorCallError<'db> {
@@ -6188,13 +6212,8 @@ impl<'db> ConstructorCallError<'db> {
             }
         };
 
-        let report_new_error = |call_dunder_error: &CallDunderError<'db>| match call_dunder_error {
-            CallDunderError::MethodNotAvailable => {
-                // 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) => {
+        let report_new_error = |error: &DunderNewCallError<'db>| match error {
+            DunderNewCallError::PossiblyUnbound(call_error) => {
                 if let Some(builder) =
                     context.report_lint(&CALL_POSSIBLY_UNBOUND_METHOD, context_expression_node)
                 {
@@ -6204,22 +6223,24 @@ impl<'db> ConstructorCallError<'db> {
                     ));
                 }
 
-                bindings.report_diagnostics(context, context_expression_node);
+                if let Some(CallError(_kind, bindings)) = call_error {
+                    bindings.report_diagnostics(context, context_expression_node);
+                }
             }
-            CallDunderError::CallError(_, bindings) => {
+            DunderNewCallError::CallError(CallError(_kind, bindings)) => {
                 bindings.report_diagnostics(context, context_expression_node);
             }
         };
 
         match self {
-            Self::Init(_, call_dunder_error) => {
-                report_init_error(call_dunder_error);
+            Self::Init(_, init_call_dunder_error) => {
+                report_init_error(init_call_dunder_error);
             }
-            Self::New(_, call_dunder_error) => {
-                report_new_error(call_dunder_error);
+            Self::New(_, new_call_error) => {
+                report_new_error(new_call_error);
             }
-            Self::NewAndInit(_, new_call_dunder_error, init_call_dunder_error) => {
-                report_new_error(new_call_dunder_error);
+            Self::NewAndInit(_, new_call_error, init_call_dunder_error) => {
+                report_new_error(new_call_error);
                 report_init_error(init_call_dunder_error);
             }
         }

crates/red_knot_python_semantic/src/types/call/bind.rs

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

crates/red_knot_python_semantic/src/types/class.rs

@@ -706,9 +706,9 @@ impl<'db> ClassLiteral<'db> {
             let namespace = KnownClass::Dict.to_instance(db);
 
             // 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)),
 
                 Err(CallError(CallErrorKind::NotCallable, bindings)) => Err(MetaclassError {
@@ -791,17 +791,14 @@ impl<'db> ClassLiteral<'db> {
             return Some(metaclass_call_function.into_callable_type(db));
         }
 
-        let new_function_symbol = self_ty
-            .member_lookup_with_policy(
-                db,
-                "__new__".into(),
-                MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
-                    | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
-            )
-            .symbol;
+        let dunder_new_method = self_ty
+            .find_name_in_mro(db, "__new__")
+            .expect("find_name_in_mro always succeeds for class literals")
+            .symbol
+            .try_call_dunder_get(db, self_ty);
 
-        if let Symbol::Type(Type::FunctionLiteral(new_function), _) = new_function_symbol {
-            return Some(new_function.into_bound_method_type(db, self.into()));
+        if let Symbol::Type(Type::FunctionLiteral(dunder_new_method), _) = dunder_new_method {
+            return Some(dunder_new_method.into_bound_method_type(db, self.into()));
         }
         // TODO handle `__init__` also
         None
@@ -879,12 +876,7 @@ impl<'db> ClassLiteral<'db> {
                         continue;
                     }
 
-                    // HACK: we should implement some more general logic here that supports arbitrary custom
-                    // metaclasses, not just `type` and `ABCMeta`.
-                    if matches!(
-                        class.known(db),
-                        Some(KnownClass::Type | KnownClass::ABCMeta)
-                    ) && policy.meta_class_no_type_fallback()
+                    if class.is_known(db, KnownClass::Type) && policy.meta_class_no_type_fallback()
                     {
                         continue;
                     }