From 40148d7b118adfe435b561c971f00fa7e89623bb Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 22 Oct 2025 12:07:01 +0100 Subject: [PATCH] [ty] Add assertions to ensure that we never call `KnownClass::Tuple.to_instance()` or similar (#21027) --- crates/ty_python_semantic/src/types/class.rs | 54 +++++++++++++++---- .../src/types/infer/builder.rs | 2 +- .../ty_python_semantic/src/types/instance.rs | 25 +++------ 3 files changed, 53 insertions(+), 28 deletions(-) diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 56df9329ed..c7b96cfa7e 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -4614,7 +4614,13 @@ impl KnownClass { /// the class. If this class is generic, this will use the default specialization. /// /// If the class cannot be found in typeshed, a debug-level log message will be emitted stating this. + #[track_caller] pub(crate) fn to_instance(self, db: &dyn Db) -> Type<'_> { + debug_assert_ne!( + self, + KnownClass::Tuple, + "Use `Type::heterogeneous_tuple` or `Type::homogeneous_tuple` to create `tuple` instances" + ); self.to_class_literal(db) .to_class_type(db) .map(|class| Type::instance(db, class)) @@ -4623,7 +4629,13 @@ impl KnownClass { /// Similar to [`KnownClass::to_instance`], but returns the Unknown-specialization where each type /// parameter is specialized to `Unknown`. + #[track_caller] pub(crate) fn to_instance_unknown(self, db: &dyn Db) -> Type<'_> { + debug_assert_ne!( + self, + KnownClass::Tuple, + "Use `Type::heterogeneous_tuple` or `Type::homogeneous_tuple` to create `tuple` instances" + ); self.try_to_class_literal(db) .map(|literal| Type::instance(db, literal.unknown_specialization(db))) .unwrap_or_else(Type::unknown) @@ -4667,11 +4679,17 @@ impl KnownClass { /// /// If the class cannot be found in typeshed, or if you provide a specialization with the wrong /// number of types, a debug-level log message will be emitted stating this. + #[track_caller] pub(crate) fn to_specialized_instance<'db>( self, db: &'db dyn Db, specialization: impl IntoIterator>, ) -> Type<'db> { + debug_assert_ne!( + self, + KnownClass::Tuple, + "Use `Type::heterogeneous_tuple` or `Type::homogeneous_tuple` to create `tuple` instances" + ); self.to_specialized_class_type(db, specialization) .and_then(|class_type| Type::from(class_type).to_instance(db)) .unwrap_or_else(Type::unknown) @@ -5566,11 +5584,19 @@ mod tests { }); for class in KnownClass::iter() { - assert_ne!( - class.to_instance(&db), - Type::unknown(), - "Unexpectedly fell back to `Unknown` for `{class:?}`" - ); + // Check the class can be looked up successfully + class.try_to_class_literal_without_logging(&db).unwrap(); + + // We can't call `KnownClass::Tuple.to_instance()`; + // there are assertions to ensure that we always call `Type::homogeneous_tuple()` + // or `Type::heterogeneous_tuple()` instead.` + if class != KnownClass::Tuple { + assert_ne!( + class.to_instance(&db), + Type::unknown(), + "Unexpectedly fell back to `Unknown` for `{class:?}`" + ); + } } } @@ -5617,11 +5643,19 @@ mod tests { current_version = version_added; } - assert_ne!( - class.to_instance(&db), - Type::unknown(), - "Unexpectedly fell back to `Unknown` for `{class:?}` on Python {version_added}" - ); + // Check the class can be looked up successfully + class.try_to_class_literal_without_logging(&db).unwrap(); + + // We can't call `KnownClass::Tuple.to_instance()`; + // there are assertions to ensure that we always call `Type::homogeneous_tuple()` + // or `Type::heterogeneous_tuple()` instead.` + if class != KnownClass::Tuple { + assert_ne!( + class.to_instance(&db), + Type::unknown(), + "Unexpectedly fell back to `Unknown` for `{class:?}` on Python {version_added}" + ); + } } } } diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index d67a39dab0..238b95a4a8 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -5950,7 +5950,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { .unwrap_or(InferableTypeVars::None); annotation.filter_disjoint_elements( self.db(), - KnownClass::Tuple.to_instance(self.db()), + Type::homogeneous_tuple(self.db(), Type::unknown()), inferable, ) }); diff --git a/crates/ty_python_semantic/src/types/instance.rs b/crates/ty_python_semantic/src/types/instance.rs index ac081ca02a..c294cde618 100644 --- a/crates/ty_python_semantic/src/types/instance.rs +++ b/crates/ty_python_semantic/src/types/instance.rs @@ -85,16 +85,6 @@ impl<'db> Type<'db> { Type::NominalInstance(NominalInstanceType(NominalInstanceInner::ExactTuple(tuple))) } - /// **Private** helper function to create a `Type::NominalInstance` from a class that - /// is known not to be `Any`, a protocol class, or a typed dict class. - fn non_tuple_instance(db: &'db dyn Db, class: ClassType<'db>) -> Self { - if class.is_known(db, KnownClass::Object) { - Type::NominalInstance(NominalInstanceType(NominalInstanceInner::Object)) - } else { - Type::NominalInstance(NominalInstanceType(NominalInstanceInner::NonTuple(class))) - } - } - pub(crate) const fn into_nominal_instance(self) -> Option> { match self { Type::NominalInstance(instance_type) => Some(instance_type), @@ -353,9 +343,9 @@ impl<'db> NominalInstanceType<'db> { NominalInstanceInner::ExactTuple(tuple) => { Type::tuple(tuple.normalized_impl(db, visitor)) } - NominalInstanceInner::NonTuple(class) => { - Type::non_tuple_instance(db, class.normalized_impl(db, visitor)) - } + NominalInstanceInner::NonTuple(class) => Type::NominalInstance(NominalInstanceType( + NominalInstanceInner::NonTuple(class.normalized_impl(db, visitor)), + )), NominalInstanceInner::Object => Type::object(), } } @@ -488,10 +478,11 @@ impl<'db> NominalInstanceType<'db> { NominalInstanceInner::ExactTuple(tuple) => { Type::tuple(tuple.apply_type_mapping_impl(db, type_mapping, tcx, visitor)) } - NominalInstanceInner::NonTuple(class) => Type::non_tuple_instance( - db, - class.apply_type_mapping_impl(db, type_mapping, tcx, visitor), - ), + NominalInstanceInner::NonTuple(class) => { + Type::NominalInstance(NominalInstanceType(NominalInstanceInner::NonTuple( + class.apply_type_mapping_impl(db, type_mapping, tcx, visitor), + ))) + } NominalInstanceInner::Object => Type::object(), } }