[ty] Do not consider a type T to satisfy a method member on a protocol unless the method is available on the meta-type of T

Author: AlexWaygood

crates/ty_python_semantic/resources/corpus/subclass_of_any_passed_to_tuple.py

@@ -0,0 +1,5 @@
+from unresolved_module import SomethingUnknown
+
+class Foo(SomethingUnknown): ...
+
+tuple(Foo)

crates/ty_python_semantic/resources/mdtest/loops/for.md

@@ -747,3 +747,42 @@ def f(never: Never):
     for x in never:
         reveal_type(x)  # revealed: Never
 ```
+
+## A class literal is iterable if it inherits from `Any`
+
+A class literal can be iterated over if it has `Any` or `Unknown` in its MRO, since the
+`Any`/`Unknown` element in the MRO could materialize to a class with a custom metaclass that defines
+`__iter__` for all instances of the metaclass:
+
+```py
+from unresolved_module import SomethingUnknown  # error: [unresolved-import]
+from typing import Any, Iterable
+from ty_extensions import static_assert, is_assignable_to, TypeOf, Unknown
+
+class Foo(SomethingUnknown): ...
+
+reveal_type(Foo.__mro__)  # revealed: tuple[<class 'Foo'>, Unknown, <class 'object'>]
+
+# TODO: these should pass
+static_assert(is_assignable_to(TypeOf[Foo], Iterable[Unknown]))  # error: [static-assert-error]
+static_assert(is_assignable_to(type[Foo], Iterable[Unknown]))  # error: [static-assert-error]
+
+# TODO: should not error
+# error: [not-iterable]
+for x in Foo:
+    reveal_type(x)  # revealed: Unknown
+
+class Bar(Any): ...
+
+reveal_type(Bar.__mro__)  # revealed: tuple[<class 'Bar'>, Any, <class 'object'>]
+
+# TODO: these should pass
+static_assert(is_assignable_to(TypeOf[Bar], Iterable[Any]))  # error: [static-assert-error]
+static_assert(is_assignable_to(type[Bar], Iterable[Any]))  # error: [static-assert-error]
+
+# TODO: should not error
+# error: [not-iterable]
+for x in Bar:
+    # TODO: should reveal `Any`
+    reveal_type(x)  # revealed: Unknown
+```

crates/ty_python_semantic/resources/mdtest/protocols.md

@@ -1460,6 +1460,48 @@ static_assert(is_subtype_of(NominalSubtype, P))
 static_assert(not is_subtype_of(NotSubtype, P))  # error: [static-assert-error]
 ```
 
+A callable instance attribute is not sufficient for a type to satisfy a protocol with a method
+member: a method member specified by a protocol `P` must exist on the *meta-type* of `T` for `T` to
+be a subtype of `P`:
+
+```py
+from typing import Callable, Protocol
+from ty_extensions import static_assert, is_assignable_to
+
+class SupportsFooMethod(Protocol):
+    def foo(self): ...
+
+class SupportsFooAttr(Protocol):
+    foo: Callable[..., object]
+
+class Foo:
+    def __init__(self):
+        self.foo: Callable[..., object] = lambda *args, **kwargs: None
+
+static_assert(not is_assignable_to(Foo, SupportsFooMethod))
+static_assert(is_assignable_to(Foo, SupportsFooAttr))
+```
+
+The reason for this is that some methods, such as dunder methods, are always looked up on the class
+directly. If a class with an `__iter__` instance attribute satisfied the `Iterable` protocol, for
+example, the `Iterable` protocol would not accurately describe the requirements Python has for a
+class to be iterable at runtime. Allowing callable instance attributes to satisfy method members of
+protocols would also make `issubclass()` narrowing of runtime-checkable protocols unsound, as the
+`issubclass()` mechanism at runtime for protocols only checks whether a method is accessible on the
+class object, not the instance. (Protocols with non-method members cannot be passed to
+`issubclass()` at all at runtime.)
+
+```py
+from typing import Iterable, Any
+from ty_extensions import static_assert, is_assignable_to
+
+class Foo:
+    def __init__(self):
+        self.__iter__: Callable[..., object] = lambda *args, **kwargs: None
+
+static_assert(not is_assignable_to(Foo, Iterable[Any]))
+```
+
 ## Equivalence of protocols with method members
 
 Two protocols `P1` and `P2`, both with a method member `x`, are considered equivalent if the

