diff --git a/crates/pyrefly_types/src/display.rs b/crates/pyrefly_types/src/display.rs index 8b66c13a4..13e3fd531 100644 --- a/crates/pyrefly_types/src/display.rs +++ b/crates/pyrefly_types/src/display.rs @@ -776,9 +776,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)> { diff --git a/pyrefly/lib/lsp/non_wasm/server.rs b/pyrefly/lib/lsp/non_wasm/server.rs index 5df6916db..4c3ddf212 100644 --- a/pyrefly/lib/lsp/non_wasm/server.rs +++ b/pyrefly/lib/lsp/non_wasm/server.rs @@ -2196,6 +2196,7 @@ impl Server { definition_range, module, docstring_range: _, + .. } = definition; // find_global_implementations_from_definition returns Vec // but we need to return Vec<(ModuleInfo, Vec)> to match the helper's @@ -2443,6 +2444,7 @@ impl Server { definition_range, module, docstring_range: _, + .. } = definition; transaction.find_global_references_from_definition( handle.sys_info(), diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs index aa8c680c0..bbcfa4cf5 100644 --- a/pyrefly/lib/lsp/wasm/hover.rs +++ b/pyrefly/lib/lsp/wasm/hover.rs @@ -18,12 +18,16 @@ 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; use pyrefly_types::callable::Param; use pyrefly_types::callable::ParamList; use pyrefly_types::callable::Params; use pyrefly_types::callable::Required; +use pyrefly_types::types::BoundMethodType; +use pyrefly_types::types::Forallable; use pyrefly_types::types::Type; use pyrefly_util::lined_buffer::LineNumber; use ruff_python_ast::Stmt; @@ -214,9 +218,16 @@ impl HoverValue { let cleaned = doc.trim().replace('\n', " \n"); format!("{prefix}**Parameter `{}`**\n{}", name, cleaned) }); - let kind_formatted = self.kind.map_or("".to_owned(), |kind| { - format!("{} ", kind.display_for_hover()) - }); + let kind_formatted = self.kind.map_or_else( + || { + 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() @@ -226,10 +237,10 @@ impl HoverValue { } else { String::new() }; - let type_display = self - .display - .clone() - .unwrap_or_else(|| self.type_.as_hover_string()); + 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 { @@ -292,6 +303,99 @@ fn expand_callable_kwargs_for_hover<'a>( } } } + +/// 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 { + match type_ { + 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 + .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 + .function_name() + .into_owned() + .to_string(), + ), + Type::Forall(forall) => match &forall.body { + 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), + _ => 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, + position: TextSize, +) -> Option { + let module = transaction.get_module_info(handle)?; + let contents = module.contents(); + let bytes = contents.as_bytes(); + let len = bytes.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]) { + start -= 1; + } + let mut end = pos; + while end < len && is_ident_char(bytes[end]) { + end += 1; + } + if start == end { + 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( transaction: &Transaction<'_>, handle: &Handle, @@ -334,20 +438,13 @@ pub fn get_hover( // Otherwise, fall through to the existing type hover logic let type_ = transaction.get_type_at(handle, position)?; - 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 fallback_name_from_type = fallback_hover_name_from_type(&type_); let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring { metadata, definition_range: definition_location, module, docstring_range, + display_name, }) = transaction .find_definition( handle, @@ -365,16 +462,32 @@ pub fn get_hover( if matches!(kind, Some(SymbolKind::Attribute)) && type_.is_function_type() { kind = Some(SymbolKind::Method); } - ( - kind, - Some(module.code_at(definition_location).to_owned()), - docstring_range, - Some(module), - ) + let name = { + let snippet = module.code_at(definition_location); + if snippet.chars().any(|c| !c.is_whitespace()) { + Some(snippet.to_owned()) + } else if let Some(name) = display_name.clone() { + Some(name) + } else { + fallback_name_from_type.clone() + } + }; + (kind, name, docstring_range, Some(module)) } else { - (None, None, None, None) + (None, fallback_name_from_type, None, None) }; + 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 { @@ -490,3 +603,55 @@ fn parameter_documentation_for_callee( let docs = parse_parameter_documentation(module.code_at(range)); if docs.is_empty() { None } else { Some(docs) } } + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + use std::sync::Arc; + + use pyrefly_python::module::Module; + use pyrefly_python::module_path::ModulePath; + use pyrefly_types::callable::Callable; + use pyrefly_types::callable::FuncFlags; + use pyrefly_types::callable::FuncId; + use pyrefly_types::callable::FuncMetadata; + use pyrefly_types::callable::Function; + use pyrefly_types::callable::FunctionKind; + use ruff_python_ast::name::Name; + + use super::*; + + fn make_function_type(module_name: &str, func_name: &str) -> Type { + let module = Module::new( + ModuleName::from_str(module_name), + ModulePath::filesystem(PathBuf::from(format!("{module_name}.pyi"))), + Arc::new(String::new()), + ); + let metadata = FuncMetadata { + kind: FunctionKind::Def(Box::new(FuncId { + module, + cls: None, + name: Name::new(func_name), + })), + flags: FuncFlags::default(), + }; + Type::Function(Box::new(Function { + signature: Callable::ellipsis(Type::None), + metadata, + })) + } + + #[test] + fn fallback_uses_function_metadata() { + let ty = make_function_type("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); + assert_eq!(fallback.as_deref(), Some("run")); + } +} diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index f542de52c..0369df0e9 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -472,6 +472,7 @@ pub struct FindDefinitionItemWithDocstring { pub definition_range: TextRange, pub module: Module, pub docstring_range: Option, + pub display_name: Option, } #[derive(Debug)] @@ -1110,6 +1111,7 @@ impl<'a> Transaction<'a> { definition_range: location, module: module_info, docstring_range, + display_name: Some(name.id.to_string()), }) } @@ -1134,6 +1136,7 @@ impl<'a> Transaction<'a> { definition_range: location, module: self.get_module_info(&handle)?, docstring_range, + display_name: Some(name.id.to_string()), }) } @@ -1158,6 +1161,7 @@ impl<'a> Transaction<'a> { definition_range: definition.range, module: definition.module, docstring_range, + display_name: Some(name.to_string()), }) } else { None @@ -1293,6 +1297,7 @@ impl<'a> Transaction<'a> { definition_range: TextRange::default(), module: module_info, docstring_range: self.get_module_docstring_range(&handle), + display_name: Some(module_name.to_string()), }) } @@ -1430,6 +1435,7 @@ impl<'a> Transaction<'a> { module, definition_range: identifier.range, docstring_range, + display_name: Some(identifier.id.to_string()), }] }), Some(IdentifierWithContext { @@ -1443,6 +1449,7 @@ impl<'a> Transaction<'a> { definition_range: item.definition_range, module: item.module, docstring_range, + display_name: Some(identifier.id.to_string()), }] }), Some(IdentifierWithContext { @@ -1456,6 +1463,7 @@ impl<'a> Transaction<'a> { definition_range: item.definition_range, module: item.module, docstring_range, + display_name: Some(identifier.id.to_string()), }] }), Some(IdentifierWithContext { @@ -1469,6 +1477,7 @@ impl<'a> Transaction<'a> { definition_range: item.definition_range, module: item.module, docstring_range: None, + display_name: Some(identifier.id.to_string()), }] }), Some(IdentifierWithContext { @@ -1482,6 +1491,7 @@ impl<'a> Transaction<'a> { definition_range: item.definition_range, module: item.module, docstring_range: None, + display_name: Some(identifier.id.to_string()), }] }), Some(IdentifierWithContext { @@ -1495,6 +1505,7 @@ impl<'a> Transaction<'a> { definition_range: item.definition_range, module: item.module, docstring_range: None, + display_name: Some(identifier.id.to_string()), }] }), Some(IdentifierWithContext { @@ -1507,6 +1518,7 @@ impl<'a> Transaction<'a> { definition_range: item.definition_range, module: item.module.clone(), docstring_range: None, + display_name: Some(identifier.id.to_string()), }), Some(IdentifierWithContext { identifier, @@ -1726,6 +1738,7 @@ impl<'a> Transaction<'a> { definition_range, module, docstring_range: _, + .. }| { self.local_references_from_definition(handle, metadata, definition_range, &module) }, diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index 7192b0ab8..efc6b10d8 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -359,10 +359,10 @@ lhs @ rhs 13 | lhs @ rhs ^ ```python -(method) __matmul__: ( +(method) __matmul__: def __matmul__( self: Matrix, other: Matrix -) -> Matrix +) -> Matrix: ... ``` "# .trim(), diff --git a/pyrefly/lib/test/lsp/lsp_interaction/hover.rs b/pyrefly/lib/test/lsp/lsp_interaction/hover.rs index 45349c94e..0bbc5b717 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/hover.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/hover.rs @@ -90,6 +90,31 @@ fn hover_attribute_prefers_py_docstring_over_pyi() { interaction.shutdown().unwrap(); } +#[test] +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() + }) + .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) + .expect_hover_response_with_markup(|value| { + value.is_some_and(|text| text.contains("external_function")) + }) + .unwrap(); + + interaction.shutdown().unwrap(); +} + #[test] fn test_hover_import() { let root = get_test_files_root();