fix Tuple Types in type hints are currently not clickable #1680

crates/pyrefly_types/src/display.rs

@@ -26,6 +26,7 @@ use starlark_map::smallmap;
 use crate::callable::Function;
 use crate::class::Class;
 use crate::literal::Lit;
+use crate::stdlib::Stdlib;
 use crate::tuple::Tuple;
 use crate::type_output::DisplayOutput;
 use crate::type_output::OutputWithLocations;
@@ -92,6 +93,7 @@ pub struct TypeDisplayContext<'a> {
     /// Should we display for IDE Hover? This makes type names more readable but less precise.
     hover: bool,
     always_display_module_name: bool,
+    extra_symbol_qnames: SmallMap<&'static str, &'a QName>,
 }
 
 impl<'a> TypeDisplayContext<'a> {
@@ -120,6 +122,15 @@ impl<'a> TypeDisplayContext<'a> {
         })
     }
 
+    pub fn add_symbol_qname(&mut self, symbol: &'static str, qname: &'a QName) {
+        self.add_qname(qname);
+        self.extra_symbol_qnames.insert(symbol, qname);
+    }
+
+    pub(crate) fn symbol_qname(&self, symbol: &'static str) -> Option<&'a QName> {
+        self.extra_symbol_qnames.get(symbol).copied()
+    }
+
     /// Force that we always display at least the module name for qualified names.
     pub fn always_display_module_name(&mut self) {
         // We pretend that every qname is also in a fake module, and thus requires disambiguating.
@@ -572,9 +583,11 @@ impl<'a> TypeDisplayContext<'a> {
                 }
             }
             Type::Intersect(x) => self.fmt_type_sequence(x.0.iter(), " & ", true, output),
-            Type::Tuple(t) => {
-                t.fmt_with_type(output, &|ty, o| self.fmt_helper_generic(ty, false, o))
-            }
+            Type::Tuple(t) => t.fmt_with_type(
+                output,
+                &|ty, o| self.fmt_helper_generic(ty, false, o),
+                self.symbol_qname("tuple"),
+            ),
             Type::Forall(box Forall {
                 tparams,
                 body: body @ Forallable::Callable(c),
@@ -785,8 +798,12 @@ impl Type {
         c.display(self).to_string()
     }
 
-    pub fn get_types_with_locations(&self) -> Vec<(String, Option<TextRangeWithModule>)> {
-        let ctx = TypeDisplayContext::new(&[self]);
+    pub fn get_types_with_locations(
+        &self,
+        stdlib: &Stdlib,
+    ) -> Vec<(String, Option<TextRangeWithModule>)> {
+        let mut ctx = TypeDisplayContext::new(&[self]);
+        ctx.add_symbol_qname("tuple", stdlib.tuple_object().qname());
         let mut output = OutputWithLocations::new(&ctx);
         ctx.fmt_helper_generic(self, false, &mut output).unwrap();
         output.parts().to_vec()
@@ -1661,6 +1678,16 @@ def overloaded_func[T](
         output.parts().to_vec()
     }
 
+    fn get_parts_with_tuple_qname(
+        t: &Type,
+        tuple_qname: &pyrefly_python::qname::QName,
+    ) -> Vec<(String, Option<TextRangeWithModule>)> {
+        let mut ctx = TypeDisplayContext::new(&[t]);
+        ctx.add_symbol_qname("tuple", tuple_qname);
+        let output = ctx.get_types_with_location(t, false);
+        output.parts().to_vec()
+    }
+
     fn parts_to_string(parts: &[(String, Option<TextRangeWithModule>)]) -> String {
         parts.iter().map(|(s, _)| s.as_str()).collect::<String>()
     }
@@ -1865,20 +1892,25 @@ def overloaded_func[T](
         let bar = fake_class("Bar", "test", 55);
         let foo_type = Type::ClassType(ClassType::new(foo, TArgs::default()));
         let bar_type = Type::ClassType(ClassType::new(bar, TArgs::default()));
+        let tuple_cls = fake_class("tuple", "builtins", 5);
 
         // Test concrete tuple: tuple[Foo, Bar]
         let concrete_tuple = Type::Tuple(Tuple::Concrete(vec![foo_type.clone(), bar_type.clone()]));
         let parts = get_parts(&concrete_tuple);
         for expected in &["tuple", "Foo", "Bar"] {
             assert_output_contains(&parts, expected);
         }
+        let parts_with_location = get_parts_with_tuple_qname(&concrete_tuple, tuple_cls.qname());
+        assert_part_has_location(&parts_with_location, "tuple", "builtins", 5);
 
         // Test unbounded tuple: tuple[Foo, ...]
         let unbounded_tuple = Type::Tuple(Tuple::Unbounded(Box::new(foo_type)));
         let parts2 = get_parts(&unbounded_tuple);
         for expected in &["tuple", "Foo", "..."] {
             assert_output_contains(&parts2, expected);
         }
+        let parts_unbounded = get_parts_with_tuple_qname(&unbounded_tuple, tuple_cls.qname());
+        assert_part_has_location(&parts_unbounded, "tuple", "builtins", 5);
     }
 
     #[test]

crates/pyrefly_types/src/tuple.rs

@@ -10,6 +10,7 @@ use std::fmt;
 use pyrefly_derive::TypeEq;
 use pyrefly_derive::Visit;
 use pyrefly_derive::VisitMut;
+use pyrefly_python::qname::QName;
 
 use crate::type_output::TypeOutput;
 use crate::types::Type;
@@ -63,8 +64,13 @@ impl Tuple {
         &self,
         output: &mut O,
         write_type: &impl Fn(&Type, &mut O) -> fmt::Result,
+        tuple_qname: Option<&QName>,
     ) -> fmt::Result {
-        output.write_str("tuple")?;
+        if let Some(qname) = tuple_qname {
+            output.write_qname(qname)?;
+        } else {
+            output.write_str("tuple")?;
+        }
         output.write_str("[")?;
         match self {
             Self::Concrete(elts) => {

crates/pyrefly_types/src/type_output.rs

@@ -482,9 +482,8 @@ mod tests {
 
     #[test]
     fn test_output_with_locations_tuple_base_not_clickable() {
-        // TODO(jvansch): When implementing clickable support for the base type in generics like tuple[int],
-        // update this test to verify that "tuple" has a location and is clickable.
-        // Expected future behavior: [("tuple", Some(location)), ("[", None), ("int", Some(location)), ("]", None)]
+        // Without stdlib context we don't attach a location to the `tuple` keyword.
+        // (See `test_output_with_locations_tuple_base_clickable_when_qname_available`.)
 
         // Create tuple[int] type
         let int_class = fake_class("int", "builtins", 10);
@@ -522,6 +521,33 @@ mod tests {
         assert!(parts[3].1.is_none(), "] should not have location");
     }
 
+    #[test]
+    fn test_output_with_locations_tuple_base_clickable_when_qname_available() {
+        let tuple_cls = fake_class("tuple", "builtins", 5);
+        let int_class = fake_class("int", "builtins", 10);
+        let int_type = Type::ClassType(ClassType::new(int_class, TArgs::default()));
+        let tuple_type = Type::Tuple(Tuple::Concrete(vec![int_type]));
+
+        let mut ctx = TypeDisplayContext::new(&[&tuple_type]);
+        ctx.add_symbol_qname("tuple", tuple_cls.qname());
+        let mut output = OutputWithLocations::new(&ctx);
+
+        ctx.fmt_helper_generic(&tuple_type, false, &mut output)
+            .unwrap();
+
+        let tuple_part = output
+            .parts()
+            .iter()
+            .find(|(text, _)| text == "tuple")
+            .expect("tuple keyword should be present");
+        let location = tuple_part
+            .1
+            .as_ref()
+            .expect("tuple should now have a location");
+        assert_eq!(location.module.name(), ModuleName::from_str("builtins"));
+        assert_eq!(location.range.start(), TextSize::new(5));
+    }
+
     #[test]
     fn test_output_with_locations_literal_base_not_clickable() {
         // TODO(jvansch): When implementing clickable support for the base type in special forms like Literal[1],

pyrefly/lib/lsp/module_helpers.rs

@@ -17,6 +17,7 @@ use tracing::warn;
 use crate::module::bundled::BundledStub;
 use crate::module::typeshed::typeshed;
 use crate::module::typeshed_third_party::typeshed_third_party;
+use crate::types::stdlib::Stdlib;
 
 /// Convert to a path we can show to the user. The contents may not match the disk, but it has
 /// to be basically right.
@@ -50,9 +51,38 @@ pub fn to_real_path(path: &ModulePath) -> Option<PathBuf> {
     }
 }
 
+#[allow(dead_code)]
 pub fn collect_symbol_def_paths(t: &Type) -> Vec<(QName, PathBuf)> {
+    collect_symbol_def_paths_with_tuple_qname(t, None)
+}
+
+pub fn collect_symbol_def_paths_with_stdlib(t: &Type, stdlib: &Stdlib) -> Vec<(QName, PathBuf)> {
+    collect_symbol_def_paths_with_tuple_qname(t, Some(stdlib.tuple_object().qname().clone()))
+}
+
+fn collect_symbol_def_paths_with_tuple_qname(
+    t: &Type,
+    tuple_qname: Option<QName>,
+) -> Vec<(QName, PathBuf)> {
     let mut tracked_def_locs = SmallSet::new();
-    t.universe(&mut |t| tracked_def_locs.extend(t.qname()));
+    t.universe(&mut |t| {
+        if let Some(qname) = t.qname() {
+            tracked_def_locs.insert(qname.clone());
+        }
+    });
+
+    if let Some(tuple_qname) = tuple_qname {
+        let mut has_tuple = false;
+        t.universe(&mut |ty| {
+            if matches!(ty, Type::Tuple(_)) {
+                has_tuple = true;
+            }
+        });
+        if has_tuple {
+            tracked_def_locs.insert(tuple_qname);
+        }
+    }
+
     tracked_def_locs
         .into_iter()
         .map(|qname| {
@@ -64,7 +94,7 @@ pub fn collect_symbol_def_paths(t: &Type) -> Vec<(QName, PathBuf)> {
                 }
                 _ => module_path.as_path().to_path_buf(),
             };
-            (qname.clone(), file_path)
+            (qname, file_path)
         })
         .collect()
 }

pyrefly/lib/lsp/wasm/hover.rs

@@ -6,6 +6,7 @@
  */
 
 use std::collections::HashMap;
+use std::path::PathBuf;
 
 use lsp_types::Hover;
 use lsp_types::HoverContents;
@@ -18,6 +19,7 @@ use pyrefly_python::docstring::parse_parameter_documentation;
 use pyrefly_python::ignore::Ignore;
 use pyrefly_python::ignore::Tool;
 use pyrefly_python::ignore::find_comment_start_in_line;
+use pyrefly_python::qname::QName;
 use pyrefly_python::symbol_kind::SymbolKind;
 use pyrefly_types::callable::Callable;
 use pyrefly_types::callable::Param;
@@ -33,7 +35,7 @@ use ruff_text_size::TextSize;
 
 use crate::alt::answers_solver::AnswersSolver;
 use crate::error::error::Error;
-use crate::lsp::module_helpers::collect_symbol_def_paths;
+use crate::lsp::module_helpers::collect_symbol_def_paths_with_stdlib;
 use crate::state::lsp::DefinitionMetadata;
 use crate::state::lsp::FindDefinitionItemWithDocstring;
 use crate::state::lsp::FindPreference;
@@ -148,12 +150,12 @@ pub struct HoverValue {
     pub parameter_doc: Option<(String, String)>,
     pub display: Option<String>,
     pub show_go_to_links: bool,
+    pub symbol_def_paths: Vec<(QName, PathBuf)>,
 }
 
 impl HoverValue {
     #[cfg(not(target_arch = "wasm32"))]
-    fn format_symbol_def_locations(t: &Type) -> Option<String> {
-        let symbol_paths = collect_symbol_def_paths(t);
+    fn format_symbol_def_locations(symbol_paths: &[(QName, PathBuf)]) -> Option<String> {
         let linked_names = symbol_paths
             .into_iter()
             .filter_map(|(qname, file_path)| {
@@ -189,7 +191,7 @@ impl HoverValue {
     }
 
     #[cfg(target_arch = "wasm32")]
-    fn format_symbol_def_locations(t: &Type) -> Option<String> {
+    fn format_symbol_def_locations(_symbol_paths: &[(QName, PathBuf)]) -> Option<String> {
         None
     }
 
@@ -222,7 +224,7 @@ impl HoverValue {
             .as_ref()
             .map_or("".to_owned(), |s| format!("{s}: "));
         let symbol_def_formatted = if self.show_go_to_links {
-            HoverValue::format_symbol_def_locations(&self.type_).unwrap_or("".to_owned())
+            HoverValue::format_symbol_def_locations(&self.symbol_def_paths).unwrap_or("".to_owned())
         } else {
             String::new()
         };
@@ -404,6 +406,9 @@ pub fn get_hover(
         }
     }
 
+    let stdlib = transaction.get_stdlib(handle);
+    let symbol_def_paths = collect_symbol_def_paths_with_stdlib(&type_, stdlib.as_ref());
+
     Some(
         HoverValue {
             kind,
@@ -413,6 +418,7 @@ pub fn get_hover(
             parameter_doc,
             display: type_display,
             show_go_to_links,
+            symbol_def_paths,
         }
         .format(),
     )

pyrefly/lib/lsp/wasm/inlay_hints.rs

@@ -119,6 +119,7 @@ impl<'a> Transaction<'a> {
                 }
         };
         let bindings = self.get_bindings(handle)?;
+        let stdlib = self.get_stdlib(handle);
         let mut res = Vec::new();
         for idx in bindings.keys::<Key>() {
             match bindings.idx_to_key(idx) {
@@ -140,7 +141,7 @@ impl<'a> Transaction<'a> {
                                         ty = return_ty;
                                     }
                                     // Use get_types_with_locations to get type parts with location info
-                                    let type_parts = ty.get_types_with_locations();
+                                    let type_parts = ty.get_types_with_locations(stdlib.as_ref());
                                     let label_parts = once((" -> ".to_owned(), None))
                                         .chain(
                                             type_parts
@@ -182,7 +183,7 @@ impl<'a> Transaction<'a> {
                         && is_interesting(e, &ty, class_name)
                     {
                         // Use get_types_with_locations to get type parts with location info
-                        let type_parts = ty.get_types_with_locations();
+                        let type_parts = ty.get_types_with_locations(stdlib.as_ref());
                         let label_parts = once((": ".to_owned(), None))
                             .chain(
                                 type_parts

pyrefly/lib/state/lsp.rs

@@ -69,7 +69,7 @@ use crate::binding::binding::Key;
 use crate::config::error_kind::ErrorKind;
 use crate::export::exports::Export;
 use crate::export::exports::ExportLocation;
-use crate::lsp::module_helpers::collect_symbol_def_paths;
+use crate::lsp::module_helpers::collect_symbol_def_paths_with_stdlib;
 use crate::state::ide::IntermediateDefinition;
 use crate::state::ide::import_regular_import_edit;
 use crate::state::ide::insert_import_edit;
@@ -1567,7 +1567,8 @@ impl<'a> Transaction<'a> {
         let type_ = self.get_type_at(handle, position);
 
         if let Some(t) = type_ {
-            let symbol_def_paths = collect_symbol_def_paths(&t);
+            let stdlib = self.get_stdlib(handle);
+            let symbol_def_paths = collect_symbol_def_paths_with_stdlib(&t, stdlib.as_ref());
 
             if !symbol_def_paths.is_empty() {
                 return symbol_def_paths.map(|(qname, _)| {