Skip to content

Commit f6ffae7

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 19a2edc commit f6ffae7

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();
@@ -1032,7 +1033,7 @@ pub mod tests {
10321033
assert_eq!(ctx.display(&int_type).to_string(), "int");
10331034
}
10341035

1035-
let union_foo_int = Type::Union(vec![foo_type, int_type]);
1036+
let union_foo_int = Type::union(vec![foo_type, int_type]);
10361037

10371038
{
10381039
let mut ctx = TypeDisplayContext::new(&[&union_foo_int]);
@@ -1048,11 +1049,11 @@ pub mod tests {
10481049
let t3 = fake_tyvar("qux", "bar", 2);
10491050

10501051
assert_eq!(
1051-
Type::Union(vec![t1.to_type(), t2.to_type()]).to_string(),
1052+
Type::union(vec![t1.to_type(), t2.to_type()]).to_string(),
10521053
"TypeVar[bar.foo@1:2] | TypeVar[bar.foo@1:3]"
10531054
);
10541055
assert_eq!(
1055-
Type::Union(vec![t1.to_type(), t3.to_type()]).to_string(),
1056+
Type::union(vec![t1.to_type(), t3.to_type()]).to_string(),
10561057
"TypeVar[foo] | TypeVar[qux]"
10571058
);
10581059
}
@@ -1092,13 +1093,21 @@ pub mod tests {
10921093
let nonlit2 = Type::LiteralString;
10931094

10941095
assert_eq!(
1095-
Type::Union(vec![nonlit1.clone(), nonlit2.clone()]).to_string(),
1096+
Type::union(vec![nonlit1.clone(), nonlit2.clone()]).to_string(),
10961097
"None | LiteralString"
10971098
);
10981099
assert_eq!(
1099-
Type::Union(vec![nonlit1, lit1, nonlit2, lit2]).to_string(),
1100+
Type::union(vec![nonlit1.clone(), lit1, nonlit2.clone(), lit2]).to_string(),
11001101
"None | Literal[True, 'test'] | LiteralString"
11011102
);
1103+
assert_eq!(
1104+
Type::Union(Box::new((
1105+
vec![nonlit1, nonlit2],
1106+
Some("MyUnion".to_owned())
1107+
)))
1108+
.to_string(),
1109+
"MyUnion"
1110+
);
11021111
}
11031112

11041113
#[test]
@@ -1508,7 +1517,7 @@ def overloaded_func[T](
15081517

15091518
#[test]
15101519
fn test_union_of_intersection() {
1511-
let x = Type::Union(vec![
1520+
let x = Type::union(vec![
15121521
Type::Intersect(Box::new((
15131522
vec![Type::any_explicit(), Type::LiteralString],
15141523
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)>),
@@ -1378,7 +1379,7 @@ impl Type {
13781379
/// type[a | b] -> type[a] | type[b]
13791380
pub fn distribute_type_over_union(self) -> Self {
13801381
self.transform(&mut |ty| {
1381-
if let Type::Type(box Type::Union(members)) = ty {
1382+
if let Type::Type(box Type::Union(box (members, _))) = ty {
13821383
*ty = unions(members.drain(..).map(Type::type_form).collect());
13831384
}
13841385
})
@@ -1423,7 +1424,7 @@ impl Type {
14231424

14241425
pub fn sort_unions(self) -> Self {
14251426
self.transform(&mut |ty| {
1426-
if let Type::Union(ts) = ty {
1427+
if let Type::Union(box (ts, _)) = ty {
14271428
ts.sort();
14281429
}
14291430
})
@@ -1474,27 +1475,27 @@ impl Type {
14741475

14751476
pub fn into_unions(self) -> Vec<Type> {
14761477
match self {
1477-
Type::Union(types) => types,
1478+
Type::Union(box (types, _)) => types,
14781479
_ => vec![self],
14791480
}
14801481
}
14811482

14821483
/// Create an optional type (union with None).
14831484
pub fn optional(x: Self) -> Self {
14841485
// We would like the resulting type not nested, and well sorted.
1485-
if let Type::Union(mut xs) = x {
1486+
if let Type::Union(box (mut xs, _)) = x {
14861487
match xs.binary_search(&Type::None) {
1487-
Ok(_) => Type::Union(xs),
1488+
Ok(_) => Type::union(xs),
14881489
Err(i) => {
14891490
xs.insert(i, Type::None);
1490-
Type::Union(xs)
1491+
Type::union(xs)
14911492
}
14921493
}
14931494
} else {
14941495
match x.cmp(&Type::None) {
14951496
Ordering::Equal => Type::None,
1496-
Ordering::Less => Type::Union(vec![x, Type::None]),
1497-
Ordering::Greater => Type::Union(vec![Type::None, x]),
1497+
Ordering::Less => Type::union(vec![x, Type::None]),
1498+
Ordering::Greater => Type::union(vec![Type::None, x]),
14981499
}
14991500
}
15001501
}
@@ -1524,7 +1525,7 @@ impl Type {
15241525
Type::Literal(Lit::Str(x)) => Some(!x.is_empty()),
15251526
Type::None => Some(false),
15261527
Type::Tuple(Tuple::Concrete(elements)) => Some(!elements.is_empty()),
1527-
Type::Union(options) => {
1528+
Type::Union(box (options, _)) => {
15281529
let mut answer = None;
15291530
for option in options {
15301531
let option_bool = option.as_bool();
@@ -1576,6 +1577,11 @@ impl Type {
15761577
})
15771578
})
15781579
}
1580+
1581+
/// Creates a union from the provided types without simplifying
1582+
pub fn union(members: Vec<Type>) -> Self {
1583+
Type::Union(Box::new((members, None)))
1584+
}
15791585
}
15801586

15811587
#[cfg(test)]
@@ -1602,8 +1608,8 @@ mod tests {
16021608
let false_lit = Type::Literal(Lit::Bool(false));
16031609
let none = Type::None;
16041610

1605-
let str_opt = Type::Union(vec![s, none.clone()]);
1606-
let false_opt = Type::Union(vec![false_lit, none]);
1611+
let str_opt = Type::union(vec![s, none.clone()]);
1612+
let false_opt = Type::union(vec![false_lit, none]);
16071613

16081614
assert_eq!(str_opt.as_bool(), None);
16091615
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
@@ -1795,12 +1795,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
17951795
Type::SuperInstance(box (cls, obj)) => {
17961796
acc.push(AttributeBase1::SuperInstance(cls, obj))
17971797
}
1798-
Type::Union(members) => {
1798+
Type::Union(box (members, _)) => {
17991799
for ty in members {
18001800
self.as_attribute_base1(ty, acc)
18011801
}
18021802
}
1803-
Type::Type(box Type::Union(members)) => {
1803+
Type::Type(box Type::Union(box (members, _))) => {
18041804
for ty in members {
18051805
self.as_attribute_base1(Type::type_form(ty), acc)
18061806
}

‎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
@@ -944,7 +944,7 @@ fn make_bound_method_helper(
944944
Type::Overload(overload) if should_bind2(&overload.metadata) => {
945945
BoundMethodType::Overload(overload)
946946
}
947-
Type::Union(ref ts) => {
947+
Type::Union(box (ref ts, _)) => {
948948
let mut bound_methods = Vec::with_capacity(ts.len());
949949
for t in ts {
950950
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)