Update type_ordering.rs

* exclude ordering of nested unions (which cannot exist in DNF) as unreachable codes
* try to avoid performance degradation in ordering of intersections

Author: mtshiba

crates/red_knot_python_semantic/src/types.rs

@@ -9,7 +9,7 @@ use ruff_db::files::File;
 use ruff_python_ast as ast;
 use ruff_python_ast::name::Name;
 use ruff_text_size::{Ranged, TextRange};
-use type_ordering::union_elements_ordering;
+use type_ordering::union_or_intersection_elements_ordering;
 
 pub(crate) use self::builder::{IntersectionBuilder, UnionBuilder};
 pub(crate) use self::diagnostic::register_lints;
@@ -5387,7 +5387,7 @@ impl<'db> UnionType<'db> {
             .iter()
             .map(|element| element.with_sorted_unions(db))
             .collect();
-        new_elements.sort_unstable_by(|l, r| union_elements_ordering(l, r, db));
+        new_elements.sort_unstable_by(|l, r| union_or_intersection_elements_ordering(l, r, db));
         UnionType::new(db, new_elements.into_boxed_slice())
     }
 
@@ -5501,7 +5501,7 @@ impl<'db> IntersectionType<'db> {
                 .map(|ty| ty.with_sorted_unions(db))
                 .collect();
 
-            elements.sort_unstable_by(|l, r| union_elements_ordering(l, r, db));
+            elements.sort_unstable_by(|l, r| union_or_intersection_elements_ordering(l, r, db));
             elements
         }
 

crates/red_knot_python_semantic/src/types/type_ordering.rs

@@ -11,7 +11,8 @@ use super::{
 /// in an [`crate::types::IntersectionType`] or a [`crate::types::UnionType`] in order for them
 /// to be compared for equivalence.
 ///
-/// Two unions / intersections are compared lexicographically.
+/// Two intersections are compared lexicographically.
+/// Two unions are never compared in this function because DNF does not permit nested unions.
 ///
 /// Element types must already be sorted.
 ///
@@ -21,7 +22,7 @@ use super::{
 /// create here is not user-facing. However, it doesn't really "make sense" for `Type` to implement
 /// [`Ord`] in terms of the semantics. There are many different ways in which you could plausibly
 /// sort a list of types; this is only one (somewhat arbitrary, at times) possible ordering.
-pub(super) fn union_elements_ordering<'db>(
+pub(super) fn union_or_intersection_elements_ordering<'db>(
     left: &Type<'db>,
     right: &Type<'db>,
     db: &'db dyn crate::Db,
@@ -269,38 +270,32 @@ pub(super) fn union_elements_ordering<'db>(
         (Type::Dynamic(_), _) => Ordering::Less,
         (_, Type::Dynamic(_)) => Ordering::Greater,
 
-        (Type::Union(left), Type::Union(right)) => {
-            if left.elements(db).len() != right.elements(db).len() {
-                return left.elements(db).len().cmp(&right.elements(db).len());
-            }
-            // Lexicographically compare the elements of the two unions.
-            for (left, right) in left.elements(db).iter().zip(right.elements(db)) {
-                let ordering = union_elements_ordering(left, right, db);
-                if ordering != Ordering::Equal {
-                    return ordering;
-                }
-            }
-            Ordering::Equal
+        (Type::Union(_), Type::Union(_)) => {
+            unreachable!("our type representation does not permit nested unions");
         }
         (Type::Union(_), _) => Ordering::Less,
         (_, Type::Union(_)) => Ordering::Greater,
 
         (Type::Intersection(left), Type::Intersection(right)) => {
             // Lexicographically compare the elements of the two intersections.
-            if left.positive(db).len() != right.positive(db).len() {
-                return left.positive(db).len().cmp(&right.positive(db).len());
+            let left_positive = left.positive(db);
+            let right_positive = right.positive(db);
+            if left_positive.len() != right_positive.len() {
+                return left_positive.len().cmp(&right_positive.len());
             }
-            if left.negative(db).len() != right.negative(db).len() {
-                return left.negative(db).len().cmp(&right.negative(db).len());
+            let left_negative = left.negative(db);
+            let right_negative = right.negative(db);
+            if left_negative.len() != right_negative.len() {
+                return left_negative.len().cmp(&right_negative.len());
             }
-            for (left, right) in left.positive(db).iter().zip(right.positive(db)) {
-                let ordering = union_elements_ordering(left, right, db);
+            for (left, right) in left_positive.iter().zip(right_positive) {
+                let ordering = union_or_intersection_elements_ordering(left, right, db);
                 if ordering != Ordering::Equal {
                     return ordering;
                 }
             }
-            for (left, right) in left.negative(db).iter().zip(right.negative(db)) {
-                let ordering = union_elements_ordering(left, right, db);
+            for (left, right) in left_negative.iter().zip(right_negative) {
+                let ordering = union_or_intersection_elements_ordering(left, right, db);
                 if ordering != Ordering::Equal {
                     return ordering;
                 }