construct constraints with typevars in fixed order

Author: dcreager

crates/ty_python_semantic/resources/mdtest/type_properties/constraints.md

@@ -602,6 +602,32 @@ def _[T, U]() -> None:
     reveal_type(~union | union)
 ```
 
+## Typevar ordering
+
+Constraints can relate two typevars — i.e., `S ≤ T`. We could encode that in one of two ways:
+`Never ≤ S ≤ T` or `S ≤ T ≤ object`. In other words, we can decide whether `S` or `T` is the typevar
+being constrained. The other is then the lower or upper bound of the constraint.
+
+To handle this, we enforce an arbitrary ordering on typevars, and always place the constraint on the
+"later" typevar. For the example above, that does not change how the constraint is displayed, since
+we always hide `Never` lower bounds and `object` upper bounds. But in the case of `S ≤ T ≤ U`, we
+end up with an ambiguity. Depending on the typevar ordering, that might display as `S ≤ T ≤ U`, or
+as `(S ≤ T) ∧ (T ≤ U)`.
+
+```py
+from typing import Never
+from ty_extensions import range_constraint
+
+def f[S, T]():
+    # These will both display the same, even though we are guaranteed to (try to) construct one of
+    # the constraints with the typevars out of order.
+
+    # revealed: ty_extensions.ConstraintSet[(S@f ≤ T@f)]
+    reveal_type(range_constraint(Never, S, T))
+    # revealed: ty_extensions.ConstraintSet[(S@f ≤ T@f)]
+    reveal_type(range_constraint(S, T, object))
+```
+
 ## Other simplifications
 
 When displaying a constraint set, we transform the internal BDD representation into a DNF formula

crates/ty_python_semantic/src/types/constraints.rs

@@ -65,7 +65,9 @@ use salsa::plumbing::AsId;
 
 use crate::Db;
 use crate::types::generics::InferableTypeVars;
-use crate::types::{BoundTypeVarIdentity, IntersectionType, Type, TypeRelation, UnionType};
+use crate::types::{
+    BoundTypeVarIdentity, BoundTypeVarInstance, IntersectionType, Type, TypeRelation, UnionType,
+};
 
 /// An extension trait for building constraint sets from [`Option`] values.
 pub(crate) trait OptionConstraintsExtension<T> {
@@ -244,20 +246,87 @@ impl<'db> ConstraintSet<'db> {
     pub(crate) fn range(
         db: &'db dyn Db,
         lower: Type<'db>,
-        typevar: BoundTypeVarIdentity<'db>,
+        typevar: BoundTypeVarInstance<'db>,
         upper: Type<'db>,
     ) -> Self {
         let lower = lower.bottom_materialization(db);
         let upper = upper.top_materialization(db);
-        Self {
-            node: ConstrainedTypeVar::new_node(db, lower, typevar, upper),
-        }
+
+        // We have an (arbitrary) ordering for typevars. If the upper and/or lower bounds are
+        // typevars, we have to ensure that the bounds are "earlier" according to that order than
+        // the typevar being constrained.
+        //
+        // In the comments below, we use brackets to indicate which typevar is "larger", and
+        // therefore the typevar that the constraint applies to.
+        let node = match (lower, upper) {
+            // A ≤ T ≤ A == (T ≤ [A] ≤ T)
+            (Type::TypeVar(lower), Type::TypeVar(upper)) if lower == upper => {
+                let (bound, typevar) = if lower < typevar {
+                    (lower, typevar)
+                } else {
+                    (typevar, lower)
+                };
+                ConstrainedTypeVar::new_node(
+                    db,
+                    Type::TypeVar(bound),
+                    typevar.identity(db),
+                    Type::TypeVar(bound),
+                )
+            }
+
+            // A ≤ T ≤ B == ([A] ≤ T) && (T ≤ [B])
+            (Type::TypeVar(lower), Type::TypeVar(upper)) if lower > typevar && upper > typevar => {
+                let lower = ConstrainedTypeVar::new_node(
+                    db,
+                    Type::Never,
+                    lower.identity(db),
+                    Type::TypeVar(typevar),
+                );
+                let upper = ConstrainedTypeVar::new_node(
+                    db,
+                    Type::TypeVar(typevar),
+                    upper.identity(db),
+                    Type::object(),
+                );
+                lower.and(db, upper)
+            }
+
+            // A ≤ T ≤ B == ([A] ≤ T) && ([T] ≤ B)
+            (Type::TypeVar(lower), _) if lower > typevar => {
+                let lower = ConstrainedTypeVar::new_node(
+                    db,
+                    Type::Never,
+                    lower.identity(db),
+                    Type::TypeVar(typevar),
+                );
+                let upper =
+                    ConstrainedTypeVar::new_node(db, Type::Never, typevar.identity(db), upper);
+                lower.and(db, upper)
+            }
+
+            // A ≤ T ≤ B == (A ≤ [T]) && (T ≤ [B])
+            (_, Type::TypeVar(upper)) if upper > typevar => {
+                let lower =
+                    ConstrainedTypeVar::new_node(db, lower, typevar.identity(db), Type::object());
+                let upper = ConstrainedTypeVar::new_node(
+                    db,
+                    Type::TypeVar(typevar),
+                    upper.identity(db),
+                    Type::object(),
+                );
+                lower.and(db, upper)
+            }
+
+            _ => ConstrainedTypeVar::new_node(db, lower, typevar.identity(db), upper),
+        };
+
+        Self { node }
     }
 
     pub(crate) fn negated_range(
         db: &'db dyn Db,
         lower: Type<'db>,
-        typevar: BoundTypeVarIdentity<'db>,
+        typevar: BoundTypeVarInstance<'db>,
         upper: Type<'db>,
     ) -> Self {
         Self::range(db, lower, typevar, upper).negate(db)

crates/ty_python_semantic/src/types/function.rs

@@ -1723,7 +1723,7 @@ impl KnownFunction {
                     return;
                 };
 
-                let constraints = ConstraintSet::range(db, *lower, typevar.identity(db), *upper);
+                let constraints = ConstraintSet::range(db, *lower, *typevar, *upper);
                 let tracked = TrackedConstraintSet::new(db, constraints);
                 overload.set_return_type(Type::KnownInstance(KnownInstanceType::ConstraintSet(
                     tracked,
@@ -1736,8 +1736,7 @@ impl KnownFunction {
                     return;
                 };
 
-                let constraints =
-                    ConstraintSet::negated_range(db, *lower, typevar.identity(db), *upper);
+                let constraints = ConstraintSet::negated_range(db, *lower, *typevar, *upper);
                 let tracked = TrackedConstraintSet::new(db, constraints);
                 overload.set_return_type(Type::KnownInstance(KnownInstanceType::ConstraintSet(
                     tracked,