Skip to content

Commit 9aa6330

Browse files
AlexWaygoodAlexWaygood
authored
[ty] Fix redundant-cast false positives when casting to Unknown (#18111)
1 parent b600ff1 commit 9aa6330

5 files changed

Lines changed: 92 additions & 39 deletions

File tree

‎crates/ty_python_semantic/resources/mdtest/directives/cast.md‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ readers of ty's output. For `Unknown` in particular, we may consider it differen
5656
of some opt-in diagnostics, as it indicates that the gradual type has come about due to an invalid
5757
annotation, missing annotation or missing type argument somewhere.
5858

59+
A cast from `Unknown` to `Todo` or `Any` is also not considered a "redundant cast", as this breaks
60+
the gradual guarantee and leads to cascading errors when an object is inferred as having type
61+
`Unknown` due to a missing import or similar.
62+
5963
```py
6064
from ty_extensions import Unknown
6165

@@ -66,5 +70,8 @@ def f(x: Any, y: Unknown, z: Any | str | int):
6670
b = cast(Any, y)
6771
reveal_type(b) # revealed: Any
6872

69-
c = cast(str | int | Any, z) # error: [redundant-cast]
73+
c = cast(Unknown, y)
74+
reveal_type(c) # revealed: Unknown
75+
76+
d = cast(str | int | Any, z) # error: [redundant-cast]
7077
```

‎crates/ty_python_semantic/src/types.rs‎

Lines changed: 62 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -657,25 +657,26 @@ impl<'db> Type<'db> {
657657
}
658658
}
659659

660-
pub fn contains_todo(&self, db: &'db dyn Db) -> bool {
661-
match self {
662-
Self::Dynamic(DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec) => true,
660+
/// Return `true` if `self`, or any of the types contained in `self`, match the closure passed in.
661+
pub fn any_over_type(self, db: &'db dyn Db, type_fn: &dyn Fn(Type<'db>) -> bool) -> bool {
662+
if type_fn(self) {
663+
return true;
664+
}
663665

666+
match self {
664667
Self::AlwaysFalsy
665668
| Self::AlwaysTruthy
666669
| Self::Never
667670
| Self::BooleanLiteral(_)
668671
| Self::BytesLiteral(_)
669-
| Self::FunctionLiteral(_)
670-
| Self::NominalInstance(_)
671672
| Self::ModuleLiteral(_)
673+
| Self::FunctionLiteral(_)
672674
| Self::ClassLiteral(_)
673675
| Self::KnownInstance(_)
674-
| Self::PropertyInstance(_)
675676
| Self::StringLiteral(_)
676677
| Self::IntLiteral(_)
677678
| Self::LiteralString
678-
| Self::Dynamic(DynamicType::Unknown | DynamicType::Any)
679+
| Self::Dynamic(_)
679680
| Self::BoundMethod(_)
680681
| Self::WrapperDescriptor(_)
681682
| Self::MethodWrapper(_)
@@ -686,62 +687,82 @@ impl<'db> Type<'db> {
686687
.specialization(db)
687688
.types(db)
688689
.iter()
689-
.any(|ty| ty.contains_todo(db)),
690+
.copied()
691+
.any(|ty| ty.any_over_type(db, type_fn)),
690692

691693
Self::Callable(callable) => {
692694
let signatures = callable.signatures(db);
693695
signatures.iter().any(|signature| {
694696
signature.parameters().iter().any(|param| {
695697
param
696698
.annotated_type()
697-
.is_some_and(|ty| ty.contains_todo(db))
698-
}) || signature.return_ty.is_some_and(|ty| ty.contains_todo(db))
699+
.is_some_and(|ty| ty.any_over_type(db, type_fn))
700+
}) || signature
701+
.return_ty
702+
.is_some_and(|ty| ty.any_over_type(db, type_fn))
699703
})
700704
}
701705

702-
Self::SubclassOf(subclass_of) => match subclass_of.subclass_of() {
703-
SubclassOfInner::Dynamic(
704-
DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec,
705-
) => true,
706-
SubclassOfInner::Dynamic(DynamicType::Unknown | DynamicType::Any) => false,
707-
SubclassOfInner::Class(_) => false,
708-
},
706+
Self::SubclassOf(subclass_of) => {
707+
Type::from(subclass_of.subclass_of()).any_over_type(db, type_fn)
708+
}
709709

710710
Self::TypeVar(typevar) => match typevar.bound_or_constraints(db) {
711711
None => false,
712-
Some(TypeVarBoundOrConstraints::UpperBound(bound)) => bound.contains_todo(db),
712+
Some(TypeVarBoundOrConstraints::UpperBound(bound)) => {
713+
bound.any_over_type(db, type_fn)
714+
}
713715
Some(TypeVarBoundOrConstraints::Constraints(constraints)) => constraints
714716
.elements(db)
715717
.iter()
716-
.any(|constraint| constraint.contains_todo(db)),
718+
.any(|constraint| constraint.any_over_type(db, type_fn)),
717719
},
718720

719721
Self::BoundSuper(bound_super) => {
720-
matches!(
721-
bound_super.pivot_class(db),
722-
ClassBase::Dynamic(DynamicType::Todo(_))
723-
) || matches!(
724-
bound_super.owner(db),
725-
SuperOwnerKind::Dynamic(DynamicType::Todo(_))
726-
)
722+
Type::from(bound_super.pivot_class(db)).any_over_type(db, type_fn)
723+
|| Type::from(bound_super.owner(db)).any_over_type(db, type_fn)
727724
}
728725

729-
Self::Tuple(tuple) => tuple.elements(db).iter().any(|ty| ty.contains_todo(db)),
726+
Self::Tuple(tuple) => tuple
727+
.elements(db)
728+
.iter()
729+
.any(|ty| ty.any_over_type(db, type_fn)),
730730

731-
Self::Union(union) => union.elements(db).iter().any(|ty| ty.contains_todo(db)),
731+
Self::Union(union) => union
732+
.elements(db)
733+
.iter()
734+
.any(|ty| ty.any_over_type(db, type_fn)),
732735

733736
Self::Intersection(intersection) => {
734737
intersection
735738
.positive(db)
736739
.iter()
737-
.any(|ty| ty.contains_todo(db))
740+
.any(|ty| ty.any_over_type(db, type_fn))
738741
|| intersection
739742
.negative(db)
740743
.iter()
741-
.any(|ty| ty.contains_todo(db))
744+
.any(|ty| ty.any_over_type(db, type_fn))
745+
}
746+
747+
Self::ProtocolInstance(protocol) => protocol.any_over_type(db, type_fn),
748+
749+
Self::PropertyInstance(property) => {
750+
property
751+
.getter(db)
752+
.is_some_and(|ty| ty.any_over_type(db, type_fn))
753+
|| property
754+
.setter(db)
755+
.is_some_and(|ty| ty.any_over_type(db, type_fn))
742756
}
743757

744-
Self::ProtocolInstance(protocol) => protocol.contains_todo(db),
758+
Self::NominalInstance(instance) => match instance.class {
759+
ClassType::NonGeneric(_) => false,
760+
ClassType::Generic(generic) => generic
761+
.specialization(db)
762+
.types(db)
763+
.iter()
764+
.any(|ty| ty.any_over_type(db, type_fn)),
765+
},
745766
}
746767
}
747768

