fix

asukaminato0721 · asukaminato0721 · commit 906643313ff3 · 2025-11-27T21:04:46.000+09:00
diff --git a/crates/pyrefly_types/src/display.rs b/crates/pyrefly_types/src/display.rs
@@ -809,41 +809,48 @@ 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 heading: Option<String>,
     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() {
-        if let Some(name) = name {
-            let trimmed = display.trim_start();
-            let body = if trimmed.starts_with('(') {
-                format!("def {}{}: ...", name, trimmed)
-            } else {
-                display
-            };
-            HoverCodeSnippet {
-                heading: Some(name.to_owned()),
-                body,
-                default_kind_label: Some("(function) "),
-            }
-        } else {
-            HoverCodeSnippet {
-                heading: None,
-                body: display,
-                default_kind_label: Some("(function) "),
+        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 {
-            heading: None,
             body: display,
             default_kind_label: None,
         }
diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs
@@ -18,6 +18,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;
+#[cfg(test)]
 use pyrefly_python::module_name::ModuleName;
 use pyrefly_python::symbol_kind::SymbolKind;
 use pyrefly_types::callable::Callable;
@@ -234,6 +235,9 @@ impl HoverValue {
             .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(
             || {
@@ -248,11 +252,6 @@ impl HoverValue {
             .name
             .as_ref()
             .map_or("".to_owned(), |s| format!("{s}: "));
-        let heading = if let Some(callable_heading) = snippet.heading.as_ref() {
-            format!("{}{}\n", kind_formatted, callable_heading)
-        } else {
-            format!("{}{}", kind_formatted, name_formatted)
-        };
 
         Hover {
             contents: HoverContents::Markup(MarkupContent {
@@ -316,30 +315,72 @@ fn expand_callable_kwargs_for_hover<'a>(
     }
 }
 
-fn fallback_hover_name_from_type(type_: &Type, current_module: ModuleName) -> Option<String> {
+/// If we can't determine a symbol name via go-to-definition, fall back to what the
+/// type metadata knows about the callable. This primarily handles third-party stubs
+/// where we only have typeshed information.
+fn fallback_hover_name_from_type(type_: &Type) -> Option<String> {
     match type_ {
-        Type::Function(function) => Some(function.metadata.kind.format(current_module)),
+        Type::Function(function) => Some(
+            function
+                .metadata
+                .kind
+                .function_name()
+                .into_owned()
+                .to_string(),
+        ),
         Type::BoundMethod(bound_method) => match &bound_method.func {
-            BoundMethodType::Function(function) => {
-                Some(function.metadata.kind.format(current_module))
-            }
-            BoundMethodType::Forall(forall) => {
-                Some(forall.body.metadata.kind.format(current_module))
-            }
-            BoundMethodType::Overload(overload) => {
-                Some(overload.metadata.kind.format(current_module))
-            }
+            BoundMethodType::Function(function) => Some(
+                function
+                    .metadata
+                    .kind
+                    .function_name()
+                    .into_owned()
+                    .to_string(),
+            ),
+            BoundMethodType::Forall(forall) => Some(
+                forall
+                    .body
+                    .metadata
+                    .kind
+                    .function_name()
+                    .into_owned()
+                    .to_string(),
+            ),
+            BoundMethodType::Overload(overload) => Some(
+                overload
+                    .metadata
+                    .kind
+                    .function_name()
+                    .into_owned()
+                    .to_string(),
+            ),
         },
-        Type::Overload(overload) => Some(overload.metadata.kind.format(current_module)),
+        Type::Overload(overload) => Some(
+            overload
+                .metadata
+                .kind
+                .function_name()
+                .into_owned()
+                .to_string(),
+        ),
         Type::Forall(forall) => match &forall.body {
-            Forallable::Function(function) => Some(function.metadata.kind.format(current_module)),
+            Forallable::Function(function) => Some(
+                function
+                    .metadata
+                    .kind
+                    .function_name()
+                    .into_owned()
+                    .to_string(),
+            ),
             Forallable::Callable(_) | Forallable::TypeAlias(_) => None,
         },
-        Type::Type(inner) => fallback_hover_name_from_type(inner, current_module),
+        Type::Type(inner) => fallback_hover_name_from_type(inner),
         _ => None,
     }
 }
 
+/// Extract the identifier under the cursor directly from the file contents so we can
+/// label hover results even when go-to-definition fails.
 fn identifier_text_at(
     transaction: &Transaction<'_>,
     handle: &Handle,
@@ -349,10 +390,7 @@ fn identifier_text_at(
     let contents = module.contents();
     let bytes = contents.as_bytes();
     let len = bytes.len();
-    let mut pos = position.to_usize();
-    if pos > len {
-        pos = len;
-    }
+    let pos = position.to_usize().min(len);
     let is_ident_char = |b: u8| b == b'_' || b.is_ascii_alphanumeric();
     let mut start = pos;
     while start > 0 && is_ident_char(bytes[start - 1]) {
@@ -363,10 +401,10 @@ fn identifier_text_at(
         end += 1;
     }
     if start == end {
-        None
-    } else {
-        Some(contents[start..end].to_string())
+        return None;
     }
+    let range = TextRange::new(TextSize::new(start as u32), TextSize::new(end as u32));
+    Some(module.code_at(range).to_owned())
 }
 
 pub fn get_hover(
@@ -411,8 +449,7 @@ pub fn get_hover(
 
     // Otherwise, fall through to the existing type hover logic
     let type_ = transaction.get_type_at(handle, position)?;
-    let current_module = handle.module();
-    let fallback_name_from_type = fallback_hover_name_from_type(&type_, current_module);
+    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| {
@@ -422,7 +459,7 @@ pub fn get_hover(
             cloned.as_hover_string()
         }
     });
-    let (kind, mut name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
+    let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
         metadata,
         definition_range: definition_location,
         module,
@@ -460,9 +497,7 @@ pub fn get_hover(
         (None, fallback_name_from_type, None, None)
     };
 
-    if name.is_none() {
-        name = identifier_text_at(transaction, handle, position);
-    }
+    let name = name.or_else(|| identifier_text_at(transaction, handle, position));
 
     let docstring = if let (Some(docstring), Some(module)) = (docstring_range, module) {
         Some(Docstring(docstring, module))
@@ -620,14 +655,14 @@ mod tests {
     #[test]
     fn fallback_uses_function_metadata() {
         let ty = make_function_type("numpy", "arange");
-        let fallback = fallback_hover_name_from_type(&ty, ModuleName::from_str("user_code"));
-        assert_eq!(fallback.as_deref(), Some("numpy.arange"));
+        let fallback = fallback_hover_name_from_type(&ty);
+        assert_eq!(fallback.as_deref(), Some("arange"));
     }
 
     #[test]
     fn fallback_recurses_through_type_wrapper() {
         let ty = Type::Type(Box::new(make_function_type("pkg.subpkg", "run")));
-        let fallback = fallback_hover_name_from_type(&ty, ModuleName::from_str("other"));
-        assert_eq!(fallback.as_deref(), Some("pkg.subpkg.run"));
+        let fallback = fallback_hover_name_from_type(&ty);
+        assert_eq!(fallback.as_deref(), Some("run"));
     }
 }
diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs
@@ -39,7 +39,7 @@ xyz = [foo.meth]
 #^
 "#;
     let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report);
-    assert!(report.contains("(method) meth\ndef meth(self: Foo) -> None: ..."));
+    assert!(report.contains("(method) meth: def meth(self: Foo) -> None: ..."));
     assert!(report.contains("(variable) xyz: list[(self: Foo) -> None]"));
     assert!(
         report.contains("Go to [list]"),
@@ -75,8 +75,7 @@ from lib import foo_renamed
 2 | from lib import foo_renamed
                          ^
 ```python
-(function) foo
-def foo() -> None: ...
+(function) foo: def foo() -> None: ...
 ```
 
 
@@ -112,8 +111,7 @@ takes(foo=1, bar="x", baz=None)
 12 | takes(foo=1, bar="x", baz=None)
       ^
 ```python
-(function) takes
-def takes(
+(function) takes: def takes(
     *,
     foo: int,
     bar: str,
@@ -361,8 +359,7 @@ lhs @ rhs
 13 | lhs @ rhs
          ^
 ```python
-(method) __matmul__
-def __matmul__(
+(method) __matmul__: def __matmul__(
     self: Matrix,
     other: Matrix
 ) -> Matrix: ...
diff --git a/pyrefly/lib/test/lsp/lsp_interaction/hover.rs b/pyrefly/lib/test/lsp/lsp_interaction/hover.rs
@@ -100,9 +100,9 @@ fn hover_shows_third_party_function_name() {
         ..Default::default()
     });
 
-    interaction.server.did_open("user_code.py");
+    interaction.client.did_open("user_code.py");
     // Column/line values follow LSP's zero-based positions
-    interaction.server.hover("user_code.py", 14, 25);
+    interaction.client.hover("user_code.py", 14, 25);
     interaction.client.expect_response_with(
         |response| {
             response
