update based on comment

Author: asukaminato0721

crates/pyrefly_types/src/display.rs

@@ -780,9 +780,22 @@ impl Display for Type {
 
 impl Type {
     pub fn as_hover_string(&self) -> String {
+        self.as_hover_string_with_fallback_name(None)
+    }
+
+    pub fn as_hover_string_with_fallback_name(&self, fallback_name: Option<&str>) -> String {
         let mut c = TypeDisplayContext::new(&[self]);
         c.set_display_mode_to_hover();
-        c.display(self).to_string()
+        let rendered = c.display(self).to_string();
+        if let Some(name) = fallback_name
+            && self.is_function_type()
+        {
+            let trimmed = rendered.trim_start();
+            if trimmed.starts_with('(') {
+                return format!("def {}{}: ...", name, trimmed);
+            }
+        }
+        rendered
     }
 
     pub fn get_types_with_locations(&self) -> Vec<(String, Option<TextRangeWithModule>)> {
@@ -809,54 +822,6 @@ impl<'a> ClassDisplayContext<'a> {
     }
 }
 
-/// Small helper describing the formatted code that should appear inside a hover block.
-///
-/// `body` is the snippet that will live inside the fenced code block, while
-/// `default_kind_label` lets callers know which `(kind)` prefix to use when the
-/// hover metadata can't provide one (e.g. when resolving a stub-only function).
-#[derive(Debug, Clone)]
-pub struct HoverCodeSnippet {
-    pub body: String,
-    pub default_kind_label: Option<&'static str>,
-}
-
-/// Format the string returned by `Type::as_hover_string()` so that callable types
-/// always resemble real Python function definitions. When `name` is provided we
-/// will synthesize a `def name(...): ...` signature if the rendered type only
-/// contains the parenthesized parameter list. This keeps IDE syntax highlighting
-/// working even when we fall back to hover strings built from metadata alone.
-///
-/// `display` is typically `Type::as_hover_string()`, but callers may pass their
-/// own rendering (for example, after expanding TypedDict kwargs).
-pub fn format_hover_code_snippet(
-    type_: &Type,
-    name: Option<&str>,
-    display: String,
-) -> HoverCodeSnippet {
-    if type_.is_function_type() {
-        let body = match name {
-            Some(name) => {
-                let trimmed = display.trim_start();
-                if trimmed.starts_with('(') {
-                    format!("def {}{}: ...", name, trimmed)
-                } else {
-                    display
-                }
-            }
-            None => display,
-        };
-        HoverCodeSnippet {
-            body,
-            default_kind_label: Some("(function) "),
-        }
-    } else {
-        HoverCodeSnippet {
-            body: display,
-            default_kind_label: None,
-        }
-    }
-}
-
 #[cfg(test)]
 pub mod tests {
     use std::path::PathBuf;

pyrefly/lib/lsp/wasm/hover.rs

@@ -26,7 +26,6 @@ use pyrefly_types::callable::Param;
 use pyrefly_types::callable::ParamList;
 use pyrefly_types::callable::Params;
 use pyrefly_types::callable::Required;
-use pyrefly_types::display::format_hover_code_snippet;
 use pyrefly_types::types::BoundMethodType;
 use pyrefly_types::types::Forallable;
 use pyrefly_types::types::Type;
@@ -219,39 +218,29 @@ impl HoverValue {
                     let cleaned = doc.trim().replace('\n', "  \n");
                     format!("{prefix}**Parameter `{}`**\n{}", name, cleaned)
                 });
-        let mut kind_formatted = self.kind.map_or("".to_owned(), |kind| {
-            format!("{} ", kind.display_for_hover())
-        });
-        let name_formatted = self
-            .name
-            .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())
-        } else {
-            String::new()
-        };
-        let type_display = self
-            .display
-            .clone()
-            .unwrap_or_else(|| self.type_.as_hover_string());
-        // Ensure callable hover bodies always contain a proper `def name(...)` so IDE syntax
-        // highlighting stays consistent, even when metadata is missing and we fall back to
-        // inferred identifiers.
-        let snippet = format_hover_code_snippet(&self.type_, self.name.as_deref(), type_display);
         let kind_formatted = self.kind.map_or_else(
             || {
-                snippet
-                    .default_kind_label
-                    .map(str::to_owned)
-                    .unwrap_or_default()
+                if self.type_.is_function_type() {
+                    "(function) ".to_owned()
+                } else {
+                    String::new()
+                }
             },
             |kind| format!("{} ", kind.display_for_hover()),
         );
         let name_formatted = self
             .name
             .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())
+        } else {
+            String::new()
+        };
+        let type_display = self.display.clone().unwrap_or_else(|| {
+            self.type_
+                .as_hover_string_with_fallback_name(self.name.as_deref())
+        });
 
         Hover {
             contents: HoverContents::Markup(MarkupContent {
@@ -450,15 +439,6 @@ pub fn get_hover(
     // Otherwise, fall through to the existing type hover logic
     let type_ = transaction.get_type_at(handle, position)?;
     let fallback_name_from_type = fallback_hover_name_from_type(&type_);
-    let type_display = transaction.ad_hoc_solve(handle, {
-        let mut cloned = type_.clone();
-        move |solver| {
-            // If the type is a callable, rewrite the signature to expand TypedDict-based
-            // `**kwargs` entries, ensuring hover text shows the actual keyword names users can pass.
-            cloned.visit_toplevel_callable_mut(|c| expand_callable_kwargs_for_hover(&solver, c));
-            cloned.as_hover_string()
-        }
-    });
     let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
         metadata,
         definition_range: definition_location,
@@ -499,6 +479,15 @@ pub fn get_hover(
 
     let name = name.or_else(|| identifier_text_at(transaction, handle, position));
 
+    let name_for_display = name.clone();
+    let type_display = transaction.ad_hoc_solve(handle, {
+        let mut cloned = type_.clone();
+        move |solver| {
+            cloned.visit_toplevel_callable_mut(|c| expand_callable_kwargs_for_hover(&solver, c));
+            cloned.as_hover_string_with_fallback_name(name_for_display.as_deref())
+        }
+    });
+
     let docstring = if let (Some(docstring), Some(module)) = (docstring_range, module) {
         Some(Docstring(docstring, module))
     } else {

pyrefly/lib/test/lsp/lsp_interaction/hover.rs

@@ -95,28 +95,24 @@ fn hover_shows_third_party_function_name() {
     let root = get_test_files_root();
     let mut interaction = LspInteraction::new();
     interaction.set_root(root.path().join("rename_third_party"));
-    interaction.initialize(InitializeSettings {
-        configuration: Some(None),
-        ..Default::default()
-    });
+    interaction
+        .initialize(InitializeSettings {
+            configuration: Some(None),
+            ..Default::default()
+        })
+        .unwrap();
 
     interaction.client.did_open("user_code.py");
     // Column/line values follow LSP's zero-based positions
-    interaction.client.hover("user_code.py", 14, 25);
-    interaction.client.expect_response_with(
-        |response| {
-            response
-                .result
-                .as_ref()
-                .and_then(|value| value.get("contents"))
-                .and_then(|contents| contents.get("value"))
-                .and_then(|value| value.as_str())
-                .is_some_and(|value| value.contains("external_function"))
-        },
-        "hover should show the function name for third-party modules",
-    );
-
-    interaction.shutdown();
+    interaction
+        .client
+        .hover("user_code.py", 14, 25)
+        .expect_hover_response_with_markup(|value| {
+            value.is_some_and(|text| text.contains("external_function"))
+        })
+        .unwrap();
+
+    interaction.shutdown().unwrap();
 }
 
 #[test]