From e71250113b40ce966538d3513b2a4b3a6b30d6ee Mon Sep 17 00:00:00 2001 From: mishamsk Date: Sun, 27 Apr 2025 21:43:09 -0400 Subject: [PATCH 1/2] fix lookups in class construction --- .../resources/mdtest/call/constructor.md | 21 +++++++++++++++ crates/red_knot_python_semantic/src/types.rs | 27 +++++++++++++++---- .../src/types/class.rs | 8 ++---- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/crates/red_knot_python_semantic/resources/mdtest/call/constructor.md b/crates/red_knot_python_semantic/resources/mdtest/call/constructor.md index 56a49fc13f97c..c3359976e87c9 100644 --- a/crates/red_knot_python_semantic/resources/mdtest/call/constructor.md +++ b/crates/red_knot_python_semantic/resources/mdtest/call/constructor.md @@ -20,6 +20,7 @@ Since every class has `object` in it's MRO, the default implementations are `obj - 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 `__init__` is defined, but `__new__` is not - `object.__new__` will allow arbitrary arguments! As of today there are a number of behaviors that we do not support: @@ -323,3 +324,23 @@ 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 ``` + +## `__new__` defined on a meta-class should not be used + +We lookup `__new__` method using descriptor protocol, which technically allows it to be found on a +meta-class if `__new__` is not defined anywher in class MRO. At runtime this is never the case, +since all classes have `object` in their MRO, which has a `__new__` method. However, for the +purposes of type checking we use special lookup logic that ignores `object.__new__` as it's runtime +behavior is not expressible in typeshed. This not cause false-positives when `__new__` is defined on +a meta-class. + +```py +class Meta(type): + def __new__(mcls, name, bases, namespace, /, **kwargs): + return super().__new__(mcls, name, bases, namespace) + +class Foo(metaclass=Meta): ... + +# This should not raise an error +reveal_type(Foo()) # revealed: Foo +``` diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 070adc5aabae3..e15f763f088fb 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -201,6 +201,7 @@ bitflags! { /// When looking up an attribute on a class, we sometimes need to avoid /// looking up attributes defined on `type` if this is the metaclass of the class. + /// For example, this is used to find if a meta-class implements a `__call__` method. /// /// This is similar to no object fallback above const META_CLASS_NO_TYPE_FALLBACK = 1 << 2; @@ -2641,12 +2642,28 @@ impl<'db> Type<'db> { qualifiers: meta_attr_qualifiers, }, meta_attr_kind, - ) = Self::try_call_dunder_get_on_attribute( - db, - self.class_member_with_policy(db, name.into(), member_policy), + ) = if matches!( self, - self.to_meta_type(db), - ); + Type::ClassLiteral(_) | Type::GenericAlias(_) | Type::SubclassOf(_) + ) && member_policy.mro_no_object_fallback() + { + // When looking up the attribute on the meta-type with `NO_OBJECT_FALLBACK` policy, we + // imply that the looked up attribute exists on `object` even if we do not want to fall back + // back to it. This in turn means that descriptor protocol will not select same named + // data-descriptor on the meta-type - hence we return `SymbolAndQualifiers::default()` here, + // which is essentially a `Symbol::Unbound` with no qualifiers. + ( + SymbolAndQualifiers::default(), + AttributeKind::NormalOrNonDataDescriptor, + ) + } else { + Self::try_call_dunder_get_on_attribute( + db, + self.class_member_with_policy(db, name.into(), member_policy), + self, + self.to_meta_type(db), + ) + }; let SymbolAndQualifiers { symbol: fallback, diff --git a/crates/red_knot_python_semantic/src/types/class.rs b/crates/red_knot_python_semantic/src/types/class.rs index b3c28473fcde0..b7025b8d2dc28 100644 --- a/crates/red_knot_python_semantic/src/types/class.rs +++ b/crates/red_knot_python_semantic/src/types/class.rs @@ -879,12 +879,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; } From c5b15ce4027bb95c909914af2a540a765907a503 Mon Sep 17 00:00:00 2001 From: mishamsk Date: Mon, 28 Apr 2025 13:58:21 -0400 Subject: [PATCH 2/2] add explanation comment to mro class search implementation --- crates/red_knot_python_semantic/src/types/class.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/red_knot_python_semantic/src/types/class.rs b/crates/red_knot_python_semantic/src/types/class.rs index b7025b8d2dc28..beb17f7ad57e3 100644 --- a/crates/red_knot_python_semantic/src/types/class.rs +++ b/crates/red_knot_python_semantic/src/types/class.rs @@ -879,6 +879,13 @@ impl<'db> ClassLiteral<'db> { continue; } + // Some meta-class methods such as `__call__` need special treatment depending on + // whether they are explicitly defined, or inherited from `type.__call__`. + // We use `MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK` flag and it's getter + // `policy.meta_class_no_type_fallback` to allow downstream code to check whether + // a custom implementation of such symbol exists on a type. + // E.g. this is currently used in `ClassLiteral::into_callable` which passes this + // policy here through a chain of calls. if matches!(class.known(db), Some(KnownClass::Type)) && policy.meta_class_no_type_fallback() {