crates/ty_python_semantic/src/types/property_tests.rs

@@ -68,7 +68,7 @@ macro_rules! type_property_test {
 
 mod stable {
     use super::union;
-    use crate::types::{CallableType, Type};
+    use crate::types::{CallableType, KnownClass, Type};
 
     // Reflexivity: `T` is equivalent to itself.
     type_property_test!(
@@ -205,6 +205,16 @@ mod stable {
         all_fully_static_type_pairs_are_subtype_of_their_union, db,
         forall fully_static_types s, t. s.is_subtype_of(db, union(db, [s, t])) && t.is_subtype_of(db, union(db, [s, t]))
     );
+
+    // Any type assignable to `Iterable[object]` should be considered iterable.
+    //
+    // Note that the inverse is not true, due to the fact that we recognize the old-style
+    // iteration protocol as well as the new-style iteration protocol: not all objects that
+    // we consider iterable are assignable to `Iterable[object]`.
+    type_property_test!(
+        all_type_assignable_to_iterable_are_iterable, db,
+        forall types t. t.is_assignable_to(db, KnownClass::Iterable.to_specialized_instance(db, [Type::object(db)])) => t.try_iterate(db).is_ok()
+    );
 }
 
 /// This module contains property tests that currently lead to many false positives.
@@ -218,7 +228,6 @@ mod flaky {
     use itertools::Itertools;
 
     use super::{intersection, union};
-    use crate::types::{KnownClass, Type};
 
     // Negating `T` twice is equivalent to `T`.
     type_property_test!(
@@ -312,14 +321,4 @@ mod flaky {
         bottom_materialization_of_type_is_assigneble_to_type, db,
         forall types t. t.bottom_materialization(db).is_assignable_to(db, t)
     );
-
-    // Any type assignable to `Iterable[object]` should be considered iterable.
-    //
-    // Note that the inverse is not true, due to the fact that we recognize the old-style
-    // iteration protocol as well as the new-style iteration protocol: not all objects that
-    // we consider iterable are assignable to `Iterable[object]`.
-    type_property_test!(
-        all_type_assignable_to_iterable_are_iterable, db,
-        forall types t. t.is_assignable_to(db, KnownClass::Iterable.to_specialized_instance(db, [Type::object(db)])) => t.try_iterate(db).is_ok()
-    );
 }

crates/ty_python_semantic/src/types/protocol_class.rs

@@ -383,15 +383,23 @@ impl<'a, 'db> ProtocolMember<'a, 'db> {
         other: Type<'db>,
         relation: TypeRelation,
     ) -> bool {
-        let Place::Type(attribute_type, Boundness::Bound) = other.member(db, self.name).place
-        else {
-            return false;
-        };
-
         match &self.kind {
-            // TODO: consider the types of the attribute on `other` for property/method members
-            ProtocolMemberKind::Method(_) | ProtocolMemberKind::Property(_) => true,
+            // TODO: consider the types of the attribute on `other` for method members
+            ProtocolMemberKind::Method(_) => matches!(
+                other.to_meta_type(db).member(db, self.name).place,
+                Place::Type(_, Boundness::Bound)
+            ),
+            // TODO: consider the types of the attribute on `other` for property members
+            ProtocolMemberKind::Property(_) => matches!(
+                other.member(db, self.name).place,
+                Place::Type(_, Boundness::Bound)
+            ),
             ProtocolMemberKind::Other(member_type) => {
+                let Place::Type(attribute_type, Boundness::Bound) =
+                    other.member(db, self.name).place
+                else {
+                    return false;
+                };
                 member_type.has_relation_to(db, attribute_type, relation)
                     && attribute_type.has_relation_to(db, *member_type, relation)
             }