fix cast() calls that pass recursive protocols as the first argument

Author: AlexWaygood

crates/ty_python_semantic/resources/mdtest/protocols.md

@@ -1935,6 +1935,27 @@ def f(x: PGconn):
     isinstance(x, Connection)
 ```
 
+### Recursive protocols used as the first argument to `cast()`
+
+These caused issues in an early version of our `Protocol` implementation due to the fact that we use
+a recursive function in our `cast()` implementation to check whether a type contains `Unknown` or
+`Todo`. Recklessly recursing into a type causes stack overflows if the type is recursive:
+
+```toml
+[environment]
+python-version = "3.12"
+```
+
+```py
+from typing import cast, Protocol
+
+class Iterator[T](Protocol):
+    def __iter__(self) -> Iterator[T]: ...
+
+def f(value: Iterator):
+    cast(Iterator, value)  # error: [redundant-cast]
+```
+
 ## TODO
 
 Add tests for:

crates/ty_python_semantic/src/lib.rs

@@ -42,6 +42,7 @@ mod util;
 pub mod pull_types;
 
 type FxOrderSet<V> = ordermap::set::OrderSet<V, BuildHasherDefault<FxHasher>>;
+type FxIndexSet<V> = indexmap::IndexSet<V, BuildHasherDefault<FxHasher>>;
 type FxIndexMap<K, V> = indexmap::IndexMap<K, V, BuildHasherDefault<FxHasher>>;
 
 /// Returns the default registry with all known semantic lints.

crates/ty_python_semantic/src/types.rs

@@ -53,7 +53,7 @@ pub(crate) use crate::types::narrow::infer_narrowing_constraint;
 use crate::types::signatures::{Parameter, ParameterForm, Parameters};
 use crate::types::tuple::{TupleSpec, TupleType};
 pub use crate::util::diagnostics::add_inferred_python_version_hint_to_diagnostic;
-use crate::{Db, FxOrderSet, Module, Program};
+use crate::{Db, FxIndexSet, FxOrderSet, Module, Program};
 pub(crate) use class::{ClassLiteral, ClassType, GenericAlias, KnownClass};
 use instance::Protocol;
 pub use instance::{NominalInstanceType, ProtocolInstanceType};
@@ -415,12 +415,18 @@ impl<'db> PropertyInstanceType<'db> {
         )
     }
 
-    fn any_over_type(self, db: &'db dyn Db, type_fn: &dyn Fn(Type<'db>) -> bool) -> bool {
+    fn any_over_type_impl(
+        self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+        seen_types: &mut FxIndexSet<Type<'db>>,
+    ) -> bool {
         self.getter(db)
-            .is_some_and(|ty| ty.any_over_type(db, type_fn))
-            || self
-                .setter(db)
-                .is_some_and(|ty| ty.any_over_type(db, type_fn))
+            .is_some_and(|ty| ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types))
+            || self.setter(db).is_some_and(|ty| {
+                ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            })
     }
 }
 
@@ -755,8 +761,27 @@ impl<'db> Type<'db> {
         }
     }
 
+    pub(crate) fn any_over_type(
+        self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+    ) -> bool {
+        self.any_over_type_impl(db, type_fn, recursive_fallback, &mut FxIndexSet::default())
+    }
+
     /// Return `true` if `self`, or any of the types contained in `self`, match the closure passed in.
-    pub fn any_over_type(self, db: &'db dyn Db, type_fn: &dyn Fn(Type<'db>) -> bool) -> bool {
+    pub(crate) fn any_over_type_impl(
+        self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+        seen_types: &mut FxIndexSet<Type<'db>>,
+    ) -> bool {
+        if !seen_types.insert(self) {
+            return recursive_fallback;
+        }
+
         if type_fn(self) {
             return true;
         }
@@ -787,64 +812,83 @@ impl<'db> Type<'db> {
                 .types(db)
                 .iter()
                 .copied()
-                .any(|ty| ty.any_over_type(db, type_fn)),
-
-            Self::Callable(callable) => callable.any_over_type(db, type_fn),
+                .any(|ty| ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)),
 
-            Self::SubclassOf(subclass_of) => {
-                Type::from(subclass_of.subclass_of()).any_over_type(db, type_fn)
+            Self::Callable(callable) => {
+                callable.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
             }
 
+            Self::SubclassOf(subclass_of) => Type::from(subclass_of.subclass_of())
+                .any_over_type_impl(db, type_fn, recursive_fallback, seen_types),
+
             Self::TypeVar(typevar) => match typevar.bound_or_constraints(db) {
                 None => false,
                 Some(TypeVarBoundOrConstraints::UpperBound(bound)) => {
-                    bound.any_over_type(db, type_fn)
+                    bound.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+                }
+                Some(TypeVarBoundOrConstraints::Constraints(constraints)) => {
+                    constraints.elements(db).iter().any(|constraint| {
+                        constraint.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+                    })
                 }
-                Some(TypeVarBoundOrConstraints::Constraints(constraints)) => constraints
-                    .elements(db)
-                    .iter()
-                    .any(|constraint| constraint.any_over_type(db, type_fn)),
             },
 
             Self::BoundSuper(bound_super) => {
-                Type::from(bound_super.pivot_class(db)).any_over_type(db, type_fn)
-                    || Type::from(bound_super.owner(db)).any_over_type(db, type_fn)
+                Type::from(bound_super.pivot_class(db)).any_over_type_impl(
+                    db,
+                    type_fn,
+                    recursive_fallback,
+                    seen_types,
+                ) || Type::from(bound_super.owner(db)).any_over_type_impl(
+                    db,
+                    type_fn,
+                    recursive_fallback,
+                    seen_types,
+                )
             }
 
             Self::Tuple(tuple) => tuple
                 .tuple(db)
                 .all_elements()
-                .any(|ty| ty.any_over_type(db, type_fn)),
+                .any(|ty| ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)),
 
             Self::Union(union) => union
                 .elements(db)
                 .iter()
-                .any(|ty| ty.any_over_type(db, type_fn)),
+                .any(|ty| ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)),
 
             Self::Intersection(intersection) => {
                 intersection
                     .positive(db)
                     .iter()
-                    .any(|ty| ty.any_over_type(db, type_fn))
-                    || intersection
-                        .negative(db)
-                        .iter()
-                        .any(|ty| ty.any_over_type(db, type_fn))
+                    .any(|ty| ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types))
+                    || intersection.negative(db).iter().any(|ty| {
+                        ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+                    })
             }
 
-            Self::ProtocolInstance(protocol) => protocol.any_over_type(db, type_fn),
-            Self::PropertyInstance(property) => property.any_over_type(db, type_fn),
+            Self::ProtocolInstance(protocol) => {
+                protocol.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            }
+            Self::PropertyInstance(property) => {
+                property.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            }
 
             Self::NominalInstance(instance) => match instance.class {
                 ClassType::NonGeneric(_) => false,
-                ClassType::Generic(generic) => generic
-                    .specialization(db)
-                    .types(db)
-                    .iter()
-                    .any(|ty| ty.any_over_type(db, type_fn)),
+                ClassType::Generic(generic) => {
+                    generic.specialization(db).types(db).iter().any(|ty| {
+                        ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+                    })
+                }
             },
 
-            Self::TypeIs(type_is) => type_is.return_type(db).any_over_type(db, type_fn),
+            Self::TypeIs(type_is) => type_is.return_type(db).any_over_type_impl(
+                db,
+                type_fn,
+                recursive_fallback,
+                seen_types,
+            ),
         }
     }
 
@@ -7298,15 +7342,21 @@ impl<'db> CallableType<'db> {
                 .is_equivalent_to(db, other.signatures(db))
     }
 
-    fn any_over_type(self, db: &'db dyn Db, type_fn: &dyn Fn(Type<'db>) -> bool) -> bool {
+    fn any_over_type_impl(
+        self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+        seen_types: &mut FxIndexSet<Type<'db>>,
+    ) -> bool {
         self.signatures(db).iter().any(|signature| {
             signature.parameters().iter().any(|param| {
-                param
-                    .annotated_type()
-                    .is_some_and(|ty| ty.any_over_type(db, type_fn))
-            }) || signature
-                .return_ty
-                .is_some_and(|ty| ty.any_over_type(db, type_fn))
+                param.annotated_type().is_some_and(|ty| {
+                    ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+                })
+            }) || signature.return_ty.is_some_and(|ty| {
+                ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            })
         })
     }
 }

crates/ty_python_semantic/src/types/function.rs

@@ -1145,11 +1145,18 @@ impl KnownFunction {
                 let [Some(casted_type), Some(source_type)] = parameter_types else {
                     return None;
                 };
-                let contains_unknown_or_todo =
-                    |ty| matches!(ty, Type::Dynamic(dynamic) if dynamic != DynamicType::Any);
+
+                let contains_unknown_or_todo = |ty: Type<'db>| {
+                    ty.any_over_type(
+                        db,
+                        &|ty| matches!(ty, Type::Dynamic(dynamic) if dynamic != DynamicType::Any),
+                        false,
+                    )
+                };
+
                 if source_type.is_equivalent_to(db, *casted_type)
-                    && !casted_type.any_over_type(db, &|ty| contains_unknown_or_todo(ty))
-                    && !source_type.any_over_type(db, &|ty| contains_unknown_or_todo(ty))
+                    && !contains_unknown_or_todo(*casted_type)
+                    && !contains_unknown_or_todo(*source_type)
                 {
                     let builder = context.report_lint(&REDUNDANT_CAST, call_expression)?;
                     builder.into_diagnostic(format_args!(

crates/ty_python_semantic/src/types/instance.rs

@@ -7,7 +7,7 @@ use super::{ClassType, KnownClass, SubclassOfType, Type, TypeVarVariance};
 use crate::place::PlaceAndQualifiers;
 use crate::types::tuple::TupleType;
 use crate::types::{DynamicType, TypeMapping, TypeRelation, TypeVarInstance, TypeVisitor};
-use crate::{Db, FxOrderSet};
+use crate::{Db, FxIndexSet, FxOrderSet};
 
 pub(super) use synthesized_protocol::SynthesizedProtocolType;
 
@@ -230,12 +230,16 @@ impl<'db> ProtocolInstanceType<'db> {
     }
 
     /// Return `true` if the types of any of the members match the closure passed in.
-    pub(super) fn any_over_type(
+    pub(super) fn any_over_type_impl(
         self,
         db: &'db dyn Db,
         type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+        seen_types: &mut FxIndexSet<Type<'db>>,
     ) -> bool {
-        self.inner.interface(db).any_over_type(db, type_fn)
+        self.inner
+            .interface(db)
+            .any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
     }
 
     /// Return `true` if this protocol type has the given type relation to the protocol `other`.

crates/ty_python_semantic/src/types/protocol_class.rs

@@ -5,7 +5,7 @@ use itertools::Itertools;
 use ruff_python_ast::name::Name;
 
 use crate::{
-    Db, FxOrderSet,
+    Db, FxIndexSet, FxOrderSet,
     place::{Boundness, Place, PlaceAndQualifiers, place_from_bindings, place_from_declarations},
     semantic_index::{place_table, use_def_map},
     types::{
@@ -153,13 +153,15 @@ impl<'db> ProtocolInterface<'db> {
     }
 
     /// Return `true` if the types of any of the members match the closure passed in.
-    pub(super) fn any_over_type(
+    pub(super) fn any_over_type_impl(
         self,
         db: &'db dyn Db,
         type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+        seen_types: &mut FxIndexSet<Type<'db>>,
     ) -> bool {
         self.members(db)
-            .any(|member| member.any_over_type(db, type_fn))
+            .any(|member| member.any_over_type_impl(db, type_fn, recursive_fallback, seen_types))
     }
 
     pub(super) fn normalized_impl(self, db: &'db dyn Db, visitor: &mut TypeVisitor<'db>) -> Self {
@@ -372,11 +374,23 @@ impl<'a, 'db> ProtocolMember<'a, 'db> {
         }
     }
 
-    fn any_over_type(&self, db: &'db dyn Db, type_fn: &dyn Fn(Type<'db>) -> bool) -> bool {
+    fn any_over_type_impl(
+        &self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+        recursive_fallback: bool,
+        seen_types: &mut FxIndexSet<Type<'db>>,
+    ) -> bool {
         match &self.kind {
-            ProtocolMemberKind::Method(callable) => callable.any_over_type(db, type_fn),
-            ProtocolMemberKind::Property(property) => property.any_over_type(db, type_fn),
-            ProtocolMemberKind::Other(ty) => ty.any_over_type(db, type_fn),
+            ProtocolMemberKind::Method(callable) => {
+                callable.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            }
+            ProtocolMemberKind::Property(property) => {
+                property.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            }
+            ProtocolMemberKind::Other(ty) => {
+                ty.any_over_type_impl(db, type_fn, recursive_fallback, seen_types)
+            }
         }
     }
 }