@@ -8172,6 +8193,16 @@ impl<'db> SuperOwnerKind<'db> {
81728193
}
81738194
}
81748195

8196+
impl<'db> From<SuperOwnerKind<'db>> for Type<'db> {
8197+
fn from(owner: SuperOwnerKind<'db>) -> Self {
8198+
match owner {
8199+
SuperOwnerKind::Dynamic(dynamic) => Type::Dynamic(dynamic),
8200+
SuperOwnerKind::Class(class) => class.into(),
8201+
SuperOwnerKind::Instance(instance) => instance.into(),
8202+
}
8203+
}
8204+
}
8205+
81758206
/// Represent a bound super object like `super(PivotClass, owner)`
81768207
#[salsa::interned(debug)]
81778208
pub struct BoundSuperType<'db> {

‎crates/ty_python_semantic/src/types/infer.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5051,7 +5051,13 @@ impl<'db> TypeInferenceBuilder<'db> {
50515051
if (source_type.is_equivalent_to(db, *casted_type)
50525052
|| source_type.normalized(db)
50535053
== casted_type.normalized(db))
5054-
&& !source_type.contains_todo(db)
5054+
&& !source_type.any_over_type(db, &|ty| {
5055+
matches!(
5056+
ty,
5057+
Type::Dynamic(dynamic)
5058+
if dynamic != DynamicType::Any
5059+
)
5060+
})
50555061
{
50565062
if let Some(builder) = self
50575063
.context

‎crates/ty_python_semantic/src/types/instance.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,13 @@ impl<'db> ProtocolInstanceType<'db> {
237237
}
238238
}
239239

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)
243247
}
244248

245249
/// Return `true` if this protocol type is fully static.

‎crates/ty_python_semantic/src/types/protocol_class.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,14 @@ impl<'db> ProtocolInterface<'db> {
149149
}
150150
}
151151

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))
155160
}
156161

157162
pub(super) fn normalized(self, db: &'db dyn Db) -> Self {

0 commit comments

Comments
 (0)
⚡