Skip to content

Commit b233cfe

Browse files
fix
1 parent 2a52aa9 commit b233cfe

File tree

4 files changed

+91
-10
lines changed

4 files changed

+91
-10
lines changed

crates/pyrefly_types/src/display.rs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use starlark_map::smallmap;
2626
use crate::callable::Function;
2727
use crate::class::Class;
2828
use crate::literal::Lit;
29+
use crate::stdlib::Stdlib;
2930
use crate::tuple::Tuple;
3031
use crate::type_output::DisplayOutput;
3132
use crate::type_output::OutputWithLocations;
@@ -92,11 +93,19 @@ pub struct TypeDisplayContext<'a> {
9293
/// Should we display for IDE Hover? This makes type names more readable but less precise.
9394
hover: bool,
9495
always_display_module_name: bool,
96+
tuple_qname: Option<&'a QName>,
9597
}
9698

9799
impl<'a> TypeDisplayContext<'a> {
98100
pub fn new(xs: &[&'a Type]) -> Self {
99-
let mut res = Self::default();
101+
Self::new_with_tuple(xs, None)
102+
}
103+
104+
pub fn new_with_tuple(xs: &[&'a Type], tuple_qname: Option<&'a QName>) -> Self {
105+
let mut res = Self {
106+
tuple_qname,
107+
..Self::default()
108+
};
100109
for x in xs {
101110
res.add(x);
102111
}
@@ -120,6 +129,10 @@ impl<'a> TypeDisplayContext<'a> {
120129
})
121130
}
122131

132+
pub(crate) fn tuple_qname(&self) -> Option<&'a QName> {
133+
self.tuple_qname
134+
}
135+
123136
/// Force that we always display at least the module name for qualified names.
124137
pub fn always_display_module_name(&mut self) {
125138
// We pretend that every qname is also in a fake module, and thus requires disambiguating.
@@ -786,7 +799,21 @@ impl Type {
786799
}
787800

788801
pub fn get_types_with_locations(&self) -> Vec<(String, Option<TextRangeWithModule>)> {
789-
let ctx = TypeDisplayContext::new(&[self]);
802+
self.get_types_with_locations_with_tuple_qname(None)
803+
}
804+
805+
pub fn get_types_with_locations_with_stdlib(
806+
&self,
807+
stdlib: &Stdlib,
808+
) -> Vec<(String, Option<TextRangeWithModule>)> {
809+
self.get_types_with_locations_with_tuple_qname(Some(stdlib.tuple_object().qname()))
810+
}
811+
812+
fn get_types_with_locations_with_tuple_qname(
813+
&self,
814+
tuple_qname: Option<&QName>,
815+
) -> Vec<(String, Option<TextRangeWithModule>)> {
816+
let ctx = TypeDisplayContext::new_with_tuple(&[self], tuple_qname);
790817
let mut output = OutputWithLocations::new(&ctx);
791818
ctx.fmt_helper_generic(self, false, &mut output).unwrap();
792819
output.parts().to_vec()
@@ -1661,6 +1688,15 @@ def overloaded_func[T](
16611688
output.parts().to_vec()
16621689
}
16631690

1691+
fn get_parts_with_tuple_qname(
1692+
t: &Type,
1693+
tuple_qname: &pyrefly_python::qname::QName,
1694+
) -> Vec<(String, Option<TextRangeWithModule>)> {
1695+
let ctx = TypeDisplayContext::new_with_tuple(&[t], Some(tuple_qname));
1696+
let output = ctx.get_types_with_location(t, false);
1697+
output.parts().to_vec()
1698+
}
1699+
16641700
fn parts_to_string(parts: &[(String, Option<TextRangeWithModule>)]) -> String {
16651701
parts.iter().map(|(s, _)| s.as_str()).collect::<String>()
16661702
}
@@ -1865,20 +1901,25 @@ def overloaded_func[T](
18651901
let bar = fake_class("Bar", "test", 55);
18661902
let foo_type = Type::ClassType(ClassType::new(foo, TArgs::default()));
18671903
let bar_type = Type::ClassType(ClassType::new(bar, TArgs::default()));
1904+
let tuple_cls = fake_class("tuple", "builtins", 5);
18681905

18691906
// Test concrete tuple: tuple[Foo, Bar]
18701907
let concrete_tuple = Type::Tuple(Tuple::Concrete(vec![foo_type.clone(), bar_type.clone()]));
18711908
let parts = get_parts(&concrete_tuple);
18721909
for expected in &["tuple", "Foo", "Bar"] {
18731910
assert_output_contains(&parts, expected);
18741911
}
1912+
let parts_with_location = get_parts_with_tuple_qname(&concrete_tuple, tuple_cls.qname());
1913+
assert_part_has_location(&parts_with_location, "tuple", "builtins", 5);
18751914

18761915
// Test unbounded tuple: tuple[Foo, ...]
18771916
let unbounded_tuple = Type::Tuple(Tuple::Unbounded(Box::new(foo_type)));
18781917
let parts2 = get_parts(&unbounded_tuple);
18791918
for expected in &["tuple", "Foo", "..."] {
18801919
assert_output_contains(&parts2, expected);
18811920
}
1921+
let parts_unbounded = get_parts_with_tuple_qname(&unbounded_tuple, tuple_cls.qname());
1922+
assert_part_has_location(&parts_unbounded, "tuple", "builtins", 5);
18821923
}
18831924

18841925
#[test]

crates/pyrefly_types/src/tuple.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ impl Tuple {
6464
output: &mut O,
6565
write_type: &impl Fn(&Type, &mut O) -> fmt::Result,
6666
) -> fmt::Result {
67-
output.write_str("tuple")?;
67+
output.write_tuple_keyword()?;
6868
output.write_str("[")?;
6969
match self {
7070
Self::Concrete(elts) => {

crates/pyrefly_types/src/type_output.rs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ pub trait TypeOutput {
2626
fn write_lit(&mut self, lit: &Lit) -> fmt::Result;
2727
fn write_targs(&mut self, targs: &TArgs) -> fmt::Result;
2828
fn write_type(&mut self, ty: &Type) -> fmt::Result;
29+
fn write_tuple_keyword(&mut self) -> fmt::Result {
30+
self.write_str("tuple")
31+
}
2932
}
3033

3134
/// Implementation of `TypeOutput` that writes formatted types to plain text.
@@ -140,6 +143,16 @@ impl TypeOutput for OutputWithLocations<'_> {
140143
// Format the type and extract location if it has a qname
141144
self.context.fmt_helper_generic(ty, false, self)
142145
}
146+
147+
fn write_tuple_keyword(&mut self) -> fmt::Result {
148+
if let Some(qname) = self.context.tuple_qname() {
149+
let location = TextRangeWithModule::new(qname.module().clone(), qname.range());
150+
self.parts.push((qname.id().to_string(), Some(location)));
151+
Ok(())
152+
} else {
153+
self.write_str("tuple")
154+
}
155+
}
143156
}
144157

145158
#[cfg(test)]
@@ -482,9 +495,8 @@ mod tests {
482495

483496
#[test]
484497
fn test_output_with_locations_tuple_base_not_clickable() {
485-
// TODO(jvansch): When implementing clickable support for the base type in generics like tuple[int],
486-
// update this test to verify that "tuple" has a location and is clickable.
487-
// Expected future behavior: [("tuple", Some(location)), ("[", None), ("int", Some(location)), ("]", None)]
498+
// Without stdlib context we don't attach a location to the `tuple` keyword.
499+
// (See `test_output_with_locations_tuple_base_clickable_when_qname_available`.)
488500

489501
// Create tuple[int] type
490502
let int_class = fake_class("int", "builtins", 10);
@@ -522,6 +534,32 @@ mod tests {
522534
assert!(parts[3].1.is_none(), "] should not have location");
523535
}
524536

537+
#[test]
538+
fn test_output_with_locations_tuple_base_clickable_when_qname_available() {
539+
let tuple_cls = fake_class("tuple", "builtins", 5);
540+
let int_class = fake_class("int", "builtins", 10);
541+
let int_type = Type::ClassType(ClassType::new(int_class, TArgs::default()));
542+
let tuple_type = Type::Tuple(Tuple::Concrete(vec![int_type]));
543+
544+
let ctx = TypeDisplayContext::new_with_tuple(&[&tuple_type], Some(tuple_cls.qname()));
545+
let mut output = OutputWithLocations::new(&ctx);
546+
547+
ctx.fmt_helper_generic(&tuple_type, false, &mut output)
548+
.unwrap();
549+
550+
let tuple_part = output
551+
.parts()
552+
.iter()
553+
.find(|(text, _)| text == "tuple")
554+
.expect("tuple keyword should be present");
555+
let location = tuple_part
556+
.1
557+
.as_ref()
558+
.expect("tuple should now have a location");
559+
assert_eq!(location.module.name(), ModuleName::from_str("builtins"));
560+
assert_eq!(location.range.start(), TextSize::new(5));
561+
}
562+
525563
#[test]
526564
fn test_output_with_locations_literal_base_not_clickable() {
527565
// TODO(jvansch): When implementing clickable support for the base type in special forms like Literal[1],

pyrefly/lib/lsp/wasm/inlay_hints.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ impl<'a> Transaction<'a> {
119119
}
120120
};
121121
let bindings = self.get_bindings(handle)?;
122+
let stdlib = self.get_stdlib(handle);
122123
let mut res = Vec::new();
123124
for idx in bindings.keys::<Key>() {
124125
match bindings.idx_to_key(idx) {
@@ -139,8 +140,9 @@ impl<'a> Transaction<'a> {
139140
{
140141
ty = return_ty;
141142
}
142-
// Use get_types_with_locations to get type parts with location info
143-
let type_parts = ty.get_types_with_locations();
143+
// Use get_types_with_locations_with_stdlib to get type parts with location info
144+
let type_parts =
145+
ty.get_types_with_locations_with_stdlib(stdlib.as_ref());
144146
let label_parts = once((" -> ".to_owned(), None))
145147
.chain(
146148
type_parts
@@ -181,8 +183,8 @@ impl<'a> Transaction<'a> {
181183
if let Some(e) = e
182184
&& is_interesting(e, &ty, class_name)
183185
{
184-
// Use get_types_with_locations to get type parts with location info
185-
let type_parts = ty.get_types_with_locations();
186+
// Use get_types_with_locations_with_stdlib to get type parts with location info
187+
let type_parts = ty.get_types_with_locations_with_stdlib(stdlib.as_ref());
186188
let label_parts = once((": ".to_owned(), None))
187189
.chain(
188190
type_parts

0 commit comments

Comments
 (0)