[ty] Fix redundant-cast false positives when casting to Unknown (#18111)

AlexWaygood · web-flow · commit 9aa6330bb143 · 2025-05-14T22:38:53.000-04:00
diff --git a/crates/ty_python_semantic/resources/mdtest/directives/cast.md b/crates/ty_python_semantic/resources/mdtest/directives/cast.md
@@ -56,6 +56,10 @@ readers of ty's output. For `Unknown` in particular, we may consider it differen
 of some opt-in diagnostics, as it indicates that the gradual type has come about due to an invalid
 annotation, missing annotation or missing type argument somewhere.
 
+A cast from `Unknown` to `Todo` or `Any` is also not considered a "redundant cast", as this breaks
+the gradual guarantee and leads to cascading errors when an object is inferred as having type
+`Unknown` due to a missing import or similar.
+
 ```py
 from ty_extensions import Unknown
 
@@ -66,5 +70,8 @@ def f(x: Any, y: Unknown, z: Any | str | int):
     b = cast(Any, y)
     reveal_type(b)  # revealed: Any
 
-    c = cast(str | int | Any, z)  # error: [redundant-cast]
+    c = cast(Unknown, y)
+    reveal_type(c)  # revealed: Unknown
+
+    d = cast(str | int | Any, z)  # error: [redundant-cast]
 ```
diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs
@@ -657,25 +657,26 @@ impl<'db> Type<'db> {
         }
     }
 
-    pub fn contains_todo(&self, db: &'db dyn Db) -> bool {
-        match self {
-            Self::Dynamic(DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec) => true,
+    /// 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 {
+        if type_fn(self) {
+            return true;
+        }
 
+        match self {
             Self::AlwaysFalsy
             | Self::AlwaysTruthy
             | Self::Never
             | Self::BooleanLiteral(_)
             | Self::BytesLiteral(_)
-            | Self::FunctionLiteral(_)
-            | Self::NominalInstance(_)
             | Self::ModuleLiteral(_)
+            | Self::FunctionLiteral(_)
             | Self::ClassLiteral(_)
             | Self::KnownInstance(_)
-            | Self::PropertyInstance(_)
             | Self::StringLiteral(_)
             | Self::IntLiteral(_)
             | Self::LiteralString
-            | Self::Dynamic(DynamicType::Unknown | DynamicType::Any)
+            | Self::Dynamic(_)
             | Self::BoundMethod(_)
             | Self::WrapperDescriptor(_)
             | Self::MethodWrapper(_)
@@ -686,62 +687,82 @@ impl<'db> Type<'db> {
                 .specialization(db)
                 .types(db)
                 .iter()
-                .any(|ty| ty.contains_todo(db)),
+                .copied()
+                .any(|ty| ty.any_over_type(db, type_fn)),
 
             Self::Callable(callable) => {
                 let signatures = callable.signatures(db);
                 signatures.iter().any(|signature| {
                     signature.parameters().iter().any(|param| {
                         param
                             .annotated_type()
-                            .is_some_and(|ty| ty.contains_todo(db))
-                    }) || signature.return_ty.is_some_and(|ty| ty.contains_todo(db))
+                            .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))
                 })
             }
 
-            Self::SubclassOf(subclass_of) => match subclass_of.subclass_of() {
-                SubclassOfInner::Dynamic(
-                    DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec,
-                ) => true,
-                SubclassOfInner::Dynamic(DynamicType::Unknown | DynamicType::Any) => false,
-                SubclassOfInner::Class(_) => false,
-            },
+            Self::SubclassOf(subclass_of) => {
+                Type::from(subclass_of.subclass_of()).any_over_type(db, type_fn)
+            }
 
             Self::TypeVar(typevar) => match typevar.bound_or_constraints(db) {
                 None => false,
-                Some(TypeVarBoundOrConstraints::UpperBound(bound)) => bound.contains_todo(db),
+                Some(TypeVarBoundOrConstraints::UpperBound(bound)) => {
+                    bound.any_over_type(db, type_fn)
+                }
                 Some(TypeVarBoundOrConstraints::Constraints(constraints)) => constraints
                     .elements(db)
                     .iter()
-                    .any(|constraint| constraint.contains_todo(db)),
+                    .any(|constraint| constraint.any_over_type(db, type_fn)),
             },
 
             Self::BoundSuper(bound_super) => {
-                matches!(
-                    bound_super.pivot_class(db),
-                    ClassBase::Dynamic(DynamicType::Todo(_))
-                ) || matches!(
-                    bound_super.owner(db),
-                    SuperOwnerKind::Dynamic(DynamicType::Todo(_))
-                )
+                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)
             }
 
-            Self::Tuple(tuple) => tuple.elements(db).iter().any(|ty| ty.contains_todo(db)),
+            Self::Tuple(tuple) => tuple
+                .elements(db)
+                .iter()
+                .any(|ty| ty.any_over_type(db, type_fn)),
 
-            Self::Union(union) => union.elements(db).iter().any(|ty| ty.contains_todo(db)),
+            Self::Union(union) => union
+                .elements(db)
+                .iter()
+                .any(|ty| ty.any_over_type(db, type_fn)),
 
             Self::Intersection(intersection) => {
                 intersection
                     .positive(db)
                     .iter()
-                    .any(|ty| ty.contains_todo(db))
+                    .any(|ty| ty.any_over_type(db, type_fn))
                     || intersection
                         .negative(db)
                         .iter()
-                        .any(|ty| ty.contains_todo(db))
+                        .any(|ty| ty.any_over_type(db, type_fn))
+            }
+
+            Self::ProtocolInstance(protocol) => protocol.any_over_type(db, type_fn),
+
+            Self::PropertyInstance(property) => {
+                property
+                    .getter(db)
+                    .is_some_and(|ty| ty.any_over_type(db, type_fn))
+                    || property
+                        .setter(db)
+                        .is_some_and(|ty| ty.any_over_type(db, type_fn))
             }
 
-            Self::ProtocolInstance(protocol) => protocol.contains_todo(db),
+            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)),
+            },
         }
     }
 
