Skip to content

Commit 7da7ae0

Browse files
mtshibaibraheemdev
andauthored
[ty] disallow type variables within PEP-695 type variable bounds/constraints (#22982)
## Summary This is a revival of #21984. Legacy typevar checks were already supported in #22949. This PR addresses PEP-695 typevars. ## Test Plan mdtest updated --------- Co-authored-by: Ibraheem Ahmed <ibraheem@ibraheem.ca>
1 parent 77fdf99 commit 7da7ae0

4 files changed

Lines changed: 162 additions & 119 deletions

File tree

crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,14 +731,23 @@ reveal_type(WithOverloadedMethod[int].method)
731731

732732
### No back-references
733733

734-
Typevar bounds/constraints/defaults are lazy, but cannot refer to later typevars:
734+
Typevar bounds/constraints/defaults are lazy, but cannot refer to later typevars. Furthermore,
735+
bounds/constraints cannot refer to other type variables, i.e. they must be non-generic.
735736

736737
```py
737-
# TODO error
738+
# error: [invalid-type-variable-bound]
738739
class C[S: T, T]:
739740
pass
740741

741-
class D[S: X]:
742+
# error: [invalid-type-variable-bound]
743+
class D[S, T: S]:
744+
pass
745+
746+
# error: [invalid-type-variable-constraints]
747+
class E[S: (int, T), T]:
748+
pass
749+
750+
class F[S: X]:
742751
pass
743752

744753
X = int

crates/ty_python_semantic/resources/mdtest/generics/pep695/variables.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -957,45 +957,45 @@ A typevar's bounds and constraints cannot be generic, cyclic or otherwise:
957957
```py
958958
from typing import Any
959959

960-
# TODO: error
960+
# error: [invalid-type-variable-bound]
961961
def f[S, T: list[S]](x: S, y: T) -> S | T:
962962
return x or y
963963

964-
# TODO: error
964+
# error: [invalid-type-variable-bound]
965965
class C[S, T: list[S]]:
966966
x: S
967967
y: T
968968

969969
reveal_type(C[int, list[Any]]().x) # revealed: int
970970
reveal_type(C[int, list[Any]]().y) # revealed: list[Any]
971971

972-
# TODO: error
972+
# error: [invalid-type-variable-bound]
973973
def g[T: list[T]](x: T) -> T:
974974
return x
975975

976-
# TODO: error
976+
# error: [invalid-type-variable-bound]
977977
class D[T: list[T]]:
978978
x: T
979979

980980
reveal_type(D[list[Any]]().x) # revealed: list[Any]
981981

982-
# TODO: error
982+
# error: [invalid-type-variable-constraints]
983983
def h[S, T: (list[S], str)](x: S, y: T) -> S | T:
984984
return x or y
985985

986-
# TODO: error
986+
# error: [invalid-type-variable-constraints]
987987
class E[S, T: (list[S], str)]:
988988
x: S
989989
y: T
990990

991991
reveal_type(E[int, str]().x) # revealed: int
992992
reveal_type(E[int, str]().y) # revealed: str
993993

994-
# TODO: error
994+
# error: [invalid-type-variable-constraints]
995995
def i[T: (list[T], str)](x: T) -> T:
996996
return x
997997

998-
# TODO: error
998+
# error: [invalid-type-variable-constraints]
999999
class F[T: (list[T], str)]:
10001000
x: T
10011001

crates/ty_python_semantic/src/types.rs

Lines changed: 115 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,6 +1200,15 @@ impl<'db> Type<'db> {
12001200
})
12011201
}
12021202

