[red-knot] Improve Symbol API for callable types (#14137)

## Summary

- Get rid of `Symbol::unwrap_or` (unclear semantics, not needed anymore)
- Introduce `Type::call_dunder`
- Emit new diagnostic for possibly-unbound `__iter__` methods
- Better diagnostics for callables with possibly-unbound /
possibly-non-callable `__call__` methods

part of: #14022 

closes #14016

## Test Plan

- Updated test for iterables with possibly-unbound `__iter__` methods.
- New tests for callables

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

@@ -18,3 +18,58 @@ class Unit: ...
 b = Unit()(3.0)  # error: "Object of type `Unit` is not callable"
 reveal_type(b)  # revealed: Unknown
 ```
+
+## Possibly unbound `__call__` method
+
+```py
+def flag() -> bool: ...
+
+class PossiblyNotCallable:
+    if flag():
+        def __call__(self) -> int: ...
+
+a = PossiblyNotCallable()
+result = a()  # error: "Object of type `PossiblyNotCallable` is not callable (possibly unbound `__call__` method)"
+reveal_type(result)  # revealed: int
+```
+
+## Possibly unbound callable
+
+```py
+def flag() -> bool: ...
+
+if flag():
+    class PossiblyUnbound:
+        def __call__(self) -> int: ...
+
+# error: [possibly-unresolved-reference]
+a = PossiblyUnbound()
+reveal_type(a())  # revealed: int
+```
+
+## Non-callable `__call__`
+
+```py
+class NonCallable:
+    __call__ = 1
+
+a = NonCallable()
+# error: "Object of type `NonCallable` is not callable"
+reveal_type(a())  # revealed: Unknown
+```
+
+## Possibly non-callable `__call__`
+
+```py
+def flag() -> bool: ...
+
+class NonCallable:
+    if flag():
+        __call__ = 1
+    else:
+        def __call__(self) -> int: ...
+
+a = NonCallable()
+# error: "Object of type `Literal[1] | Literal[__call__]` is not callable (due to union element `Literal[1]`)"
+reveal_type(a())  # revealed: Unknown | int
+```

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

@@ -44,3 +44,16 @@ reveal_type(bar())  # revealed: @Todo
 nonsense = 123
 x = nonsense()  # error: "Object of type `Literal[123]` is not callable"
 ```
+
+## Potentially unbound function
+
+```py
+def flag() -> bool: ...
+
+if flag():
+    def foo() -> int:
+        return 42
+
+# error: [possibly-unresolved-reference]
+reveal_type(foo())  # revealed: int
+```

crates/red_knot_python_semantic/resources/mdtest/loops/for_loop.md

@@ -238,7 +238,7 @@ class Test:
 def coinflip() -> bool:
     return True
 
-# TODO: we should emit a diagnostic here (it might not be iterable)
+# error: [not-iterable] "Object of type `Test | Literal[42]` is not iterable because its `__iter__` method is possibly unbound"
 for x in Test() if coinflip() else 42:
     reveal_type(x)  # revealed: int
 ```

crates/red_knot_python_semantic/src/symbol.rs

@@ -3,7 +3,7 @@ use crate::{
     Db,
 };
 
-#[derive(Debug, Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub(crate) enum Boundness {
     Bound,
     MayBeUnbound,
@@ -44,17 +44,13 @@ impl<'db> Symbol<'db> {
         }
     }
 
-    pub(crate) fn unwrap_or(&self, other: Type<'db>) -> Type<'db> {
+    pub(crate) fn unwrap_or_unknown(&self) -> Type<'db> {
         match self {
             Symbol::Type(ty, _) => *ty,
-            Symbol::Unbound => other,
+            Symbol::Unbound => Type::Unknown,
         }
     }
 
-    pub(crate) fn unwrap_or_unknown(&self) -> Type<'db> {
-        self.unwrap_or(Type::Unknown)
-    }
-
     pub(crate) fn as_type(&self) -> Option<Type<'db>> {
         match self {
             Symbol::Type(ty, _) => Some(*ty),

crates/red_knot_python_semantic/src/types.rs

@@ -1208,17 +1208,33 @@ impl<'db> Type<'db> {
                 })
             }
 
-            Type::Instance(InstanceType { class, .. }) => {
-                // Since `__call__` is a dunder, we need to access it as an attribute on the class
-                // rather than the instance (matching runtime semantics).
-                match class.class_member(db, "__call__") {
-                    Symbol::Type(dunder_call_method, Boundness::Bound) => {
-                        let args = std::iter::once(self)
-                            .chain(arg_types.iter().copied())
-                            .collect::<Vec<_>>();
-                        dunder_call_method.call(db, &args)
+            instance_ty @ Type::Instance(InstanceType { .. }) => {
+                let args = std::iter::once(self)
+                    .chain(arg_types.iter().copied())
+                    .collect::<Vec<_>>();
+                match instance_ty.call_dunder(db, "__call__", &args) {
+                    CallDunderResult::CallOutcome(CallOutcome::NotCallable { .. }) => {
+                        // Turn "`<type of illegal '__call__'>` not callable" into
+                        // "`X` not callable"
+                        CallOutcome::NotCallable {
+                            not_callable_ty: self,
+                        }
+                    }
+                    CallDunderResult::CallOutcome(outcome) => outcome,
+                    CallDunderResult::PossiblyUnbound(call_outcome) => {
+                        // Turn "possibly unbound object of type `Literal['__call__']`"
+                        // into "`X` not callable (possibly unbound `__call__` method)"
+                        CallOutcome::PossiblyUnboundDunderCall {
+                            called_ty: self,
+                            call_outcome: Box::new(call_outcome),
+                        }
+                    }
+                    CallDunderResult::MethodNotAvailable => {
+                        // Turn "`X.__call__` unbound" into "`X` not callable"
+                        CallOutcome::NotCallable {
+                            not_callable_ty: self,
+                        }
                     }
-                    _ => CallOutcome::not_callable(self),
                 }
             }
 
@@ -1244,6 +1260,24 @@ impl<'db> Type<'db> {
         }
     }
 
+    /// Look up a dunder method on the meta type of `self` and call it.
+    fn call_dunder(
+        self,
+        db: &'db dyn Db,
+        name: &str,
+        arg_types: &[Type<'db>],
+    ) -> CallDunderResult<'db> {
+        match self.to_meta_type(db).member(db, name) {
+            Symbol::Type(callable_ty, Boundness::Bound) => {
+                CallDunderResult::CallOutcome(callable_ty.call(db, arg_types))
+            }
+            Symbol::Type(callable_ty, Boundness::MayBeUnbound) => {
+                CallDunderResult::PossiblyUnbound(callable_ty.call(db, arg_types))
+            }
+            Symbol::Unbound => CallDunderResult::MethodNotAvailable,
+        }
+    }
+
     /// Given the type of an object that is iterated over in some way,
     /// return the type of objects that are yielded by that iteration.
     ///
@@ -1265,34 +1299,35 @@ impl<'db> Type<'db> {
             return IterationOutcome::Iterable { element_ty: self };
         }
 
-        // `self` represents the type of the iterable;
-        // `__iter__` and `__next__` are both looked up on the class of the iterable:
-        let iterable_meta_type = self.to_meta_type(db);
-
-        let dunder_iter_method = iterable_meta_type.member(db, "__iter__");
-        if let Symbol::Type(dunder_iter_method, _) = dunder_iter_method {
-            let Some(iterator_ty) = dunder_iter_method.call(db, &[self]).return_ty(db) else {
-                return IterationOutcome::NotIterable {
-                    not_iterable_ty: self,
-                };
-            };
-
-            match iterator_ty.to_meta_type(db).member(db, "__next__") {
-                Symbol::Type(dunder_next_method, Boundness::Bound) => {
-                    return dunder_next_method
-                        .call(db, &[iterator_ty])
-                        .return_ty(db)
-                        .map(|element_ty| IterationOutcome::Iterable { element_ty })
-                        .unwrap_or(IterationOutcome::NotIterable {
-                            not_iterable_ty: self,
-                        });
-                }
-                _ => {
+        let dunder_iter_result = self.call_dunder(db, "__iter__", &[self]);
+        match dunder_iter_result {
+            CallDunderResult::CallOutcome(ref call_outcome)
+            | CallDunderResult::PossiblyUnbound(ref call_outcome) => {
+                let Some(iterator_ty) = call_outcome.return_ty(db) else {
                     return IterationOutcome::NotIterable {
                         not_iterable_ty: self,
                     };
-                }
+                };
+
+                return if let Some(element_ty) = iterator_ty
+                    .call_dunder(db, "__next__", &[iterator_ty])
+                    .return_ty(db)
+                {
+                    if matches!(dunder_iter_result, CallDunderResult::PossiblyUnbound(..)) {
+                        IterationOutcome::PossiblyUnboundDunderIter {
+                            iterable_ty: self,
+                            element_ty,
+                        }
+                    } else {
+                        IterationOutcome::Iterable { element_ty }
+                    }
+                } else {
+                    IterationOutcome::NotIterable {
+                        not_iterable_ty: self,
+                    }
+                };
             }
+            CallDunderResult::MethodNotAvailable => {}
         }
 
         // Although it's not considered great practice,
@@ -1301,17 +1336,15 @@ impl<'db> Type<'db> {
         //
         // TODO(Alex) this is only valid if the `__getitem__` method is annotated as
         // accepting `int` or `SupportsIndex`
-        match iterable_meta_type.member(db, "__getitem__") {
-            Symbol::Type(dunder_get_item_method, Boundness::Bound) => dunder_get_item_method
-                .call(db, &[self, KnownClass::Int.to_instance(db)])
-                .return_ty(db)
-                .map(|element_ty| IterationOutcome::Iterable { element_ty })
-                .unwrap_or(IterationOutcome::NotIterable {
-                    not_iterable_ty: self,
-                }),
-            _ => IterationOutcome::NotIterable {
+        if let Some(element_ty) = self
+            .call_dunder(db, "__getitem__", &[self, KnownClass::Int.to_instance(db)])
+            .return_ty(db)
+        {
+            IterationOutcome::Iterable { element_ty }
+        } else {
+            IterationOutcome::NotIterable {
                 not_iterable_ty: self,
-            },
+            }
         }
     }
 
@@ -1632,6 +1665,10 @@ enum CallOutcome<'db> {
         called_ty: Type<'db>,
         outcomes: Box<[CallOutcome<'db>]>,
     },
+    PossiblyUnboundDunderCall {
+        called_ty: Type<'db>,
+        call_outcome: Box<CallOutcome<'db>>,
+    },
 }
 
 impl<'db> CallOutcome<'db> {
@@ -1689,6 +1726,7 @@ impl<'db> CallOutcome<'db> {
                     }
                 })
                 .map(UnionBuilder::build),
+            Self::PossiblyUnboundDunderCall { call_outcome, .. } => call_outcome.return_ty(db),
         }
     }
 
@@ -1747,6 +1785,20 @@ impl<'db> CallOutcome<'db> {
                 );
                 return_ty
             }
+            Err(NotCallableError::PossiblyUnboundDunderCall {
+                callable_ty: called_ty,
+                return_ty,
+            }) => {
+                diagnostics.add(
+                    node,
+                    "call-non-callable",
+                    format_args!(
+                        "Object of type `{}` is not callable (possibly unbound `__call__` method)",
+                        called_ty.display(db)
+                    ),
+                );
+                return_ty
+            }
         }
     }
 
@@ -1774,6 +1826,13 @@ impl<'db> CallOutcome<'db> {
                 not_callable_ty: *not_callable_ty,
                 return_ty: Type::Unknown,
             }),
+            Self::PossiblyUnboundDunderCall {
+                called_ty,
+                call_outcome,
+            } => Err(NotCallableError::PossiblyUnboundDunderCall {
+                callable_ty: *called_ty,
+                return_ty: call_outcome.return_ty(db).unwrap_or(Type::Unknown),
+            }),
             Self::Union {
                 outcomes,
                 called_ty,
@@ -1825,6 +1884,22 @@ impl<'db> CallOutcome<'db> {
     }
 }
 
+enum CallDunderResult<'db> {
+    CallOutcome(CallOutcome<'db>),
+    PossiblyUnbound(CallOutcome<'db>),
+    MethodNotAvailable,
+}
+
+impl<'db> CallDunderResult<'db> {
+    fn return_ty(&self, db: &'db dyn Db) -> Option<Type<'db>> {
+        match self {
+            Self::CallOutcome(outcome) => outcome.return_ty(db),
+            Self::PossiblyUnbound { .. } => None,
+            Self::MethodNotAvailable => None,
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 enum NotCallableError<'db> {
     /// The type is not callable.
@@ -1844,6 +1919,10 @@ enum NotCallableError<'db> {
         called_ty: Type<'db>,
         return_ty: Type<'db>,
     },
+    PossiblyUnboundDunderCall {
+        callable_ty: Type<'db>,
+        return_ty: Type<'db>,
+    },
 }
 
 impl<'db> NotCallableError<'db> {
@@ -1853,6 +1932,7 @@ impl<'db> NotCallableError<'db> {
             Self::Type { return_ty, .. } => *return_ty,
             Self::UnionElement { return_ty, .. } => *return_ty,
             Self::UnionElements { return_ty, .. } => *return_ty,
+            Self::PossiblyUnboundDunderCall { return_ty, .. } => *return_ty,
         }
     }
 
@@ -1867,14 +1947,26 @@ impl<'db> NotCallableError<'db> {
             } => *not_callable_ty,
             Self::UnionElement { called_ty, .. } => *called_ty,
             Self::UnionElements { called_ty, .. } => *called_ty,
+            Self::PossiblyUnboundDunderCall {
+                callable_ty: called_ty,
+                ..
+            } => *called_ty,
         }
     }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum IterationOutcome<'db> {
-    Iterable { element_ty: Type<'db> },
-    NotIterable { not_iterable_ty: Type<'db> },
+    Iterable {
+        element_ty: Type<'db>,
+    },
+    NotIterable {
+        not_iterable_ty: Type<'db>,
+    },
+    PossiblyUnboundDunderIter {
+        iterable_ty: Type<'db>,
+        element_ty: Type<'db>,
+    },
 }
 
 impl<'db> IterationOutcome<'db> {
@@ -1889,6 +1981,13 @@ impl<'db> IterationOutcome<'db> {
                 diagnostics.add_not_iterable(iterable_node, not_iterable_ty);
                 Type::Unknown
             }
+            Self::PossiblyUnboundDunderIter {
+                iterable_ty,
+                element_ty,
+            } => {
+                diagnostics.add_not_iterable_possibly_unbound(iterable_node, iterable_ty);
+                element_ty
+            }
         }
     }
 }

crates/red_knot_python_semantic/src/types/diagnostic.rs

@@ -164,6 +164,23 @@ impl<'db> TypeCheckDiagnosticsBuilder<'db> {
         );
     }
 
+    /// Emit a diagnostic declaring that the object represented by `node` is not iterable
+    /// because its `__iter__` method is possibly unbound.
+    pub(super) fn add_not_iterable_possibly_unbound(
+        &mut self,
+        node: AnyNodeRef,
+        element_ty: Type<'db>,
+    ) {
+        self.add(
+            node,
+            "not-iterable",
+            format_args!(
+                "Object of type `{}` is not iterable because its `__iter__` method is possibly unbound",
+                element_ty.display(self.db)
+            ),
+        );
+    }
+
     /// Emit a diagnostic declaring that an index is out of bounds for a tuple.
     pub(super) fn add_index_out_of_bounds(
         &mut self,

crates/red_knot_python_semantic/src/types/infer.rs

@@ -2957,22 +2957,19 @@ impl<'db> TypeInferenceBuilder<'db> {
                 op,
             ),
 
-            (Type::Instance(left), Type::Instance(right), op) => {
+            (left_ty @ Type::Instance(left), right_ty @ Type::Instance(right), op) => {
                 if left != right && right.is_instance_of(self.db, left.class) {
                     let reflected_dunder = op.reflected_dunder();
                     let rhs_reflected = right.class.class_member(self.db, reflected_dunder);
                     if !rhs_reflected.is_unbound()
                         && rhs_reflected != left.class.class_member(self.db, reflected_dunder)
                     {
-                        return rhs_reflected
-                            .unwrap_or(Type::Never)
-                            .call(self.db, &[right_ty, left_ty])
+                        return right_ty
+                            .call_dunder(self.db, reflected_dunder, &[right_ty, left_ty])
                             .return_ty(self.db)
                             .or_else(|| {
-                                left.class
-                                    .class_member(self.db, op.dunder())
-                                    .unwrap_or(Type::Never)
-                                    .call(self.db, &[left_ty, right_ty])
+                                left_ty
+                                    .call_dunder(self.db, op.dunder(), &[left_ty, right_ty])
                                     .return_ty(self.db)
                             });
                     }
@@ -4398,7 +4395,8 @@ fn perform_membership_test_comparison<'db>(
             // iteration-based membership test
             match Type::Instance(right).iterate(db) {
                 IterationOutcome::Iterable { .. } => Some(KnownClass::Bool.to_instance(db)),
-                IterationOutcome::NotIterable { .. } => None,
+                IterationOutcome::NotIterable { .. }
+                | IterationOutcome::PossiblyUnboundDunderIter { .. } => None,
             }
         }
     };