Skip to content

Commit 3b1acd1

Browse files
fix
1 parent b5843bd commit 3b1acd1

File tree

4 files changed

+328
-122
lines changed

4 files changed

+328
-122
lines changed

crates/pyrefly_types/src/display.rs

Lines changed: 58 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@
77

88
//! Display a type. The complexity comes from if we have two classes with the same name,
99
//! we want to display disambiguating information (e.g. module name or location).
10+
use std::borrow::Cow;
1011
use std::fmt;
1112
use std::fmt::Display;
1213

13-
use pyrefly_python::module::TextRangeWithModule;
1414
use pyrefly_python::module_name::ModuleName;
1515
use pyrefly_python::qname::QName;
1616
use pyrefly_util::display::Fmt;
@@ -29,6 +29,7 @@ use crate::literal::Lit;
2929
use crate::tuple::Tuple;
3030
use crate::type_output::DisplayOutput;
3131
use crate::type_output::OutputWithLocations;
32+
use crate::type_output::TypeLabelPart;
3233
use crate::type_output::TypeOutput;
3334
use crate::types::AnyStyle;
3435
use crate::types::BoundMethod;
@@ -237,11 +238,12 @@ impl<'a> TypeDisplayContext<'a> {
237238
name: &str,
238239
output: &mut impl TypeOutput,
239240
) -> fmt::Result {
240-
if self.always_display_module_name {
241-
output.write_str(&format!("{}.{}", module, name))
242-
} else {
243-
output.write_str(name)
244-
}
241+
let module_name = ModuleName::from_str(module);
242+
output.write_symbol(
243+
module_name,
244+
Cow::Borrowed(name),
245+
self.always_display_module_name,
246+
)
245247
}
246248

