Skip to content

Commit f4b8833

Browse files
yangdanny97meta-codesync[bot]
yangdanny97
authored and
meta-codesync[bot]
committed
add optional display name to Type::Union
Summary: This is an alternative to D84849556, which aims to solve facebook#1219 Instead of introducing a new type variant, this diff introduces a new field in Type::Union to hold an optional display name. It will be populated when resolving a type alias. Compared to the other approach: benefits - less plumbing - type checking behavior unaffected drawbacks - only applies to type aliases of unions (though arguably this is where readability matters most) - name information is not preserved during union flattening (so if we union a type alias with something else, the result is the full expanded type) Differential Revision: D87558541
1 parent dbdc45e commit f4b8833

File tree

27 files changed

+90
-75
lines changed

27 files changed

+90
-75
lines changed

‎crates/pyrefly_types/src/display.rs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -412,10 +412,11 @@ impl<'a> TypeDisplayContext<'a> {
412412
self.maybe_fmt_with_module("typing", "NoReturn", output)
413413
}
414414
Type::Never(NeverStyle::Never) => self.maybe_fmt_with_module("typing", "Never", output),
415-
Type::Union(types) if types.is_empty() => {
415+
Type::Union(box (types, _)) if types.is_empty() => {
416416
self.maybe_fmt_with_module("typing", "Never", output)
417417
}
418-
Type::Union(types) => {
418+
Type::Union(box (_, Some(name))) => output.write_str(name),
419+
Type::Union(box (types, None)) => {
419420
// All Literals will be collected into a single Literal at the index of the first Literal.
420421
let mut literal_idx = None;
421422
let mut literals = Vec::new();
@@ -1034,7 +1035,7 @@ pub mod tests {
10341035
assert_eq!(ctx.display(&int_type).to_string(), "int");
10351036
}
10361037

1037-
let union_foo_int = Type::Union(vec![foo_type, int_type]);
1038+
let union_foo_int = Type::union(vec![foo_type, int_type]);
10381039

10391040
{
10401041
let mut ctx = TypeDisplayContext::new(&[&union_foo_int]);
@@ -1050,11 +1051,11 @@ pub mod tests {
10501051
let t3 = fake_tyvar("qux", "bar", 2);
10511052

10521053
assert_eq!(
1053-
Type::Union(vec![t1.to_type(), t2.to_type()]).to_string(),
1054+
Type::union(vec![t1.to_type(), t2.to_type()]).to_string(),
10541055
"TypeVar[bar.foo@1:2] | TypeVar[bar.foo@1:3]"
10551056
);
10561057
assert_eq!(
1057-
Type::Union(vec![t1.to_type(), t3.to_type()]).to_string(),
1058+
Type::union(vec![t1.to_type(), t3.to_type()]).to_string(),
10581059
"TypeVar[foo] | TypeVar[qux]"
10591060
);
10601061
}
@@ -1094,13 +1095,21 @@ pub mod tests {
10941095
let nonlit2 = Type::LiteralString;
10951096

10961097
assert_eq!(
1097-
Type::Union(vec![nonlit1.clone(), nonlit2.clone()]).to_string(),
1098+
Type::union(vec![nonlit1.clone(), nonlit2.clone()]).to_string(),
10981099
"None | LiteralString"
10991100
);
11001101
assert_eq!(
1101-
Type::Union(vec![nonlit1, lit1, nonlit2, lit2]).to_string(),
1102+
Type::union(vec![nonlit1.clone(), lit1, nonlit2.clone(), lit2]).to_string(),
11021103
"None | Literal[True, 'test'] | LiteralString"
11031104
);
1105+
assert_eq!(
1106+
Type::Union(Box::new((
1107+
vec![nonlit1, nonlit2],
1108+
Some("MyUnion".to_owned())
1109+
)))
1110+
.to_string(),
1111+
"MyUnion"
1112+
);
11041113
}
11051114

11061115
#[test]
@@ -1510,7 +1519,7 @@ def overloaded_func[T](
15101519

15111520
#[test]
15121521
fn test_union_of_intersection() {
1513-
let x = Type::Union(vec![
1522+
let x = Type::union(vec![
15141523
Type::Intersect(Box::new((
15151524
vec![Type::any_explicit(), Type::LiteralString],
15161525
Type::any_implicit(),

‎crates/pyrefly_types/src/simplify.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ fn flatten_and_dedup(xs: Vec<Type>) -> Vec<Type> {
2020
fn flatten(xs: Vec<Type>, res: &mut Vec<Type>) {
2121
for x in xs {
2222
match x {
23-
Type::Union(xs) => flatten(xs, res),
23+
Type::Union(box (xs, _)) => flatten(xs, res),
2424
Type::Never(_) => {}
2525
_ => res.push(x),
2626
}
@@ -76,7 +76,7 @@ fn unions_internal(
7676
}
7777
collapse_tuple_unions_with_empty(&mut res);
7878
// `res` is collapsible again if `flatten_and_dedup` drops `xs` to 0 or 1 elements
79-
try_collapse(res).unwrap_or_else(Type::Union)
79+
try_collapse(res).unwrap_or_else(|members| Type::Union(Box::new((members, None))))
8080
})
8181
}
8282

‎crates/pyrefly_types/src/types.rs‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,8 @@ pub enum Type {
625625
BoundMethod(Box<BoundMethod>),
626626
/// An overloaded function.
627627
Overload(Overload),
628-
Union(Vec<Type>),
628+
/// Unions will hold an optional name to use when displaying the type
629+
Union(Box<(Vec<Type>, Option<String>)>),
629630
/// Our intersection support is partial, so we store a fallback type that we use for operations
630631
/// that are not yet supported on intersections.
631632
Intersect(Box<(Vec<Type>, Type)>),
@@ -1362,7 +1363,7 @@ impl Type {
13621363
/// type[a | b] -> type[a] | type[b]
13631364
pub fn distribute_type_over_union(self) -> Self {
13641365
self.transform(&mut |ty| {
1365-
if let Type::Type(box Type::Union(members)) = ty {
1366+
if let Type::Type(box Type::Union(box (members, _))) = ty {
13661367
*ty = unions(members.drain(..).map(Type::type_form).collect());
13671368
}
13681369
})
@@ -1407,7 +1408,7 @@ impl Type {
14071408

14081409
pub fn sort_unions(self) -> Self {
14091410
self.transform(&mut |ty| {
1410-
if let Type::Union(ts) = ty {
1411+
if let Type::Union(box (ts, _)) = ty {
14111412
ts.sort();
14121413
}
14131414
})
@@ -1458,27 +1459,27 @@ impl Type {
14581459

14591460
pub fn into_unions(self) -> Vec<Type> {
14601461
match self {
1461-
Type::Union(types) => types,
1462+
Type::Union(box (types, _)) => types,
14621463
_ => vec![self],
14631464
}
14641465
}
14651466

14661467
/// Create an optional type (union with None).
14671468
pub fn optional(x: Self) -> Self {
14681469
// We would like the resulting type not nested, and well sorted.
1469-
if let Type::Union(mut xs) = x {
1470+
if let Type::Union(box (mut xs, _)) = x {
14701471
match xs.binary_search(&Type::None) {
1471-
Ok(_) => Type::Union(xs),
1472+
Ok(_) => Type::union(xs),
14721473
Err(i) => {
14731474
xs.insert(i, Type::None);
1474-
Type::Union(xs)
1475+
Type::union(xs)
14751476
}
14761477
}
14771478
} else {
14781479
match x.cmp(&Type::None) {
14791480
Ordering::Equal => Type::None,
1480-
Ordering::Less => Type::Union(vec![x, Type::None]),
1481-
Ordering::Greater => Type::Union(vec![Type::None, x]),
1481+
Ordering::Less => Type::union(vec![x, Type::None]),
1482+
Ordering::Greater => Type::union(vec![Type::None, x]),
14821483
}
14831484
}
14841485
}
@@ -1508,7 +1509,7 @@ impl Type {
15081509
Type::Literal(Lit::Str(x)) => Some(!x.is_empty()),
15091510
Type::None => Some(false),
15101511
Type::Tuple(Tuple::Concrete(elements)) => Some(!elements.is_empty()),
1511-
Type::Union(options) => {
1512+
Type::Union(box (options, _)) => {
15121513
let mut answer = None;
15131514
for option in options {
15141515
let option_bool = option.as_bool();
@@ -1560,6 +1561,11 @@ impl Type {
15601561
})
15611562
})
15621563
}
1564+
1565+
/// Creates a union from the provided types without simplifying
1566+
pub fn union(members: Vec<Type>) -> Self {
1567+
Type::Union(Box::new((members, None)))
1568+
}
15631569
}
15641570

15651571
#[cfg(test)]
@@ -1586,8 +1592,8 @@ mod tests {
15861592
let false_lit = Type::Literal(Lit::Bool(false));
15871593
let none = Type::None;
15881594

1589-
let str_opt = Type::Union(vec![s, none.clone()]);
1590-
let false_opt = Type::Union(vec![false_lit, none]);
1595+
let str_opt = Type::union(vec![s, none.clone()]);
1596+
let false_opt = Type::union(vec![false_lit, none]);
15911597

15921598
assert_eq!(str_opt.as_bool(), None);
15931599
assert_eq!(false_opt.as_bool(), Some(false));

‎pyrefly/lib/alt/answers_solver.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -797,11 +797,11 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
797797
fn go(&mut self, ty: &Type, in_type: bool) {
798798
match ty {
799799
Type::Never(_) if !in_type => (),
800-
Type::Union(tys) => {
800+
Type::Union(box (tys, _)) => {
801801
self.seen_union = true;
802802
tys.iter().for_each(|ty| self.go(ty, in_type))
803803
}
804-
Type::Type(box Type::Union(tys)) if !in_type => {
804+
Type::Type(box Type::Union(box (tys, _))) if !in_type => {
805805
tys.iter().for_each(|ty| self.go(ty, true))
806806
}
807807
Type::Var(v) if let Some(_guard) = self.me.recurse(*v) => {

‎pyrefly/lib/alt/attr.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1776,12 +1776,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
17761776
Type::SuperInstance(box (cls, obj)) => {
17771777
acc.push(AttributeBase1::SuperInstance(cls, obj))
17781778
}
1779-
Type::Union(members) => {
1779+
Type::Union(box (members, _)) => {
17801780
for ty in members {
17811781
self.as_attribute_base1(ty, acc)
17821782
}
17831783
}
1784-
Type::Type(box Type::Union(members)) => {
1784+
Type::Type(box Type::Union(box (members, _))) => {
17851785
for ty in members {
17861786
self.as_attribute_base1(Type::type_form(ty), acc)
17871787
}

‎pyrefly/lib/alt/call.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
260260
Type::Var(v) if let Some(_guard) = self.recurse(v) => {
261261
self.as_call_target_impl(self.solver().force_var(v), quantified, dunder_call)
262262
}
263-
Type::Union(xs) => {
263+
Type::Union(box (xs, _)) => {
264264
let xs_length = xs.len();
265265
let targets = xs
266266
.into_iter()
@@ -315,7 +315,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
315315
Type::Quantified(q) if q.is_type_var() => match q.restriction() {
316316
Restriction::Unrestricted => CallTargetLookup::Error(vec![]),
317317
Restriction::Bound(bound) => match bound {
318-
Type::Union(members) => {
318+
Type::Union(box (members, _)) => {
319319
let mut targets = Vec::new();
320320
for member in members {
321321
if let CallTargetLookup::Ok(target) = self.as_call_target_impl(

‎pyrefly/lib/alt/class/class_field.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ fn make_bound_method_helper(
942942
Type::Overload(overload) if should_bind2(&overload.metadata) => {
943943
BoundMethodType::Overload(overload)
944944
}
945-
Type::Union(ref ts) => {
945+
Type::Union(box (ref ts, _)) => {
946946
let mut bound_methods = Vec::with_capacity(ts.len());
947947
for t in ts {
948948
match make_bound_method_helper(obj.clone(), t.clone(), should_bind) {

‎pyrefly/lib/alt/class/classdef.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
5151
Type::SelfType(ty_cls) | Type::ClassType(ty_cls) => {
5252
self.has_superclass(ty_cls.class_object(), class)
5353
}
54-
Type::Union(xs) => xs
54+
Type::Union(box (xs, _)) => xs
5555
.iter()
5656
.all(|x| self.is_compatible_constructor_return(x, class)),
5757
_ => false,

‎pyrefly/lib/alt/class/django.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
6767
Type::ClassDef(cls) => {
6868
self.get_django_field_type_from_class(cls, class, field_name, initial_value_expr)
6969
}
70-
Type::Union(union) => {
70+
Type::Union(box (union, _)) => {
7171
let transformed: Vec<_> = union
7272
.iter()
7373
.map(|variant| {
@@ -365,7 +365,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
365365
// Get the related model type from the field
366366
let ty = class_field.ty();
367367
let (related_cls, is_foreign_key_nullable) = match ty {
368-
Type::Union(union) => {
368+
Type::Union(box (union, _)) => {
369369
// Nullable foreign key: extract the class type from the union
370370
let cls = union.iter().find_map(|variant| match variant {
371371
Type::ClassType(cls) => Some(cls.clone()),

‎pyrefly/lib/alt/class/pydantic.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
141141
/// Recursively expands nested RootModels (e.g., RootModel[RootModel[int]] expands to RootModel[int] | int).
142142
pub fn extract_root_model_inner_type(&self, ty: &Type) -> Option<Type> {
143143
match ty {
144-
Type::Union(types) => {
144+
Type::Union(box (types, _)) => {
145145
let root_types: Vec<Type> = types
146146
.iter()
147147
.filter_map(|t| self.extract_root_model_inner_type(t))

0 commit comments

Comments
 (0)