Merge remote-tracking branch 'origin/main' into dcreager/object-variant

* origin/main:
  [ty] Minor fixes to `Protocol` tests (#20347)
  [ty] use Type::Divergent to avoid panic in infinitely-nested-tuple implicit attribute (#20333)
  [ty] Require that implementors of `Constraints` also implement `Debug` (#20348)

Author: dcreager

Cargo.lock

@@ -143,7 +143,7 @@ regex-automata = { version = "0.4.9" }
 rustc-hash = { version = "2.0.0" }
 rustc-stable-hash = { version = "0.1.2" }
 # When updating salsa, make sure to also update the revision in `fuzz/Cargo.toml`
-salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "a3ffa22cb26756473d56f867aedec3fd907c4dd9", default-features = false, features = [
+salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "3713cd7eb30821c0c086591832dd6f59f2af7fe7", default-features = false, features = [
     "compact_str",
     "macros",
     "salsa_unstable",

Cargo.toml

@@ -2419,6 +2419,16 @@ reveal_type(Answer.NO.value)  # revealed: Any
 reveal_type(Answer.__members__)  # revealed: MappingProxyType[str, Unknown]
 ```
 
+## Divergent inferred implicit instance attribute types
+
+```py
+class C:
+    def f(self, other: "C"):
+        self.x = (other.x, 1)
+
+reveal_type(C().x)  # revealed: Unknown | tuple[Divergent, Literal[1]]
+```
+
 ## References
 
 Some of the tests in the *Class and instance variables* section draw inspiration from

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -260,7 +260,7 @@ class SecondRequiredArgument:
     def __len__(self, v: int) -> Literal[1]:
         return 1
 
-# TODO: Emit a diagnostic
+# this is fine: the call succeeds at runtime since the second argument is optional
 reveal_type(len(SecondOptionalArgument()))  # revealed: Literal[0]
 
 # TODO: Emit a diagnostic

crates/ty_python_semantic/resources/mdtest/expression/len.md

@@ -77,7 +77,7 @@ from typing import Protocol, TypeVar
 T = TypeVar("T")
 
 class CanIndex(Protocol[T]):
-    def __getitem__(self, index: int) -> T: ...
+    def __getitem__(self, index: int, /) -> T: ...
 
 class ExplicitlyImplements(CanIndex[T]): ...
 

crates/ty_python_semantic/resources/mdtest/generics/legacy/functions.md

@@ -72,7 +72,7 @@ from typing import Protocol, TypeVar
 S = TypeVar("S")
 
 class CanIndex(Protocol[S]):
-    def __getitem__(self, index: int) -> S: ...
+    def __getitem__(self, index: int, /) -> S: ...
 
 class ExplicitlyImplements[T](CanIndex[T]): ...
 

crates/ty_python_semantic/resources/mdtest/generics/pep695/functions.md

@@ -1813,9 +1813,28 @@ class Foo:
 static_assert(not is_assignable_to(Foo, Iterable[Any]))
 ```
 
-Because method members must always be available on the class, it is safe to access a method on
-`type[P]`, where `P` is a protocol class, just like it is generally safe to access a method on
-`type[C]` where `C` is a nominal class:
+Because method members are always looked up on the meta-type of an object when testing assignability
+and subtyping, we understand that `IterableClass` here is a subtype of `Iterable[int]` even though
+`IterableClass.__iter__` has the wrong signature:
+
+```py
+from typing import Iterator, Iterable
+from ty_extensions import static_assert, is_subtype_of, TypeOf
+
+class Meta(type):
+    def __iter__(self) -> Iterator[int]:
+        yield from range(42)
+
+class IterableClass(metaclass=Meta):
+    def __iter__(self) -> Iterator[str]:
+        yield from "abc"
+
+static_assert(is_subtype_of(TypeOf[IterableClass], Iterable[int]))
+```
+
+Enforcing that members must always be available on the class also means that it is safe to access a
+method on `type[P]`, where `P` is a protocol class, just like it is generally safe to access a
+method on `type[C]` where `C` is a nominal class:
 
 ```py
 from typing import Protocol

crates/ty_python_semantic/resources/mdtest/protocols.md

@@ -762,6 +762,10 @@ impl<'db> Type<'db> {
         Self::Dynamic(DynamicType::Unknown)
     }
 
+    pub(crate) fn divergent(scope: ScopeId<'db>) -> Self {
+        Self::Dynamic(DynamicType::Divergent(DivergentType { scope }))
+    }
+
     pub const fn is_unknown(&self) -> bool {
         matches!(self, Type::Dynamic(DynamicType::Unknown))
     }
@@ -6499,7 +6503,6 @@ impl<'db> Type<'db> {
         }
     }
 
-    #[allow(unused)]
     pub(super) fn has_divergent_type(self, db: &'db dyn Db, div: Type<'db>) -> bool {
         any_over_type(db, self, &|ty| match ty {
             Type::Dynamic(DynamicType::Divergent(_)) => ty == div,

crates/ty_python_semantic/src/types.rs

@@ -111,7 +111,7 @@ fn incomparable<'db>(db: &'db dyn Db, left: Type<'db>, right: Type<'db>) -> bool
 }
 
 /// Encodes the constraints under which a type property (e.g. assignability) holds.
-pub(crate) trait Constraints<'db>: Clone + Sized {
+pub(crate) trait Constraints<'db>: Clone + Sized + std::fmt::Debug {
     /// Returns a constraint set that never holds
     fn unsatisfiable(db: &'db dyn Db) -> Self;
 

crates/ty_python_semantic/src/types/constraints.rs

@@ -85,7 +85,7 @@ fn scope_cycle_recover<'db>(
 }
 
 fn scope_cycle_initial<'db>(_db: &'db dyn Db, scope: ScopeId<'db>) -> ScopeInference<'db> {
-    ScopeInference::cycle_fallback(scope)
+    ScopeInference::cycle_initial(scope)
 }
 
 /// Infer all types for a [`Definition`] (including sub-expressions).
@@ -123,7 +123,7 @@ fn definition_cycle_initial<'db>(
     db: &'db dyn Db,
     definition: Definition<'db>,
 ) -> DefinitionInference<'db> {
-    DefinitionInference::cycle_fallback(definition.scope(db))
+    DefinitionInference::cycle_initial(definition.scope(db))
 }
 
 /// Infer types for all deferred type expressions in a [`Definition`].
@@ -164,7 +164,7 @@ fn deferred_cycle_initial<'db>(
     db: &'db dyn Db,
     definition: Definition<'db>,
 ) -> DefinitionInference<'db> {
-    DefinitionInference::cycle_fallback(definition.scope(db))
+    DefinitionInference::cycle_initial(definition.scope(db))
 }
 
 /// Infer all types for an [`Expression`] (including sub-expressions).
@@ -192,20 +192,29 @@ pub(crate) fn infer_expression_types<'db>(
         .finish_expression()
 }
 
+/// How many fixpoint iterations to allow before falling back to Divergent type.
+const ITERATIONS_BEFORE_FALLBACK: u32 = 10;
+
 fn expression_cycle_recover<'db>(
-    _db: &'db dyn Db,
+    db: &'db dyn Db,
     _value: &ExpressionInference<'db>,
-    _count: u32,
-    _expression: Expression<'db>,
+    count: u32,
+    expression: Expression<'db>,
 ) -> salsa::CycleRecoveryAction<ExpressionInference<'db>> {
-    salsa::CycleRecoveryAction::Iterate
+    if count == ITERATIONS_BEFORE_FALLBACK {
+        salsa::CycleRecoveryAction::Fallback(ExpressionInference::cycle_fallback(
+            expression.scope(db),
+        ))
+    } else {
+        salsa::CycleRecoveryAction::Iterate
+    }
 }
 
 fn expression_cycle_initial<'db>(
     db: &'db dyn Db,
     expression: Expression<'db>,
 ) -> ExpressionInference<'db> {
-    ExpressionInference::cycle_fallback(expression.scope(db))
+    ExpressionInference::cycle_initial(expression.scope(db))
 }
 
 /// Infers the type of an `expression` that is guaranteed to be in the same file as the calling query.
@@ -324,7 +333,7 @@ fn unpack_cycle_recover<'db>(
 }
 
 fn unpack_cycle_initial<'db>(_db: &'db dyn Db, _unpack: Unpack<'db>) -> UnpackResult<'db> {
-    UnpackResult::cycle_fallback(Type::Never)
+    UnpackResult::cycle_initial(Type::Never)
 }
 
 /// Returns the type of the nearest enclosing class for the given scope.
@@ -378,34 +387,61 @@ impl<'db> InferenceRegion<'db> {
     }
 }
 
+#[derive(Debug, Clone, Copy, Eq, PartialEq, get_size2::GetSize, salsa::Update)]
+enum CycleRecovery<'db> {
+    /// An initial-value for fixpoint iteration; all types are `Type::Never`.
+    Initial,
+    /// A divergence-fallback value for fixpoint iteration; all types are `Divergent`.
+    Divergent(ScopeId<'db>),
+}
+
+impl<'db> CycleRecovery<'db> {
+    fn merge(self, other: Option<CycleRecovery<'db>>) -> Self {
+        if let Some(other) = other {
+            match (self, other) {
+                // It's important here that we keep the scope of `self` if merging two `Divergent`.
+                (Self::Divergent(scope), _) | (_, Self::Divergent(scope)) => Self::Divergent(scope),
+                _ => Self::Initial,
+            }
+        } else {
+            self
+        }
+    }
+
+    fn fallback_type(self) -> Type<'db> {
+        match self {
+            Self::Initial => Type::Never,
+            Self::Divergent(scope) => Type::divergent(scope),
+        }
+    }
+}
+
 /// The inferred types for a scope region.
 #[derive(Debug, Eq, PartialEq, salsa::Update, get_size2::GetSize)]
 pub(crate) struct ScopeInference<'db> {
     /// The types of every expression in this region.
     expressions: FxHashMap<ExpressionNodeKey, Type<'db>>,
 
     /// The extra data that is only present for few inference regions.
-    extra: Option<Box<ScopeInferenceExtra>>,
+    extra: Option<Box<ScopeInferenceExtra<'db>>>,
 }
 
 #[derive(Debug, Eq, PartialEq, get_size2::GetSize, salsa::Update, Default)]
-struct ScopeInferenceExtra {
-    /// The fallback type for missing expressions/bindings/declarations.
-    ///
-    /// This is used only when constructing a cycle-recovery `TypeInference`.
-    cycle_fallback: bool,
+struct ScopeInferenceExtra<'db> {
+    /// Is this a cycle-recovery inference result, and if so, what kind?
+    cycle_recovery: Option<CycleRecovery<'db>>,
 
     /// The diagnostics for this region.
     diagnostics: TypeCheckDiagnostics,
 }
 
 impl<'db> ScopeInference<'db> {
-    fn cycle_fallback(scope: ScopeId<'db>) -> Self {
+    fn cycle_initial(scope: ScopeId<'db>) -> Self {
         let _ = scope;
 
         Self {
             extra: Some(Box::new(ScopeInferenceExtra {
-                cycle_fallback: true,
+                cycle_recovery: Some(CycleRecovery::Initial),
                 ..ScopeInferenceExtra::default()
             })),
             expressions: FxHashMap::default(),
@@ -431,14 +467,10 @@ impl<'db> ScopeInference<'db> {
             .or_else(|| self.fallback_type())
     }
 
-    fn is_cycle_callback(&self) -> bool {
+    fn fallback_type(&self) -> Option<Type<'db>> {
         self.extra
             .as_ref()
-            .is_some_and(|extra| extra.cycle_fallback)
-    }
-
-    fn fallback_type(&self) -> Option<Type<'db>> {
-        self.is_cycle_callback().then_some(Type::Never)
+            .and_then(|extra| extra.cycle_recovery.map(CycleRecovery::fallback_type))
     }
 }
 
@@ -471,10 +503,8 @@ pub(crate) struct DefinitionInference<'db> {
 
 #[derive(Debug, Eq, PartialEq, get_size2::GetSize, salsa::Update, Default)]
 struct DefinitionInferenceExtra<'db> {
-    /// The fallback type for missing expressions/bindings/declarations.
-    ///
-    /// This is used only when constructing a cycle-recovery `TypeInference`.
-    cycle_fallback: bool,
+    /// Is this a cycle-recovery inference result, and if so, what kind?
+    cycle_recovery: Option<CycleRecovery<'db>>,
 
     /// The definitions that are deferred.
     deferred: Box<[Definition<'db>]>,
@@ -487,7 +517,7 @@ struct DefinitionInferenceExtra<'db> {
 }
 
 impl<'db> DefinitionInference<'db> {
-    fn cycle_fallback(scope: ScopeId<'db>) -> Self {
+    fn cycle_initial(scope: ScopeId<'db>) -> Self {
         let _ = scope;
 
         Self {
@@ -497,7 +527,7 @@ impl<'db> DefinitionInference<'db> {
             #[cfg(debug_assertions)]
             scope,
             extra: Some(Box::new(DefinitionInferenceExtra {
-                cycle_fallback: true,
+                cycle_recovery: Some(CycleRecovery::Initial),
                 ..DefinitionInferenceExtra::default()
             })),
         }
@@ -566,14 +596,10 @@ impl<'db> DefinitionInference<'db> {
         self.declarations.iter().map(|(_, qualifiers)| *qualifiers)
     }
 
-    fn is_cycle_callback(&self) -> bool {
+    fn fallback_type(&self) -> Option<Type<'db>> {
         self.extra
             .as_ref()
-            .is_some_and(|extra| extra.cycle_fallback)
-    }
-
-    fn fallback_type(&self) -> Option<Type<'db>> {
-        self.is_cycle_callback().then_some(Type::Never)
+            .and_then(|extra| extra.cycle_recovery.map(CycleRecovery::fallback_type))
     }
 
     pub(crate) fn undecorated_type(&self) -> Option<Type<'db>> {
@@ -605,21 +631,34 @@ struct ExpressionInferenceExtra<'db> {
     /// The diagnostics for this region.
     diagnostics: TypeCheckDiagnostics,
 
-    /// `true` if this region is part of a cycle-recovery `TypeInference`.
-    ///
-    /// Falls back to `Type::Never` if an expression is missing.
-    cycle_fallback: bool,
+    /// Is this a cycle recovery inference result, and if so, what kind?
+    cycle_recovery: Option<CycleRecovery<'db>>,
 
     /// `true` if all places in this expression are definitely bound
     all_definitely_bound: bool,
 }
 
 impl<'db> ExpressionInference<'db> {
+    fn cycle_initial(scope: ScopeId<'db>) -> Self {
+        let _ = scope;
+        Self {
+            extra: Some(Box::new(ExpressionInferenceExtra {
+                cycle_recovery: Some(CycleRecovery::Initial),
+                all_definitely_bound: true,
+                ..ExpressionInferenceExtra::default()
+            })),
+            expressions: FxHashMap::default(),
+
+            #[cfg(debug_assertions)]
+            scope,
+        }
+    }
+
     fn cycle_fallback(scope: ScopeId<'db>) -> Self {
         let _ = scope;
         Self {
             extra: Some(Box::new(ExpressionInferenceExtra {
-                cycle_fallback: true,
+                cycle_recovery: Some(CycleRecovery::Divergent(scope)),
                 all_definitely_bound: true,
                 ..ExpressionInferenceExtra::default()
             })),
@@ -645,14 +684,10 @@ impl<'db> ExpressionInference<'db> {
             .unwrap_or_else(Type::unknown)
     }
 
-    fn is_cycle_callback(&self) -> bool {
+    fn fallback_type(&self) -> Option<Type<'db>> {
         self.extra
             .as_ref()
-            .is_some_and(|extra| extra.cycle_fallback)
-    }
-
-    fn fallback_type(&self) -> Option<Type<'db>> {
-        self.is_cycle_callback().then_some(Type::Never)
+            .and_then(|extra| extra.cycle_recovery.map(CycleRecovery::fallback_type))
     }
 
     /// Returns true if all places in this expression are definitely bound.