Skip to content

Commit 58b1cd4

Browse files
yangdanny97meta-codesync[bot]
authored andcommitted
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 Reviewed By: rchen152
1 parent 29d542e commit 58b1cd4

File tree

28 files changed

+201
-94
lines changed

28 files changed

+201
-94
lines changed

crates/pyrefly_types/src/display.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ use crate::types::SuperObj;
4040
use crate::types::TArgs;
4141
use crate::types::TParam;
4242
use crate::types::Type;
43+
use crate::types::Union;
4344

4445
/// Information about the qnames we have seen.
4546
/// Set to None to indicate we have seen different values, or Some if they are all the same.
@@ -442,17 +443,24 @@ impl<'a> TypeDisplayContext<'a> {
442443
self.maybe_fmt_with_module("typing", "NoReturn", output)
443444
}
444445
Type::Never(NeverStyle::Never) => self.maybe_fmt_with_module("typing", "Never", output),
445-
Type::Union(types) if types.is_empty() => {
446+
Type::Union(box Union { members: types, .. }) if types.is_empty() => {
446447
self.maybe_fmt_with_module("typing", "Never", output)
447448
}
448-
Type::Union(types) => {
449+
Type::Union(box Union {
450+
display_name: Some(name),
451+
..
452+
}) => output.write_str(name),
453+
Type::Union(box Union {
454+
members,
455+
display_name: None,
456+
}) => {
449457
let mut literal_idx = None;
450458
let mut literals = Vec::new();
451459
let mut union_members: Vec<&Type> = Vec::new();
452460
// Track seen types to deduplicate (mainly to prettify types for functions with different names but the same signature)
453461
let mut seen_types = SmallSet::new();
454462

455-
for t in types.iter() {
463+
for t in members.iter() {
456464
match t {
457465
Type::Literal(lit) => {
458466
if literal_idx.is_none() {
@@ -1099,7 +1107,7 @@ pub mod tests {
10991107
assert_eq!(ctx.display(&int_type).to_string(), "int");
11001108
}
11011109

1102-
let union_foo_int = Type::Union(vec![foo_type, int_type]);
1110+
let union_foo_int = Type::union(vec![foo_type, int_type]);
11031111

11041112
{
11051113
let mut ctx = TypeDisplayContext::new(&[&union_foo_int]);
@@ -1115,11 +1123,11 @@ pub mod tests {
11151123
let t3 = fake_tyvar("qux", "bar", 2);
11161124

11171125
assert_eq!(
1118-
Type::Union(vec![t1.to_type(), t2.to_type()]).to_string(),
1126+
Type::union(vec![t1.to_type(), t2.to_type()]).to_string(),
11191127
"TypeVar[bar.foo@1:2] | TypeVar[bar.foo@1:3]"
11201128
);
11211129
assert_eq!(
1122-
Type::Union(vec![t1.to_type(), t3.to_type()]).to_string(),
1130+
Type::union(vec![t1.to_type(), t3.to_type()]).to_string(),
11231131
"TypeVar[foo] | TypeVar[qux]"
11241132
);
11251133
}
@@ -1159,13 +1167,14 @@ pub mod tests {
11591167
let nonlit2 = Type::LiteralString;
11601168

11611169
assert_eq!(
1162-
Type::Union(vec![nonlit1.clone(), nonlit2.clone()]).to_string(),
1170+
Type::union(vec![nonlit1.clone(), nonlit2.clone()]).to_string(),
11631171
"None | LiteralString"
11641172
);
11651173
assert_eq!(
1166-
Type::Union(vec![nonlit1, lit1, nonlit2, lit2]).to_string(),
1174+
Type::union(vec![nonlit1.clone(), lit1, nonlit2.clone(), lit2]).to_string(),
11671175
"None | Literal[True, 'test'] | LiteralString"
11681176
);
1177+
assert_eq!(Type::union(vec![nonlit1, nonlit2]).to_string(), "MyUnion");
11691178
}
11701179

11711180
#[test]
@@ -1575,7 +1584,7 @@ def overloaded_func[T](
15751584

15761585
#[test]
15771586
fn test_union_of_intersection() {
1578-
let x = Type::Union(vec![
1587+
let x = Type::union(vec![
15791588
Type::Intersect(Box::new((
15801589
vec![Type::any_explicit(), Type::LiteralString],
15811590
Type::any_implicit(),

crates/pyrefly_types/src/simplify.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,14 @@ use crate::literal::Lit;
1414
use crate::stdlib::Stdlib;
1515
use crate::tuple::Tuple;
1616
use crate::types::Type;
17+
use crate::types::Union;
1718

1819
/// Turn unions of unions into a flattened list for one union, and return the deduped list.
1920
fn flatten_and_dedup(xs: Vec<Type>) -> Vec<Type> {
2021
fn flatten(xs: Vec<Type>, res: &mut Vec<Type>) {
2122
for x in xs {
2223
match x {
23-
Type::Union(xs) => flatten(xs, res),
24+
Type::Union(box Union { members, .. }) => flatten(members, res),
2425
Type::Never(_) => {}
2526
_ => res.push(x),
2627
}
@@ -76,7 +77,12 @@ fn unions_internal(
7677
}
7778
collapse_tuple_unions_with_empty(&mut res);
7879
// `res` is collapsible again if `flatten_and_dedup` drops `xs` to 0 or 1 elements
79-
try_collapse(res).unwrap_or_else(Type::Union)
80+
try_collapse(res).unwrap_or_else(|members| {
81+
Type::Union(Box::new(Union {
82+
members,
83+
display_name: None,
84+
}))
85+
})
8086
})
8187
}
8288

crates/pyrefly_types/src/type_output.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ mod tests {
402402

403403
let int_type = Type::ClassType(ClassType::new(int_class, TArgs::default()));
404404
let str_type = Type::ClassType(ClassType::new(str_class, TArgs::default()));
405-
let union_type = Type::Union(vec![int_type, str_type, Type::None]);
405+
let union_type = Type::union(vec![int_type, str_type, Type::None]);
406406

407407
let ctx = TypeDisplayContext::new(&[&union_type]);
408408
let mut output = OutputWithLocations::new(&ctx);

crates/pyrefly_types/src/types.rs

Lines changed: 61 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ use std::borrow::Cow;
99
use std::cmp::Ordering;
1010
use std::fmt;
1111
use std::fmt::Display;
12+
use std::hash::Hash;
13+
use std::hash::Hasher;
1214
use std::sync::Arc;
1315

1416
use dupe::Dupe;
@@ -611,6 +613,40 @@ pub enum SuperObj {
611613
Class(ClassType),
612614
}
613615

616+
#[derive(Debug, Clone, Eq, TypeEq, PartialOrd, Ord)]
617+
pub struct Union {
618+
pub members: Vec<Type>,
619+
pub display_name: Option<String>,
620+
}
621+
622+
impl PartialEq for Union {
623+
fn eq(&self, other: &Self) -> bool {
624+
self.members == other.members
625+
}
626+
}
627+
628+
impl Hash for Union {
629+
fn hash<H: Hasher>(&self, state: &mut H) {
630+
self.members.hash(state)
631+
}
632+
}
633+
634+
impl Visit<Type> for Union {
635+
fn recurse<'a>(&'a self, f: &mut dyn FnMut(&'a Type)) {
636+
for member in &self.members {
637+
member.visit(f);
638+
}
639+
}
640+
}
641+
642+
impl VisitMut<Type> for Union {
643+
fn recurse_mut(&mut self, f: &mut dyn FnMut(&mut Type)) {
644+
for member in &mut self.members {
645+
member.visit_mut(f);
646+
}
647+
}
648+
}
649+
614650
// Note: The fact that Literal and LiteralString are at the front is important for
615651
// optimisations in `unions_with_literals`.
616652
#[derive(Debug, Clone, PartialEq, Eq, TypeEq, PartialOrd, Ord, Hash)]
@@ -626,7 +662,8 @@ pub enum Type {
626662
BoundMethod(Box<BoundMethod>),
627663
/// An overloaded function.
628664
Overload(Overload),
629-
Union(Vec<Type>),
665+
/// Unions will hold an optional name to use when displaying the type
666+
Union(Box<Union>),
630667
/// Our intersection support is partial, so we store a fallback type that we use for operations
631668
/// that are not yet supported on intersections.
632669
Intersect(Box<(Vec<Type>, Type)>),
@@ -1392,7 +1429,7 @@ impl Type {
13921429
/// type[a | b] -> type[a] | type[b]
13931430
pub fn distribute_type_over_union(self) -> Self {
13941431
self.transform(&mut |ty| {
1395-
if let Type::Type(box Type::Union(members)) = ty {
1432+
if let Type::Type(box Type::Union(box Union { members, .. })) = ty {
13961433
*ty = unions(members.drain(..).map(Type::type_form).collect());
13971434
}
13981435
})
@@ -1437,7 +1474,7 @@ impl Type {
14371474

14381475
pub fn sort_unions(self) -> Self {
14391476
self.transform(&mut |ty| {
1440-
if let Type::Union(ts) = ty {
1477+
if let Type::Union(box Union { members: ts, .. }) = ty {
14411478
ts.sort();
14421479
}
14431480
})
@@ -1488,27 +1525,30 @@ impl Type {
14881525

14891526
pub fn into_unions(self) -> Vec<Type> {
14901527
match self {
1491-
Type::Union(types) => types,
1528+
Type::Union(box Union { members: types, .. }) => types,
14921529
_ => vec![self],
14931530
}
14941531
}
14951532

14961533
/// Create an optional type (union with None).
14971534
pub fn optional(x: Self) -> Self {
14981535
// We would like the resulting type not nested, and well sorted.
1499-
if let Type::Union(mut xs) = x {
1536+
if let Type::Union(box Union {
1537+
members: mut xs, ..
1538+
}) = x
1539+
{
15001540
match xs.binary_search(&Type::None) {
1501-
Ok(_) => Type::Union(xs),
1541+
Ok(_) => Type::union(xs),
15021542
Err(i) => {
15031543
xs.insert(i, Type::None);
1504-
Type::Union(xs)
1544+
Type::union(xs)
15051545
}
15061546
}
15071547
} else {
15081548
match x.cmp(&Type::None) {
15091549
Ordering::Equal => Type::None,
1510-
Ordering::Less => Type::Union(vec![x, Type::None]),
1511-
Ordering::Greater => Type::Union(vec![Type::None, x]),
1550+
Ordering::Less => Type::union(vec![x, Type::None]),
1551+
Ordering::Greater => Type::union(vec![Type::None, x]),
15121552
}
15131553
}
15141554
}
@@ -1538,9 +1578,9 @@ impl Type {
15381578
Type::Literal(Lit::Str(x)) => Some(!x.is_empty()),
15391579
Type::None => Some(false),
15401580
Type::Tuple(Tuple::Concrete(elements)) => Some(!elements.is_empty()),
1541-
Type::Union(options) => {
1581+
Type::Union(box Union { members, .. }) => {
15421582
let mut answer = None;
1543-
for option in options {
1583+
for option in members {
15441584
let option_bool = option.as_bool();
15451585
option_bool?;
15461586
if answer.is_none() {
@@ -1590,6 +1630,14 @@ impl Type {
15901630
})
15911631
})
15921632
}
1633+
1634+
/// Creates a union from the provided types without simplifying
1635+
pub fn union(members: Vec<Type>) -> Self {
1636+
Type::Union(Box::new(Union {
1637+
members,
1638+
display_name: None,
1639+
}))
1640+
}
15931641
}
15941642

15951643
#[cfg(test)]
@@ -1616,8 +1664,8 @@ mod tests {
16161664
let false_lit = Type::Literal(Lit::Bool(false));
16171665
let none = Type::None;
16181666

1619-
let str_opt = Type::Union(vec![s, none.clone()]);
1620-
let false_opt = Type::Union(vec![false_lit, none]);
1667+
let str_opt = Type::union(vec![s, none.clone()]);
1668+
let false_opt = Type::union(vec![false_lit, none]);
16211669

16221670
assert_eq!(str_opt.as_bool(), None);
16231671
assert_eq!(false_opt.as_bool(), Some(false));

pyrefly/lib/alt/answers_solver.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use dupe::IterDupedExt;
1717
use itertools::Either;
1818
use pyrefly_python::module_name::ModuleName;
1919
use pyrefly_python::module_path::ModulePath;
20+
use pyrefly_types::types::Union;
2021
use pyrefly_util::display::DisplayWithCtx;
2122
use pyrefly_util::display::commas_iter;
2223
use pyrefly_util::recurser::Guard;
@@ -797,12 +798,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
797798
fn go(&mut self, ty: &Type, in_type: bool) {
798799
match ty {
799800
Type::Never(_) if !in_type => (),
800-
Type::Union(tys) => {
801+
Type::Union(box Union { members, .. }) => {
801802
self.seen_union = true;
802-
tys.iter().for_each(|ty| self.go(ty, in_type))
803+
members.iter().for_each(|ty| self.go(ty, in_type))
803804
}
804-
Type::Type(box Type::Union(tys)) if !in_type => {
805-
tys.iter().for_each(|ty| self.go(ty, true))
805+
Type::Type(box Type::Union(box Union { members, .. })) if !in_type => {
806+
members.iter().for_each(|ty| self.go(ty, true))
806807
}
807808
Type::Var(v) if let Some(_guard) = self.me.recurse(*v) => {
808809
self.go(&self.me.solver().force_var(*v), in_type)

pyrefly/lib/alt/attr.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use pyrefly_types::special_form::SpecialForm;
1616
use pyrefly_types::types::Forall;
1717
use pyrefly_types::types::Forallable;
1818
use pyrefly_types::types::TArgs;
19+
use pyrefly_types::types::Union;
1920
use pyrefly_types::types::Var;
2021
use ruff_python_ast::helpers::is_dunder;
2122
use ruff_python_ast::name::Name;
@@ -1795,12 +1796,12 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
17951796
Type::SuperInstance(box (cls, obj)) => {
17961797
acc.push(AttributeBase1::SuperInstance(cls, obj))
17971798
}
1798-
Type::Union(members) => {
1799+
Type::Union(box Union { members, .. }) => {
17991800
for ty in members {
18001801
self.as_attribute_base1(ty, acc)
18011802
}
18021803
}
1803-
Type::Type(box Type::Union(members)) => {
1804+
Type::Type(box Type::Union(box Union { members, .. })) => {
18041805
for ty in members {
18051806
self.as_attribute_base1(Type::type_form(ty), acc)
18061807
}

pyrefly/lib/alt/call.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use pyrefly_types::quantified::Quantified;
1414
use pyrefly_types::types::CalleeKind;
1515
use pyrefly_types::types::TArgs;
1616
use pyrefly_types::types::TParams;
17+
use pyrefly_types::types::Union;
1718
use pyrefly_util::prelude::SliceExt;
1819
use pyrefly_util::prelude::VecExt;
1920
use ruff_python_ast::Arguments;
@@ -260,7 +261,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
260261
Type::Var(v) if let Some(_guard) = self.recurse(v) => {
261262
self.as_call_target_impl(self.solver().force_var(v), quantified, dunder_call)
262263
}
263-
Type::Union(xs) => {
264+
Type::Union(box Union { members: xs, .. }) => {
264265
let xs_length = xs.len();
265266
let targets = xs
266267
.into_iter()
@@ -315,7 +316,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
315316
Type::Quantified(q) if q.is_type_var() => match q.restriction() {
316317
Restriction::Unrestricted => CallTargetLookup::Error(vec![]),
317318
Restriction::Bound(bound) => match bound {
318-
Type::Union(members) => {
319+
Type::Union(box Union { members, .. }) => {
319320
let mut targets = Vec::new();
320321
for member in members {
321322
if let CallTargetLookup::Ok(target) = self.as_call_target_impl(

pyrefly/lib/alt/class/class_field.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use pyrefly_types::callable::Params;
2222
use pyrefly_types::simplify::unions;
2323
use pyrefly_types::type_var::Restriction;
2424
use pyrefly_types::types::TParams;
25+
use pyrefly_types::types::Union;
2526
use pyrefly_util::owner::Owner;
2627
use pyrefly_util::prelude::ResultExt;
2728
use pyrefly_util::visit::Visit;
@@ -944,7 +945,9 @@ fn make_bound_method_helper(
944945
Type::Overload(overload) if should_bind2(&overload.metadata) => {
945946
BoundMethodType::Overload(overload)
946947
}
947-
Type::Union(ref ts) => {
948+
Type::Union(box Union {
949+
members: ref ts, ..
950+
}) => {
948951
let mut bound_methods = Vec::with_capacity(ts.len());
949952
for t in ts {
950953
match make_bound_method_helper(obj.clone(), t.clone(), should_bind) {

pyrefly/lib/alt/class/classdef.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use dupe::Dupe;
1111
use pyrefly_python::nesting_context::NestingContext;
1212
use pyrefly_types::callable::Callable;
1313
use pyrefly_types::special_form::SpecialForm;
14+
use pyrefly_types::types::Union;
1415
use ruff_python_ast::Identifier;
1516
use ruff_python_ast::StmtClassDef;
1617
use ruff_python_ast::name::Name;
@@ -51,7 +52,7 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
5152
Type::SelfType(ty_cls) | Type::ClassType(ty_cls) => {
5253
self.has_superclass(ty_cls.class_object(), class)
5354
}
54-
Type::Union(xs) => xs
55+
Type::Union(box Union { members: xs, .. }) => xs
5556
.iter()
5657
.all(|x| self.is_compatible_constructor_return(x, class)),
5758
_ => false,

0 commit comments

Comments
 (0)