[red-knot] Lookup of __new__

Author: sharkdp

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
+When classes are instantiated, Python calls the metaclass `__call__` method, which can either be
 customized by the user or `type.__call__` is used.
 
 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.
 
-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 bound method and
+is 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,33 @@ 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.
+    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>,
@@ -4348,16 +4348,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, CallArgumentTypes::clone(argument_types));
+
+                    if boundness == Boundness::PossiblyUnbound {
+                        return Some(Err(DunderNewCallError::PossiblyUnbound(result.err())));
+                    }
+
+                    Some(result.map_err(DunderNewCallError::CallError))
+                }
+                Symbol::Unbound => None,
             }
         });
 
@@ -6133,12 +6144,18 @@ impl<'db> BoolError<'db> {
     }
 }
 
+#[derive(Debug)]
+enum DunderNewCallError<'db> {
+    CallError(CallError<'db>),
+    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 +6205,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 +6216,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/arguments.rs

@@ -55,6 +55,7 @@ pub(crate) enum Argument<'a> {
 }
 
 /// Arguments for a single call, in source order, along with inferred types for each argument.
+#[derive(Clone, Debug)]
 pub(crate) struct CallArgumentTypes<'a, 'db> {
     arguments: CallArguments<'a>,
     types: VecDeque<Type<'db>>,

crates/red_knot_python_semantic/src/types/class.rs

@@ -792,13 +792,10 @@ impl<'db> ClassLiteral<'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;
+            .find_name_in_mro(db, "__new__".into())
+            .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()));
@@ -879,12 +876,8 @@ 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 matches!(class.known(db), Some(KnownClass::Type))
+                        && policy.meta_class_no_type_fallback()
                     {
                         continue;
                     }