@@ -8172,6 +8193,16 @@ impl<'db> SuperOwnerKind<'db> {
     }
 }
 
+impl<'db> From<SuperOwnerKind<'db>> for Type<'db> {
+    fn from(owner: SuperOwnerKind<'db>) -> Self {
+        match owner {
+            SuperOwnerKind::Dynamic(dynamic) => Type::Dynamic(dynamic),
+            SuperOwnerKind::Class(class) => class.into(),
+            SuperOwnerKind::Instance(instance) => instance.into(),
+        }
+    }
+}
+
 /// Represent a bound super object like `super(PivotClass, owner)`
 #[salsa::interned(debug)]
 pub struct BoundSuperType<'db> {
diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs
@@ -5051,7 +5051,13 @@ impl<'db> TypeInferenceBuilder<'db> {
                                             if (source_type.is_equivalent_to(db, *casted_type)
                                                 || source_type.normalized(db)
                                                     == casted_type.normalized(db))
-                                                && !source_type.contains_todo(db)
+                                                && !source_type.any_over_type(db, &|ty| {
+                                                    matches!(
+                                                        ty,
+                                                        Type::Dynamic(dynamic)
+                                                            if dynamic != DynamicType::Any
+                                                    )
+                                                })
                                             {
                                                 if let Some(builder) = self
                                                     .context
diff --git a/crates/ty_python_semantic/src/types/instance.rs b/crates/ty_python_semantic/src/types/instance.rs
@@ -237,9 +237,13 @@ impl<'db> ProtocolInstanceType<'db> {
         }
     }
 
-    /// Return `true` if any of the members of this protocol type contain any `Todo` types.
-    pub(super) fn contains_todo(self, db: &'db dyn Db) -> bool {
-        self.inner.interface(db).contains_todo(db)
+    /// Return `true` if the types of any of the members match the closure passed in.
+    pub(super) fn any_over_type(
+        self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+    ) -> bool {
+        self.inner.interface(db).any_over_type(db, type_fn)
     }
 
     /// Return `true` if this protocol type is fully static.
diff --git a/crates/ty_python_semantic/src/types/protocol_class.rs b/crates/ty_python_semantic/src/types/protocol_class.rs
@@ -149,9 +149,14 @@ impl<'db> ProtocolInterface<'db> {
         }
     }
 
-    /// Return `true` if any of the members of this protocol type contain any `Todo` types.
-    pub(super) fn contains_todo(self, db: &'db dyn Db) -> bool {
-        self.members(db).any(|member| member.ty.contains_todo(db))
+    /// Return `true` if the types of any of the members match the closure passed in.
+    pub(super) fn any_over_type(
+        self,
+        db: &'db dyn Db,
+        type_fn: &dyn Fn(Type<'db>) -> bool,
+    ) -> bool {
+        self.members(db)
+            .any(|member| member.ty.any_over_type(db, type_fn))
     }
 
     pub(super) fn normalized(self, db: &'db dyn Db) -> Self {

Original file line number	Diff line number	Diff line change
`@@ -237,9 +237,13 @@ impl<'db> ProtocolInstanceType<'db> {`
`237`	`237`	`}`
`238`	`238`	`}`
`239`	`239`
`240`		- /// Return `true` if any of the members of this protocol type contain any `Todo` types.
`241`		`- pub(super) fn contains_todo(self, db: &'db dyn Db) -> bool {`
`242`		`- self.inner.interface(db).contains_todo(db)`
	`240`	+ /// Return `true` if the types of any of the members match the closure passed in.
	`241`	`+ pub(super) fn any_over_type(`
	`242`	`+ self,`
	`243`	`+ db: &'db dyn Db,`
	`244`	`+ type_fn: &dyn Fn(Type<'db>) -> bool,`
	`245`	`+ ) -> bool {`
	`246`	`+ self.inner.interface(db).any_over_type(db, type_fn)`
`243`	`247`	`}`
`244`	`248`
`245`	`249`	/// Return `true` if this protocol type is fully static.
Original file line number	Diff line number	Diff line change
`@@ -149,9 +149,14 @@ impl<'db> ProtocolInterface<'db> {`
`149`	`149`	`}`
`150`	`150`	`}`
`151`	`151`
`152`		- /// Return `true` if any of the members of this protocol type contain any `Todo` types.
`153`		`- pub(super) fn contains_todo(self, db: &'db dyn Db) -> bool {`
`154`		`- self.members(db).any(\|member\| member.ty.contains_todo(db))`
	`152`	+ /// Return `true` if the types of any of the members match the closure passed in.
	`153`	`+ pub(super) fn any_over_type(`
	`154`	`+ self,`
	`155`	`+ db: &'db dyn Db,`
	`156`	`+ type_fn: &dyn Fn(Type<'db>) -> bool,`
	`157`	`+ ) -> bool {`
	`158`	`+ self.members(db)`
	`159`	`+ .any(\|member\| member.ty.any_over_type(db, type_fn))`
`155`	`160`	`}`
`156`	`161`
`157`	`162`	`pub(super) fn normalized(self, db: &'db dyn Db) -> Self {`