247249
/// Helper function to format a sequence of types with a separator.
@@ -302,7 +304,8 @@ impl<'a> TypeDisplayContext<'a> {
302304
output.write_targs(class_type.targs())
303305
}
304306
Type::TypedDict(typed_dict) => {
305-
output.write_str("TypedDict[")?;
307+
self.maybe_fmt_with_module("typing", "TypedDict", output)?;
308+
output.write_str("[")?;
306309
output.write_qname(typed_dict.qname())?;
307310
output.write_targs(typed_dict.targs())?;
308311
output.write_str("]")
@@ -677,7 +680,10 @@ impl<'a> TypeDisplayContext<'a> {
677680
output.write_str(&format!("{q}"))?;
678681
output.write_str("]")
679682
}
680-
Type::SpecialForm(x) => output.write_str(&format!("{x}")),
683+
Type::SpecialForm(x) => {
684+
let name = x.to_string();
685+
self.maybe_fmt_with_module("typing", &name, output)
686+
}
681687
Type::Ellipsis => output.write_str("Ellipsis"),
682688
Type::Any(style) => match style {
683689
AnyStyle::Explicit => self.maybe_fmt_with_module("typing", "Any", output),
@@ -746,7 +752,7 @@ impl Type {
746752
c.display(self).to_string()
747753
}
748754

749-
pub fn get_types_with_locations(&self) -> Vec<(String, Option<TextRangeWithModule>)> {
755+
pub fn get_types_with_locations(&self) -> Vec<TypeLabelPart> {
750756
let ctx = TypeDisplayContext::new(&[self]);
751757
let mut output = OutputWithLocations::new(&ctx);
752758
ctx.fmt_helper_generic(self, false, &mut output).unwrap();
@@ -1616,32 +1622,30 @@ def overloaded_func[T](
16161622
}
16171623

16181624
// Helper functions for testing get_types_with_location
1619-
fn get_parts(t: &Type) -> Vec<(String, Option<TextRangeWithModule>)> {
1625+
fn get_parts(t: &Type) -> Vec<TypeLabelPart> {
16201626
let ctx = TypeDisplayContext::new(&[t]);
16211627
let output = ctx.get_types_with_location(t, false);
16221628
output.parts().to_vec()
16231629
}
16241630

1625-
fn parts_to_string(parts: &[(String, Option<TextRangeWithModule>)]) -> String {
1626-
parts.iter().map(|(s, _)| s.as_str()).collect::<String>()
1631+
fn parts_to_string(parts: &[TypeLabelPart]) -> String {
1632+
parts
1633+
.iter()
1634+
.map(|part| part.text.as_str())
1635+
.collect::<String>()
16271636
}
16281637

1629-
fn assert_part_has_location(
1630-
parts: &[(String, Option<TextRangeWithModule>)],
1631-
name: &str,
1632-
module: &str,
1633-
position: u32,
1634-
) {
1635-
let part = parts.iter().find(|(s, _)| s == name);
1638+
fn assert_part_has_location(parts: &[TypeLabelPart], name: &str, module: &str, position: u32) {
1639+
let part = parts.iter().find(|part| part.text == name);
16361640
assert!(part.is_some(), "Should have {} in parts", name);
1637-
let (_, location) = part.unwrap();
1638-
assert!(location.is_some(), "{} should have location", name);
1639-
let loc = location.as_ref().unwrap();
1641+
let loc = part.unwrap().location.as_ref();
1642+
assert!(loc.is_some(), "{} should have location", name);
1643+
let loc = loc.unwrap();
16401644
assert_eq!(loc.module.name().as_str(), module);
16411645
assert_eq!(loc.range.start().to_u32(), position);
16421646
}
16431647

1644-
fn assert_output_contains(parts: &[(String, Option<TextRangeWithModule>)], needle: &str) {
1648+
fn assert_output_contains(parts: &[TypeLabelPart], needle: &str) {
16451649
let full_str = parts_to_string(parts);
16461650
assert!(
16471651
full_str.contains(needle),
@@ -1670,9 +1674,12 @@ def overloaded_func[T](
16701674
let t = Type::ClassType(ClassType::new(foo, TArgs::new(tparams, vec![inner_type])));
16711675
let parts = get_parts(&t);
16721676

1673-
assert_eq!(parts[0].0, "Foo");
1677+
assert_eq!(parts[0].text, "Foo");
16741678
assert_part_has_location(&parts, "Foo", "test.module", 10);
1675-
assert!(parts.iter().any(|(s, _)| s == "Bar"), "Should have Bar");
1679+
assert!(
1680+
parts.iter().any(|part| part.text == "Bar"),
1681+
"Should have Bar"
1682+
);
16761683
}
16771684

16781685
#[test]
@@ -1681,8 +1688,11 @@ def overloaded_func[T](
16811688
let t = tvar.to_type();
16821689
let parts = get_parts(&t);
16831690

1684-
assert_eq!(parts[0].0, "TypeVar[");
1685-
assert!(parts[0].1.is_none(), "TypeVar[ should not have location");
1691+
assert_eq!(parts[0].text, "TypeVar[");
1692+
assert!(
1693+
parts[0].location.is_none(),
1694+
"TypeVar[ should not have location"
1695+
);
16861696
assert_part_has_location(&parts, "T", "test.module", 15);
16871697
}
16881698

@@ -1698,8 +1708,14 @@ def overloaded_func[T](
16981708
let parts1 = ctx.get_types_with_location(&t1, false).parts().to_vec();
16991709
let parts2 = ctx.get_types_with_location(&t2, false).parts().to_vec();
17001710

1701-
let loc1 = parts1.iter().find_map(|(_, loc)| loc.as_ref()).unwrap();
1702-
let loc2 = parts2.iter().find_map(|(_, loc)| loc.as_ref()).unwrap();
1711+
let loc1 = parts1
1712+
.iter()
1713+
.find_map(|part| part.location.as_ref())
1714+
.unwrap();
1715+
let loc2 = parts2
1716+
.iter()
1717+
.find_map(|part| part.location.as_ref())
1718+
.unwrap();
17031719
assert_ne!(
17041720
loc1.range.start().to_u32(),
17051721
loc2.range.start().to_u32(),
@@ -1714,6 +1730,11 @@ def overloaded_func[T](
17141730

17151731
assert_output_contains(&parts, "Literal");
17161732
assert_output_contains(&parts, "True");
1733+
let literal_part = parts.iter().find(|part| part.text == "Literal");
1734+
assert!(literal_part.is_some());
1735+
let symbol = literal_part.unwrap().symbol.as_ref().unwrap();
1736+
assert_eq!(symbol.module.as_str(), "typing");
1737+
assert_eq!(symbol.name, "Literal");
17171738
}
17181739

17191740
#[test]
@@ -1738,8 +1759,8 @@ def overloaded_func[T](
17381759
let parts = get_parts(&t);
17391760

17401761
assert_eq!(parts.len(), 1);
1741-
assert_eq!(parts[0].0, "None");
1742-
assert!(parts[0].1.is_none(), "None should not have location");
1762+
assert_eq!(parts[0].text, "None");
1763+
assert!(parts[0].location.is_none(), "None should not have location");
17431764
}
17441765

17451766
#[test]
@@ -1765,7 +1786,11 @@ def overloaded_func[T](
17651786
for param in &["T", "U", "Ts"] {
17661787
assert_output_contains(&parts, param);
17671788
}
1768-
assert!(parts.iter().any(|(s, loc)| s == "[" && loc.is_none()));
1789+
assert!(
1790+
parts
1791+
.iter()
1792+
.any(|part| part.text == "[" && part.location.is_none())
1793+
);
17691794
assert!(parts_to_string(&parts).starts_with('['));
17701795
assert_output_contains(&parts, "](");
17711796
}
@@ -1796,7 +1821,7 @@ def overloaded_func[T](
17961821
for expected in &["Literal", "Color", "RED"] {
17971822
assert_output_contains(&parts, expected);
17981823
}
1799-
assert!(parts.iter().any(|(_, loc)| loc.is_some()));
1824+
assert!(parts.iter().any(|part| part.location.is_some()));
18001825
}
18011826

18021827
#[test]

0 commit comments

Comments
 (0)