1203+
pub(crate) fn has_typevar_or_typevar_instance(self, db: &'db dyn Db) -> bool {
1204+
any_over_type(db, self, false, |ty| {
1205+
matches!(
1206+
ty,
1207+
Type::KnownInstance(KnownInstanceType::TypeVar(_)) | Type::TypeVar(_)
1208+
)
1209+
})
1210+
}
1211+
12031212
pub(crate) const fn as_special_form(self) -> Option<SpecialFormType> {
12041213
match self {
12051214
Type::SpecialForm(special_form) => Some(special_form),
@@ -8197,8 +8206,12 @@ impl<'db> TypeVarInstance<'db> {
81978206
TypeVarBoundOrConstraintsEvaluation::Eager(bound_or_constraints) => {
81988207
Some(bound_or_constraints)
81998208
}
8200-
TypeVarBoundOrConstraintsEvaluation::LazyUpperBound => self.lazy_bound(db),
8201-
TypeVarBoundOrConstraintsEvaluation::LazyConstraints => self.lazy_constraints(db),
8209+
TypeVarBoundOrConstraintsEvaluation::LazyUpperBound => self
8210+
.lazy_bound(db)
8211+
.map(TypeVarBoundOrConstraints::UpperBound),
8212+
TypeVarBoundOrConstraintsEvaluation::LazyConstraints => self
8213+
.lazy_constraints(db)
8214+
.map(TypeVarBoundOrConstraints::Constraints),
82028215
})
82038216
}
82048217

