diff --git a/crates/ty_python_semantic/resources/corpus/invalid_typevar_constraints.py b/crates/ty_python_semantic/resources/corpus/invalid_typevar_constraints.py new file mode 100644 index 0000000000..14a79363e6 --- /dev/null +++ b/crates/ty_python_semantic/resources/corpus/invalid_typevar_constraints.py @@ -0,0 +1,6 @@ +class C[T: (A, B)]: + def f(foo: T): + try: + pass + except foo: + pass diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 8beb1cf68a..61cc160769 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -68,7 +68,7 @@ pub(crate) use crate::types::narrow::infer_narrowing_constraint; use crate::types::newtype::NewType; pub(crate) use crate::types::signatures::{Parameter, Parameters}; use crate::types::signatures::{ParameterForm, walk_signature}; -use crate::types::tuple::{TupleSpec, TupleSpecBuilder}; +use crate::types::tuple::{Tuple, TupleSpec, TupleSpecBuilder}; pub(crate) use crate::types::typed_dict::{TypedDictParams, TypedDictType, walk_typed_dict_type}; pub use crate::types::variance::TypeVarVariance; use crate::types::variance::VarianceInferable; @@ -5401,9 +5401,9 @@ impl<'db> Type<'db> { Some(TypeVarBoundOrConstraints::UpperBound(bound)) => { bound.try_bool_impl(db, allow_short_circuit, visitor)? } - Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { - try_union(constraints)? - } + Some(TypeVarBoundOrConstraints::Constraints(constraints)) => constraints + .as_type(db) + .try_bool_impl(db, allow_short_circuit, visitor)?, } } @@ -6453,7 +6453,7 @@ impl<'db> Type<'db> { TypeVarBoundOrConstraints::UpperBound(bound) => { non_async_special_case(db, bound) } - TypeVarBoundOrConstraints::Constraints(union) => non_async_special_case(db, Type::Union(union)), + TypeVarBoundOrConstraints::Constraints(constraints) => non_async_special_case(db, constraints.as_type(db)), }, Type::Union(union) => { let elements = union.elements(db); @@ -9594,7 +9594,7 @@ impl<'db> TypeVarInstance<'db> { TypeVarBoundOrConstraints::UpperBound(upper_bound.to_instance(db)?) } TypeVarBoundOrConstraints::Constraints(constraints) => { - TypeVarBoundOrConstraints::Constraints(constraints.to_instance(db)?.as_union()?) + TypeVarBoundOrConstraints::Constraints(constraints.to_instance(db)?) } }; let identity = TypeVarIdentity::new( @@ -9645,22 +9645,30 @@ impl<'db> TypeVarInstance<'db> { fn lazy_constraints(self, db: &'db dyn Db) -> Option> { let definition = self.definition(db)?; let module = parsed_module(db, definition.file(db)).load(db); - let ty = match definition.kind(db) { + let constraints = match definition.kind(db) { // PEP 695 typevar DefinitionKind::TypeVar(typevar) => { let typevar_node = typevar.node(&module); - definition_expression_type(db, definition, typevar_node.bound.as_ref()?) - .as_union()? + let bound = + definition_expression_type(db, definition, typevar_node.bound.as_ref()?); + let constraints = if let Some(tuple) = bound + .as_nominal_instance() + .and_then(|instance| instance.tuple_spec(db)) + { + if let Tuple::Fixed(tuple) = tuple.into_owned() { + tuple.owned_elements() + } else { + vec![Type::unknown()].into_boxed_slice() + } + } else { + vec![Type::unknown()].into_boxed_slice() + }; + TypeVarConstraints::new(db, constraints) } // legacy typevar DefinitionKind::Assignment(assignment) => { let call_expr = assignment.value(&module).as_call_expr()?; - // We don't use `UnionType::from_elements` or `UnionBuilder` here, - // because we don't want to simplify the list of constraints as we would with - // an actual union type. - // TODO: We probably shouldn't use `UnionType` to store these at all? TypeVar - // constraints are not a union. - UnionType::new( + TypeVarConstraints::new( db, call_expr .arguments @@ -9669,12 +9677,11 @@ impl<'db> TypeVarInstance<'db> { .skip(1) .map(|arg| definition_expression_type(db, definition, arg)) .collect::>(), - RecursivelyDefined::No, ) } _ => return None, }; - Some(TypeVarBoundOrConstraints::Constraints(ty)) + Some(TypeVarBoundOrConstraints::Constraints(constraints)) } #[salsa::tracked(cycle_fn=lazy_default_cycle_recover, cycle_initial=lazy_default_cycle_initial, heap_size=ruff_memory_usage::heap_size)] @@ -10086,10 +10093,133 @@ impl<'db> From> for TypeVarBoundOrConstraintsEval } } +/// Type variable constraints (e.g. `T: (int, str)`). +/// This is structurally identical to [`UnionType`], except that it does not perform simplification and preserves the element types. +#[salsa::interned(debug, heap_size=ruff_memory_usage::heap_size)] +pub struct TypeVarConstraints<'db> { + #[returns(ref)] + elements: Box<[Type<'db>]>, +} + +impl get_size2::GetSize for TypeVarConstraints<'_> {} + +fn walk_type_var_constraints<'db, V: visitor::TypeVisitor<'db> + ?Sized>( + db: &'db dyn Db, + constraints: TypeVarConstraints<'db>, + visitor: &V, +) { + for ty in constraints.elements(db) { + visitor.visit_type(db, *ty); + } +} + +impl<'db> TypeVarConstraints<'db> { + fn as_type(self, db: &'db dyn Db) -> Type<'db> { + let mut builder = UnionBuilder::new(db); + for ty in self.elements(db) { + builder = builder.add(*ty); + } + builder.build() + } + + fn to_instance(self, db: &'db dyn Db) -> Option> { + let mut instance_elements = Vec::new(); + for ty in self.elements(db) { + instance_elements.push(ty.to_instance(db)?); + } + Some(TypeVarConstraints::new( + db, + instance_elements.into_boxed_slice(), + )) + } + + fn map(self, db: &'db dyn Db, transform_fn: impl FnMut(&Type<'db>) -> Type<'db>) -> Self { + let mapped = self + .elements(db) + .iter() + .map(transform_fn) + .collect::>(); + TypeVarConstraints::new(db, mapped) + } + + pub(crate) fn map_with_boundness_and_qualifiers( + self, + db: &'db dyn Db, + mut transform_fn: impl FnMut(&Type<'db>) -> PlaceAndQualifiers<'db>, + ) -> PlaceAndQualifiers<'db> { + let mut builder = UnionBuilder::new(db); + let mut qualifiers = TypeQualifiers::empty(); + + let mut all_unbound = true; + let mut possibly_unbound = false; + let mut origin = TypeOrigin::Declared; + for ty in self.elements(db) { + let PlaceAndQualifiers { + place: ty_member, + qualifiers: new_qualifiers, + } = transform_fn(ty); + qualifiers |= new_qualifiers; + match ty_member { + Place::Undefined => { + possibly_unbound = true; + } + Place::Defined(ty_member, member_origin, member_boundness) => { + origin = origin.merge(member_origin); + if member_boundness == Definedness::PossiblyUndefined { + possibly_unbound = true; + } + + all_unbound = false; + builder = builder.add(ty_member); + } + } + } + PlaceAndQualifiers { + place: if all_unbound { + Place::Undefined + } else { + Place::Defined( + builder.build(), + origin, + if possibly_unbound { + Definedness::PossiblyUndefined + } else { + Definedness::AlwaysDefined + }, + ) + }, + qualifiers, + } + } + + fn normalized_impl(self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self { + let normalized = self + .elements(db) + .iter() + .map(|ty| ty.normalized_impl(db, visitor)) + .collect::>(); + TypeVarConstraints::new(db, normalized) + } + + fn materialize_impl( + self, + db: &'db dyn Db, + materialization_kind: MaterializationKind, + visitor: &ApplyTypeMappingVisitor<'db>, + ) -> Self { + let materialized = self + .elements(db) + .iter() + .map(|ty| ty.materialize(db, materialization_kind, visitor)) + .collect::>(); + TypeVarConstraints::new(db, materialized) + } +} + #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, salsa::Update, get_size2::GetSize)] pub enum TypeVarBoundOrConstraints<'db> { UpperBound(Type<'db>), - Constraints(UnionType<'db>), + Constraints(TypeVarConstraints<'db>), } fn walk_type_var_bounds<'db, V: visitor::TypeVisitor<'db> + ?Sized>( @@ -10100,7 +10230,7 @@ fn walk_type_var_bounds<'db, V: visitor::TypeVisitor<'db> + ?Sized>( match bounds { TypeVarBoundOrConstraints::UpperBound(bound) => visitor.visit_type(db, bound), TypeVarBoundOrConstraints::Constraints(constraints) => { - visitor.visit_union_type(db, constraints); + walk_type_var_constraints(db, constraints, visitor); } } } @@ -10112,18 +10242,7 @@ impl<'db> TypeVarBoundOrConstraints<'db> { TypeVarBoundOrConstraints::UpperBound(bound.normalized_impl(db, visitor)) } TypeVarBoundOrConstraints::Constraints(constraints) => { - // Constraints are a non-normalized union by design (it's not really a union at - // all, we are just using a union to store the types). Normalize the types but not - // the containing union. - TypeVarBoundOrConstraints::Constraints(UnionType::new( - db, - constraints - .elements(db) - .iter() - .map(|ty| ty.normalized_impl(db, visitor)) - .collect::>(), - constraints.recursively_defined(db), - )) + TypeVarBoundOrConstraints::Constraints(constraints.normalized_impl(db, visitor)) } } } @@ -10147,14 +10266,13 @@ impl<'db> TypeVarBoundOrConstraints<'db> { // Normalize each constraint with its corresponding previous constraint let current_elements = constraints.elements(db); let prev_elements = prev_constraints.elements(db); - TypeVarBoundOrConstraints::Constraints(UnionType::new( + TypeVarBoundOrConstraints::Constraints(TypeVarConstraints::new( db, current_elements .iter() .zip(prev_elements.iter()) .map(|(ty, prev_ty)| ty.cycle_normalized(db, *prev_ty, cycle)) .collect::>(), - constraints.recursively_defined(db), )) } // The choice of whether it's an upper bound or constraints is purely syntactic and @@ -10175,15 +10293,9 @@ impl<'db> TypeVarBoundOrConstraints<'db> { TypeVarBoundOrConstraints::UpperBound(bound.recursive_type_normalized(db, cycle)) } TypeVarBoundOrConstraints::Constraints(constraints) => { - TypeVarBoundOrConstraints::Constraints(UnionType::new( - db, - constraints - .elements(db) - .iter() - .map(|ty| ty.recursive_type_normalized(db, cycle)) - .collect::>(), - constraints.recursively_defined(db), - )) + TypeVarBoundOrConstraints::Constraints( + constraints.map(db, |ty| ty.recursive_type_normalized(db, cycle)), + ) } } } @@ -10199,14 +10311,10 @@ impl<'db> TypeVarBoundOrConstraints<'db> { bound.materialize(db, materialization_kind, visitor), ), TypeVarBoundOrConstraints::Constraints(constraints) => { - TypeVarBoundOrConstraints::Constraints(UnionType::new( + TypeVarBoundOrConstraints::Constraints(constraints.materialize_impl( db, - constraints - .elements(db) - .iter() - .map(|ty| ty.materialize(db, materialization_kind, visitor)) - .collect::>(), - RecursivelyDefined::No, + materialization_kind, + visitor, )) } } diff --git a/crates/ty_python_semantic/src/types/bound_super.rs b/crates/ty_python_semantic/src/types/bound_super.rs index c67aacb323..442ae0d0b9 100644 --- a/crates/ty_python_semantic/src/types/bound_super.rs +++ b/crates/ty_python_semantic/src/types/bound_super.rs @@ -157,7 +157,7 @@ impl<'db> BoundSuperError<'db> { .map(|c| c.display(db)) .join(", ") )); - Type::Union(constraints) + constraints.as_type(db) } None => { diagnostic.info(format_args!( @@ -374,7 +374,7 @@ impl<'db> BoundSuperType<'db> { delegate_with_error_mapped(bound, Some(type_var)) } Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { - delegate_with_error_mapped(Type::Union(constraints), Some(type_var)) + delegate_with_error_mapped(constraints.as_type(db), Some(type_var)) } None => delegate_with_error_mapped(Type::object(), Some(type_var)), }; diff --git a/crates/ty_python_semantic/src/types/builder.rs b/crates/ty_python_semantic/src/types/builder.rs index 64ca36010a..36977078e9 100644 --- a/crates/ty_python_semantic/src/types/builder.rs +++ b/crates/ty_python_semantic/src/types/builder.rs @@ -1255,7 +1255,7 @@ impl<'db> InnerIntersectionBuilder<'db> { speculative = speculative.add_positive(bound); } Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { - speculative = speculative.add_positive(Type::Union(constraints)); + speculative = speculative.add_positive(constraints.as_type(db)); } // TypeVars without a bound or constraint implicitly have `object` as their // upper bound, and it is always a no-op to add `object` to an intersection. diff --git a/crates/ty_python_semantic/src/types/infer/builder.rs b/crates/ty_python_semantic/src/types/infer/builder.rs index d308553801..b33883d234 100644 --- a/crates/ty_python_semantic/src/types/infer/builder.rs +++ b/crates/ty_python_semantic/src/types/infer/builder.rs @@ -50,7 +50,6 @@ use crate::semantic_index::{ ApplicableConstraints, EnclosingSnapshotResult, SemanticIndex, place_table, }; use crate::subscript::{PyIndex, PySlice}; -use crate::types::builder::RecursivelyDefined; use crate::types::call::bind::{CallableDescription, MatchingOverloadIndex}; use crate::types::call::{Binding, Bindings, CallArguments, CallError, CallErrorKind}; use crate::types::class::{CodeGeneratorKind, FieldKind, MetaclassErrorKind, MethodDecorator}; @@ -3274,19 +3273,14 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { std::mem::replace(&mut self.deferred_state, DeferredExpressionState::Deferred); match bound.as_deref() { Some(expr @ ast::Expr::Tuple(ast::ExprTuple { elts, .. })) => { - // We don't use UnionType::from_elements or UnionBuilder here, because we don't - // want to simplify the list of constraints like we do with the elements of an - // actual union type. - // TODO: Consider using a new `OneOfType` connective here instead, since that - // more accurately represents the actual semantics of typevar constraints. - let ty = Type::Union(UnionType::new( + // Here, we interpret `bound` as a heterogeneous tuple and convert it to `TypeVarConstraints` in `TypeVarInstance::lazy_constraints`. + let tuple_ty = Type::heterogeneous_tuple( self.db(), elts.iter() .map(|expr| self.infer_type_expression(expr)) .collect::>(), - RecursivelyDefined::No, - )); - self.store_expression_type(expr, ty); + ); + self.store_expression_type(expr, tuple_ty); } Some(expr) => { self.infer_type_expression(expr); @@ -11521,7 +11515,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> { if provided_type .when_assignable_to( db, - Type::Union(constraints), + constraints.as_type(db), InferableTypeVars::None, ) .is_never_satisfied(db) diff --git a/crates/ty_python_semantic/src/types/narrow.rs b/crates/ty_python_semantic/src/types/narrow.rs index 984f214414..4aa8b85b6f 100644 --- a/crates/ty_python_semantic/src/types/narrow.rs +++ b/crates/ty_python_semantic/src/types/narrow.rs @@ -198,7 +198,7 @@ impl ClassInfoConstraintFunction { self.generate_constraint(db, bound) } TypeVarBoundOrConstraints::Constraints(constraints) => { - self.generate_constraint(db, Type::Union(constraints)) + self.generate_constraint(db, constraints.as_type(db)) } } } diff --git a/crates/ty_python_semantic/src/types/subclass_of.rs b/crates/ty_python_semantic/src/types/subclass_of.rs index 4d15700163..898a82e086 100644 --- a/crates/ty_python_semantic/src/types/subclass_of.rs +++ b/crates/ty_python_semantic/src/types/subclass_of.rs @@ -8,7 +8,7 @@ use crate::types::{ ApplyTypeMappingVisitor, BoundTypeVarInstance, ClassType, DynamicType, FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsDisjointVisitor, KnownClass, MaterializationKind, MemberLookupPolicy, NormalizedVisitor, SpecialFormType, Type, TypeContext, - TypeMapping, TypeRelation, TypeVarBoundOrConstraints, UnionType, todo_type, + TypeMapping, TypeRelation, TypeVarBoundOrConstraints, todo_type, }; use crate::{Db, FxOrderSet}; @@ -190,7 +190,9 @@ impl<'db> SubclassOfType<'db> { match bound_typevar.typevar(db).bound_or_constraints(db) { None => unreachable!(), Some(TypeVarBoundOrConstraints::UpperBound(bound)) => bound, - Some(TypeVarBoundOrConstraints::Constraints(union)) => Type::Union(union), + Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { + constraints.as_type(db) + } } } }; @@ -351,7 +353,7 @@ impl<'db> SubclassOfInner<'db> { .and_then(|subclass_of| subclass_of.into_class(db)) } Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { - match constraints.elements(db) { + match &**constraints.elements(db) { [bound] => Self::try_from_instance(db, *bound) .and_then(|subclass_of| subclass_of.into_class(db)), _ => Some(ClassType::object(db)), @@ -416,20 +418,10 @@ impl<'db> SubclassOfInner<'db> { ) } Some(TypeVarBoundOrConstraints::Constraints(constraints)) => { - let constraints_types = constraints - .elements(db) - .iter() - .map(|constraint| { - SubclassOfType::try_from_instance(db, *constraint) - .unwrap_or(SubclassOfType::subclass_of_unknown()) - }) - .collect::>(); - - TypeVarBoundOrConstraints::Constraints(UnionType::new( - db, - constraints_types, - constraints.recursively_defined(db), - )) + TypeVarBoundOrConstraints::Constraints(constraints.map(db, |constraint| { + SubclassOfType::try_from_instance(db, *constraint) + .unwrap_or(SubclassOfType::subclass_of_unknown()) + })) } }) }); diff --git a/crates/ty_python_semantic/src/types/tuple.rs b/crates/ty_python_semantic/src/types/tuple.rs index 6405ae3ae7..787bbe0688 100644 --- a/crates/ty_python_semantic/src/types/tuple.rs +++ b/crates/ty_python_semantic/src/types/tuple.rs @@ -349,6 +349,10 @@ impl FixedLengthTuple { &self.0 } + pub(crate) fn owned_elements(self) -> Box<[T]> { + self.0 + } + pub(crate) fn elements(&self) -> impl DoubleEndedIterator + ExactSizeIterator + '_ { self.0.iter() }