@@ -8228,12 +8241,20 @@ impl<'db> TypeVarInstance<'db> {
82288241
TypeVarBoundOrConstraintsEvaluation::Eager(bound_or_constraints) => {
82298242
Some(bound_or_constraints.normalized_impl(db, visitor).into())
82308243
}
8231-
TypeVarBoundOrConstraintsEvaluation::LazyUpperBound => self
8232-
.lazy_bound(db)
8233-
.map(|bound| bound.normalized_impl(db, visitor).into()),
8234-
TypeVarBoundOrConstraintsEvaluation::LazyConstraints => self
8235-
.lazy_constraints(db)
8236-
.map(|constraints| constraints.normalized_impl(db, visitor).into()),
8244+
TypeVarBoundOrConstraintsEvaluation::LazyUpperBound => {
8245+
self.lazy_bound(db).map(|bound| {
8246+
TypeVarBoundOrConstraints::UpperBound(bound)
8247+
.normalized_impl(db, visitor)
8248+
.into()
8249+
})
8250+
}
8251+
TypeVarBoundOrConstraintsEvaluation::LazyConstraints => {
8252+
self.lazy_constraints(db).map(|constraints| {
8253+
TypeVarBoundOrConstraints::Constraints(constraints)
8254+
.normalized_impl(db, visitor)
8255+
.into()
8256+
})
8257+
}
82378258
}),
82388259
self.explicit_variance(db),
82398260
self._default(db).and_then(|default| match default {
@@ -8263,14 +8284,14 @@ impl<'db> TypeVarInstance<'db> {
82638284
),
82648285
TypeVarBoundOrConstraintsEvaluation::LazyUpperBound => {
82658286
self.lazy_bound(db).map(|bound| {
8266-
bound
8287+
TypeVarBoundOrConstraints::UpperBound(bound)
82678288
.materialize_impl(db, materialization_kind, visitor)
82688289
.into()
82698290
})
82708291
}
82718292
TypeVarBoundOrConstraintsEvaluation::LazyConstraints => {
82728293
self.lazy_constraints(db).map(|constraints| {
8273-
constraints
8294+
TypeVarBoundOrConstraints::Constraints(constraints)
82748295
.materialize_impl(db, materialization_kind, visitor)
82758296
.into()
82768297
})
@@ -8323,12 +8344,14 @@ impl<'db> TypeVarInstance<'db> {
83238344
})
83248345
}
83258346

8347+
/// Returns the "unchecked" upper bound of a type variable instance.
8348+
/// `lazy_bound` checks if the upper bound type is generic (generic upper bound is not allowed).
83268349
#[salsa::tracked(
8327-
cycle_fn=lazy_bound_or_constraints_cycle_recover,
8328-
cycle_initial=lazy_bound_or_constraints_cycle_initial,
8350+
cycle_fn=lazy_bound_cycle_recover,
8351+
cycle_initial=|_, _, _| None,
83298352
heap_size=ruff_memory_usage::heap_size
83308353
)]
8331-
fn lazy_bound(self, db: &'db dyn Db) -> Option<TypeVarBoundOrConstraints<'db>> {
8354+
fn lazy_bound_unchecked(self, db: &'db dyn Db) -> Option<Type<'db>> {
83328355
let definition = self.definition(db)?;
83338356
let module = parsed_module(db, definition.file(db)).load(db);
83348357
let ty = match definition.kind(db) {
@@ -8346,19 +8369,27 @@ impl<'db> TypeVarInstance<'db> {
83468369
_ => return None,
83478370
};
83488371

8349-
if self.type_is_self_referential(db, ty) {
8372+
Some(ty)
8373+
}
8374+
8375+
fn lazy_bound(self, db: &'db dyn Db) -> Option<Type<'db>> {
8376+
let bound = self.lazy_bound_unchecked(db)?;
8377+
8378+
if bound.has_typevar_or_typevar_instance(db) {
83508379
return None;
83518380
}
83528381

8353-
Some(TypeVarBoundOrConstraints::UpperBound(ty))
8382+
Some(bound)
83548383
}
83558384

8385+
/// Returns the "unchecked" constraints of a type variable instance.
8386+
/// `lazy_constraints` checks if any of the constraint types are generic (generic constraints are not allowed).
83568387
#[salsa::tracked(
8357-
cycle_fn=lazy_bound_or_constraints_cycle_recover,
8358-
cycle_initial=lazy_bound_or_constraints_cycle_initial,
8388+
cycle_fn=lazy_constraints_cycle_recover,
8389+
cycle_initial=|_, _, _| None,
83598390
heap_size=ruff_memory_usage::heap_size
83608391
)]
8361-
fn lazy_constraints(self, db: &'db dyn Db) -> Option<TypeVarBoundOrConstraints<'db>> {
8392+
fn lazy_constraints_unchecked(self, db: &'db dyn Db) -> Option<TypeVarConstraints<'db>> {
83628393
let definition = self.definition(db)?;
83638394
let module = parsed_module(db, definition.file(db)).load(db);
83648395
let constraints = match definition.kind(db) {
@@ -8393,19 +8424,27 @@ impl<'db> TypeVarInstance<'db> {
83938424
_ => return None,
83948425
};
83958426

8427+
Some(constraints)
8428+
}
8429+
8430+
fn lazy_constraints(self, db: &'db dyn Db) -> Option<TypeVarConstraints<'db>> {
8431+
let constraints = self.lazy_constraints_unchecked(db)?;
8432+
83968433
if constraints
83978434
.elements(db)
83988435
.iter()
8399-
.any(|ty| self.type_is_self_referential(db, *ty))
8436+
.any(|ty| ty.has_typevar_or_typevar_instance(db))
84008437
{
84018438
return None;
84028439
}
84038440

8404-
Some(TypeVarBoundOrConstraints::Constraints(constraints))
8441+
Some(constraints)
84058442
}
84068443

8444+
/// Returns the "unchecked" default type of a type variable instance.
8445+
/// `lazy_default` checks if the default type is not self-referential.
84078446
#[salsa::tracked(cycle_initial=|_, id, _| Some(Type::divergent(id)), cycle_fn=lazy_default_cycle_recover, heap_size=ruff_memory_usage::heap_size)]
8408-
fn lazy_default(self, db: &'db dyn Db) -> Option<Type<'db>> {
8447+
fn lazy_default_unchecked(self, db: &'db dyn Db) -> Option<Type<'db>> {
84098448
fn convert_type_to_paramspec_value<'db>(db: &'db dyn Db, ty: Type<'db>) -> Type<'db> {
84108449
let parameters = match ty {
84118450
Type::NominalInstance(nominal_instance)
@@ -8478,11 +8517,20 @@ impl<'db> TypeVarInstance<'db> {
84788517
_ => return None,
84798518
};
84808519

8481-
if self.type_is_self_referential(db, ty) {
8520+
Some(ty)
8521+
}
8522+
8523+
fn lazy_default(self, db: &'db dyn Db) -> Option<Type<'db>> {
8524+
let default = self.lazy_default_unchecked(db)?;
8525+
8526+
// Unlike bounds/constraints, default types are allowed to be generic (https://peps.python.org/pep-0696/#using-another-type-parameter-as-default).
8527+
// Here we simply check for non-self-referential.
8528+
// TODO: We should also check for non-forward references.
8529+
if self.type_is_self_referential(db, default) {
84828530
return None;
84838531
}
84848532

8485-
Some(ty)
8533+
Some(default)
84868534
}
84878535

84888536
pub fn bind_pep695(self, db: &'db dyn Db) -> Option<BoundTypeVarInstance<'db>> {
@@ -8504,25 +8552,34 @@ impl<'db> TypeVarInstance<'db> {
85048552
}
85058553
}
85068554

8507-
fn lazy_bound_or_constraints_cycle_initial<'db>(
8508-
_db: &'db dyn Db,
8509-
_id: salsa::Id,
8510-
_self: TypeVarInstance<'db>,
8511-
) -> Option<TypeVarBoundOrConstraints<'db>> {
8512-
None
8555+
#[expect(clippy::ref_option)]
8556+
fn lazy_bound_cycle_recover<'db>(
8557+
db: &'db dyn Db,
8558+
cycle: &salsa::Cycle,
8559+
previous: &Option<Type<'db>>,
8560+
current: Option<Type<'db>>,
8561+
_typevar: TypeVarInstance<'db>,
8562+
) -> Option<Type<'db>> {
8563+
// Normalize the bounds/constraints to ensure cycle convergence.
8564+
match (previous, current) {
8565+
(Some(prev), Some(current)) => Some(current.cycle_normalized(db, *prev, cycle)),
8566+
(None, Some(current)) => Some(current.recursive_type_normalized(db, cycle)),
8567+
(_, None) => None,
8568+
}
85138569
}
85148570

8571+
#[allow(clippy::trivially_copy_pass_by_ref)]
85158572
#[expect(clippy::ref_option)]
8516-
fn lazy_bound_or_constraints_cycle_recover<'db>(
8573+
fn lazy_constraints_cycle_recover<'db>(
85178574
db: &'db dyn Db,
85188575
cycle: &salsa::Cycle,
8519-
previous: &Option<TypeVarBoundOrConstraints<'db>>,
8520-
current: Option<TypeVarBoundOrConstraints<'db>>,
8576+
previous: &Option<TypeVarConstraints<'db>>,
8577+
current: Option<TypeVarConstraints<'db>>,
85218578
_typevar: TypeVarInstance<'db>,
8522-
) -> Option<TypeVarBoundOrConstraints<'db>> {
8579+
) -> Option<TypeVarConstraints<'db>> {
85238580
// Normalize the bounds/constraints to ensure cycle convergence.
85248581
match (previous, current) {
8525-
(Some(prev), Some(current)) => Some(current.cycle_normalized(db, *prev, cycle)),
8582+
(Some(prev), Some(constraints)) => Some(constraints.cycle_normalized(db, *prev, cycle)),
85268583
(None, Some(current)) => Some(current.recursive_type_normalized(db, cycle)),
85278584
(_, None) => None,
85288585
}
@@ -9107,6 +9164,30 @@ impl<'db> TypeVarConstraints<'db> {
91079164
.collect::<Box<_>>();
91089165
TypeVarConstraints::new(db, materialized)
91099166
}
9167+
9168+
/// Normalize for cycle recovery by combining with the previous value and
9169+
/// removing divergent types introduced by the cycle.
9170+
///
9171+
/// See [`Type::cycle_normalized`] for more details on how this works.
9172+
fn cycle_normalized(self, db: &'db dyn Db, previous: Self, cycle: &salsa::Cycle) -> Self {
9173+
let current_elements = self.elements(db);
9174+
let prev_elements = previous.elements(db);
9175+
TypeVarConstraints::new(
9176+
db,
9177+
current_elements
9178+
.iter()
9179+
.zip(prev_elements.iter())
9180+
.map(|(ty, prev_ty)| ty.cycle_normalized(db, *prev_ty, cycle))
9181+
.collect::<Box<_>>(),
9182+
)
9183+
}
9184+
9185+
/// Normalize recursive types for cycle recovery when there's no previous value.
9186+
///
9187+
/// See [`Type::recursive_type_normalized`] for more details.
9188+
fn recursive_type_normalized(self, db: &'db dyn Db, cycle: &salsa::Cycle) -> Self {
9189+
self.map(db, |ty| ty.recursive_type_normalized(db, cycle))
9190+
}
91109191
}
91119192

91129193
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, salsa::Update, get_size2::GetSize)]
@@ -9140,59 +9221,6 @@ impl<'db> TypeVarBoundOrConstraints<'db> {
91409221
}
91419222
}
91429223

9143-
/// Normalize for cycle recovery by combining with the previous value and
9144-
/// removing divergent types introduced by the cycle.
9145-
///
9146-
/// See [`Type::cycle_normalized`] for more details on how this works.
9147-
fn cycle_normalized(self, db: &'db dyn Db, previous: Self, cycle: &salsa::Cycle) -> Self {
9148-
match (self, previous) {
9149-
(
9150-
TypeVarBoundOrConstraints::UpperBound(bound),
9151-
TypeVarBoundOrConstraints::UpperBound(prev_bound),
9152-
) => {
9153-
TypeVarBoundOrConstraints::UpperBound(bound.cycle_normalized(db, prev_bound, cycle))
9154-
}
9155-
(
9156-
TypeVarBoundOrConstraints::Constraints(constraints),
9157-
TypeVarBoundOrConstraints::Constraints(prev_constraints),
9158-
) => {
9159-
// Normalize each constraint with its corresponding previous constraint
9160-
let current_elements = constraints.elements(db);
9161-
let prev_elements = prev_constraints.elements(db);
9162-
TypeVarBoundOrConstraints::Constraints(TypeVarConstraints::new(
9163-
db,
9164-
current_elements
9165-
.iter()
9166-
.zip(prev_elements.iter())
9167-
.map(|(ty, prev_ty)| ty.cycle_normalized(db, *prev_ty, cycle))
9168-
.collect::<Box<_>>(),
9169-
))
9170-
}
9171-
// The choice of whether it's an upper bound or constraints is purely syntactic and
9172-
// thus can never change in a cycle: `parsed_module` does not participate in cycles,
9173-
// the AST will never change from one iteration to the next.
9174-
_ => unreachable!(
9175-
"TypeVar switched from bound to constraints (or vice versa) in fixpoint iteration"
9176-
),
9177-
}
9178-
}
9179-
9180-
/// Normalize recursive types for cycle recovery when there's no previous value.
9181-
///
9182-
/// See [`Type::recursive_type_normalized`] for more details.
9183-
fn recursive_type_normalized(self, db: &'db dyn Db, cycle: &salsa::Cycle) -> Self {
9184-
match self {
9185-
TypeVarBoundOrConstraints::UpperBound(bound) => {
9186-
TypeVarBoundOrConstraints::UpperBound(bound.recursive_type_normalized(db, cycle))
9187-
}
9188-
TypeVarBoundOrConstraints::Constraints(constraints) => {
9189-
TypeVarBoundOrConstraints::Constraints(
9190-
constraints.map(db, |ty| ty.recursive_type_normalized(db, cycle)),
9191-
)
9192-
}
9193-
}
9194-
}
9195-
91969224
fn materialize_impl(
91979225
self,
91989226
db: &'db dyn Db,

0 commit comments

Comments
 (0)