From d9e2299d13ad3d3b230612d0dc8d71e32f897236 Mon Sep 17 00:00:00 2001 From: spyrosgavriil Date: Wed, 14 Jan 2026 18:16:31 +0200 Subject: [PATCH 1/4] Show instance type in constructor signatures Constructor calls now display the instance type instead of None in both signature help and hover. Direct __init__ calls still show -> None. Fixes #1805 --- pyrefly/lib/lsp/wasm/hover.rs | 78 ++++++++++++++++++++++++- pyrefly/lib/lsp/wasm/signature_help.rs | 72 ++++++++++++++++++++++- pyrefly/lib/test/lsp/signature_help.rs | 79 ++++++++++++++++++++++++++ 3 files changed, 226 insertions(+), 3 deletions(-) diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs index 1937030747..409cc3e389 100644 --- a/pyrefly/lib/lsp/wasm/hover.rs +++ b/pyrefly/lib/lsp/wasm/hover.rs @@ -30,8 +30,10 @@ use pyrefly_types::callable::Required; use pyrefly_types::display::LspDisplayMode; use pyrefly_types::types::Type; use pyrefly_util::lined_buffer::LineNumber; +use pyrefly_util::visit::Visit; use ruff_python_ast::Stmt; use ruff_python_ast::name::Name; +use ruff_text_size::Ranged; use ruff_text_size::TextRange; use ruff_text_size::TextSize; @@ -460,7 +462,81 @@ pub fn get_hover( } // Otherwise, fall through to the existing type hover logic - let type_ = transaction.get_type_at(handle, position)?; + let mut type_ = transaction.get_type_at(handle, position)?; + + // Helper function to check if we're hovering over a callee and get its range + let find_callee_range_at_position = || -> Option { + use ruff_python_ast::Expr; + let mod_module = transaction.get_ast(handle)?; + let mut result = None; + mod_module.visit(&mut |expr: &Expr| { + if let Expr::Call(call) = expr { + // Check if position is within the callee (func) range + if call.func.range().contains(position) { + result = Some(call.func.range()); + } + } + }); + result + }; + + // Check both: hovering in arguments area OR hovering over the callee itself + let callee_range_opt = transaction + .get_callables_from_call(handle, position) + .map(|(_, _, _, range)| range) + .or_else(find_callee_range_at_position); + + if let Some(callee_range) = callee_range_opt { + // Determine if this is a constructor call vs direct __init__ call or regular method call + let is_constructor_call = transaction + .get_module_info(handle) + .map(|module| { + let contents = module.contents(); + let start = callee_range.start().to_usize(); + let end = callee_range.end().to_usize(); + contents + .get(start..end) + .map(|callee_text| { + if callee_text.ends_with(".__init__") { + return false; + } + if let Some(last_dot_pos) = callee_text.rfind('.') { + let after_dot = &callee_text[last_dot_pos + 1..]; + if !after_dot.is_empty() + && after_dot.chars().all(|c| c.is_alphanumeric() || c == '_') + && (after_dot.chars().next().unwrap().is_alphabetic() + || after_dot.starts_with('_')) + { + return false; + } + } + true + }) + .unwrap_or(false) + }) + .unwrap_or(false); + + // Override the return type if this is a constructor call + if is_constructor_call { + if let Some(mut callable) = type_.clone().to_callable() + && callable.ret.is_none() + { + // Check if first parameter is self or cls + if let Params::List(ref params_list) = callable.params { + if let Some( + Param::Pos(name, self_type, _) | Param::PosOnly(Some(name), self_type, _), + ) = params_list.items().first() + { + if name.as_str() == "self" || name.as_str() == "cls" { + callable.ret = self_type.clone(); + type_ = Type::Callable(Box::new(callable)); + } + } + } + } + } + } + let fallback_name_from_type = fallback_hover_name_from_type(&type_); let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring { metadata, diff --git a/pyrefly/lib/lsp/wasm/signature_help.rs b/pyrefly/lib/lsp/wasm/signature_help.rs index 594c6e9f59..32a1222b9a 100644 --- a/pyrefly/lib/lsp/wasm/signature_help.rs +++ b/pyrefly/lib/lsp/wasm/signature_help.rs @@ -280,11 +280,46 @@ impl Transaction<'_> { active_argument: &ActiveArgument, parameter_docs: Option<&HashMap>, function_docstring: Option<&Docstring>, + is_constructor_call: bool, ) -> SignatureInformation { let type_ = type_.deterministic_printing(); - let label = type_.as_lsp_string(LspDisplayMode::SignatureHelp); + + // Display the return type as the class instance type instead of None + let display_type = if is_constructor_call + && let Some(mut callable) = type_.clone().to_callable() + && callable.ret.is_none() + { + // Check if first parameter is self or cls + let should_override = if let Params::List(ref params_list) = callable.params { + if let Some( + Param::Pos(name, self_type, _) | Param::PosOnly(Some(name), self_type, _), + ) = params_list.items().first() + { + if name.as_str() == "self" || name.as_str() == "cls" { + callable.ret = self_type.clone(); + true + } else { + false + } + } else { + false + } + } else { + false + }; + + if should_override { + Type::Callable(Box::new(callable)) + } else { + type_ + } + } else { + type_ + }; + + let label = display_type.as_lsp_string(LspDisplayMode::SignatureHelp); let (parameters, active_parameter) = if let Some(params) = - Self::normalize_singleton_function_type_into_params(type_) + Self::normalize_singleton_function_type_into_params(display_type) { // Create a type display context for consistent parameter formatting let param_types: Vec<&Type> = params.iter().map(|p| p.as_type()).collect(); @@ -335,6 +370,38 @@ impl Transaction<'_> { |(callables, chosen_overload_index, active_argument, callee_range)| { let parameter_docs = self.parameter_documentation_for_callee(handle, callee_range); let function_docstring = self.function_docstring_for_callee(handle, callee_range); + + // Determine if this is a constructor call vs direct __init__ call or regular method call + let is_constructor_call = self + .get_module_info(handle) + .map(|module| { + let contents = module.contents(); + let start = callee_range.start().to_usize(); + let end = callee_range.end().to_usize(); + contents + .get(start..end) + .map(|callee_text| { + if callee_text.ends_with(".__init__") { + return false; + } + if let Some(last_dot_pos) = callee_text.rfind('.') { + let after_dot = &callee_text[last_dot_pos + 1..]; + if !after_dot.is_empty() + && after_dot + .chars() + .all(|c| c.is_alphanumeric() || c == '_') + && (after_dot.chars().next().unwrap().is_alphabetic() + || after_dot.starts_with('_')) + { + return false; + } + } + true + }) + .unwrap_or(false) + }) + .unwrap_or(false); + let signatures = callables .into_iter() .map(|t| { @@ -343,6 +410,7 @@ impl Transaction<'_> { &active_argument, parameter_docs.as_ref(), function_docstring.as_ref(), + is_constructor_call, ) }) .collect_vec(); diff --git a/pyrefly/lib/test/lsp/signature_help.rs b/pyrefly/lib/test/lsp/signature_help.rs index 1699d09834..bd53110088 100644 --- a/pyrefly/lib/test/lsp/signature_help.rs +++ b/pyrefly/lib/test/lsp/signature_help.rs @@ -903,3 +903,82 @@ Signature Help Result: active=0 report.trim(), ); } + +#[test] +fn constructor_signature_shows_instance_type() { + let code = r#" +class Person: + def __init__(self, name: str, age: int) -> None: ... + +Person() +# ^ +Person("Alice", ) +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert_eq!( + r#" +# main.py +5 | Person() + ^ +Signature Help Result: active=0 +- (self: Person, name: str, age: int) -> Person, parameters=[name: str, age: int], active parameter = 0 + +7 | Person("Alice", ) + ^ +Signature Help Result: active=0 +- (self: Person, name: str, age: int) -> Person, parameters=[name: str, age: int], active parameter = 1 +"# + .trim(), + report.trim(), + ); +} + +#[test] +fn direct_init_call_shows_none() { + let code = r#" +class Person: + def __init__(self, name: str) -> None: ... + +p = Person.__new__(Person) +Person.__init__(p, ) +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + // Direct __init__ call should still show -> None + assert!( + report.contains("-> None"), + "Expected direct __init__ call to show -> None, got: {report}" + ); + assert!( + report.contains("parameters=[name: str]"), + "Expected parameters, got: {report}" + ); +} + +#[test] +fn generic_constructor_signature() { + let code = r#" +from typing import Generic, TypeVar + +T = TypeVar("T") + +class Box(Generic[T]): + def __init__(self, value: T) -> None: ... + +Box[str]() +# ^ +Box[int](42) +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + // Generic constructors should show the specialized instance type + assert!( + report.contains("-> Box[str]") || report.contains("Box[str]"), + "Expected generic constructor to show Box[str], got: {report}" + ); + assert!( + report.contains("-> Box[int]") || report.contains("Box[int]"), + "Expected generic constructor to show Box[int], got: {report}" + ); +} From d2fa4e396bfa0fced9ae909caf44959d77c60405 Mon Sep 17 00:00:00 2001 From: spyrosgavriil Date: Sat, 17 Jan 2026 11:08:02 +0200 Subject: [PATCH 2/4] Add tests for constructor signature display Fixes #1805 --- pyrefly/lib/test/lsp/hover.rs | 117 +++++++++++++++++++++++++ pyrefly/lib/test/lsp/signature_help.rs | 22 +++++ 2 files changed, 139 insertions(+) diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index 7049f4fa64..7558aa82da 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -14,6 +14,7 @@ use ruff_text_size::TextSize; use crate::lsp::wasm::hover::get_hover; use crate::state::state::State; use crate::test::util::get_batched_lsp_operations_report; +use crate::test::util::get_batched_lsp_operations_report_allow_error; fn get_test_report(state: &State, handle: &Handle, position: TextSize) -> String { match get_hover(&state.transaction(), handle, position, true) { @@ -903,3 +904,119 @@ from mymod.submod.deep import Bar "Expected hover to show full module name 'mymod.submod.deep', got: {report}" ); } + +#[test] +fn hover_on_constructor_shows_instance_type() { + let code = r#" +class Person: + def __init__(self, name: str, age: int) -> None: ... + +Person() +#^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert!( + report.contains("-> Person"), + "Expected constructor hover to show -> Person, got: {report}" + ); + assert!( + !report.contains("-> None"), + "Constructor hover should not show -> None, got: {report}" + ); +} + +#[test] +fn hover_on_constructor_with_arguments() { + let code = r#" +class Person: + def __init__(self, name: str, age: int) -> None: ... + +Person("Alice", 25) +#^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert!( + report.contains("-> Person"), + "Expected constructor hover to show -> Person, got: {report}" + ); +} + +#[test] +fn hover_on_direct_init_call_shows_none() { + let code = r#" +class Person: + def __init__(self, name: str) -> None: ... + +p = Person.__new__(Person) +Person.__init__(p, "Alice") +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert!( + report.contains("-> None"), + "Expected direct __init__ call to show -> None, got: {report}" + ); + assert!( + !report.contains("-> Person") || report.contains("__init__"), + "Direct __init__ call should show -> None, got: {report}" + ); +} + +#[test] +fn hover_on_method_call_unchanged() { + let code = r#" +class Foo: + def method(self) -> str: ... + +foo = Foo() +foo.method() +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert!( + report.contains("-> str"), + "Expected method hover to show -> str, got: {report}" + ); +} + +#[test] +fn hover_on_argument_shows_argument_type() { + let code = r#" +class Person: + def __init__(self, name: str) -> None: ... + +Person("Alice") +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + // Hovering over a string literal shows its literal type + assert!( + report.contains("Literal['Alice']") || report.contains("str"), + "Expected argument hover to show literal type or str, got: {report}" + ); + // The argument hover should not show the constructor signature + assert!( + !report.contains("__init__") || !report.contains("name: str"), + "Argument hover should show argument type, not constructor, got: {report}" + ); +} + +#[test] +fn hover_on_generic_constructor() { + let code = r#" +from typing import Generic, TypeVar + +T = TypeVar("T") + +class Box(Generic[T]): + def __init__(self, value: T) -> None: ... + +Box[str]("hello") +#^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + assert!( + report.contains("Box[str]"), + "Expected generic constructor to show Box[str], got: {report}" + ); +} diff --git a/pyrefly/lib/test/lsp/signature_help.rs b/pyrefly/lib/test/lsp/signature_help.rs index bd53110088..4d5b0356c6 100644 --- a/pyrefly/lib/test/lsp/signature_help.rs +++ b/pyrefly/lib/test/lsp/signature_help.rs @@ -982,3 +982,25 @@ Box[int](42) "Expected generic constructor to show Box[int], got: {report}" ); } + +#[test] +fn method_call_signature_unchanged() { + let code = r#" +class Foo: + def method(self, x: int) -> str: ... + +foo = Foo() +foo.method() +# ^ +"#; + let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); + // Method calls should still show their original return type + assert!( + report.contains("-> str"), + "Expected method signature to show -> str, got: {report}" + ); + assert!( + report.contains("parameters=[x: int]"), + "Expected parameters, got: {report}" + ); +} From 879d8796f81aef8883b7cde8f09fc059c84e7715 Mon Sep 17 00:00:00 2001 From: spyrosgavriil Date: Sat, 17 Jan 2026 11:33:48 +0200 Subject: [PATCH 3/4] Fix syntax errors from merge conflict resolution --- pyrefly/lib/test/lsp/hover.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index e43dfc76ce..d60535ad2b 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -922,6 +922,10 @@ Person() assert!( !report.contains("-> None"), "Constructor hover should not show -> None, got: {report}" + ); +} + +#[test] fn hover_over_in_operator_shows_contains_dunder() { let code = r#" class Container: @@ -952,6 +956,10 @@ Person("Alice", 25) assert!( report.contains("-> Person"), "Expected constructor hover to show -> Person, got: {report}" + ); +} + +#[test] fn hover_over_in_keyword_in_for_loop() { let code = r#" for x in [1, 2, 3]: @@ -984,6 +992,10 @@ Person.__init__(p, "Alice") assert!( !report.contains("-> Person") || report.contains("__init__"), "Direct __init__ call should show -> None, got: {report}" + ); +} + +#[test] fn hover_over_in_keyword_in_list_comprehension() { let code = r#" result = [x for x in [1, 2, 3] if x in [1]] @@ -1053,6 +1065,10 @@ Box[str]("hello") assert!( report.contains("Box[str]"), "Expected generic constructor to show Box[str], got: {report}" + ); +} + +#[test] fn hover_over_in_keyword_for_membership_in_comprehension() { let code = r#" result = [x for x in [1, 2, 3] if x in [1]] From e1ee044f1a240517b08c6e9799d194d11a137b78 Mon Sep 17 00:00:00 2001 From: spyrosgavriil Date: Sun, 18 Jan 2026 15:44:12 +0200 Subject: [PATCH 4/4] Refactor constructor detection logic - Extract is_constructor_call() utility function to eliminate duplication - Simplify should_override initialization logic - Update tests to assert on complete signature strings - Replace string slicing with type-based detection --- pyrefly/lib/lsp/wasm/hover.rs | 54 +++------------- pyrefly/lib/lsp/wasm/signature_help.rs | 90 ++++++++++---------------- pyrefly/lib/test/lsp/hover.rs | 9 +-- 3 files changed, 44 insertions(+), 109 deletions(-) diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs index 98407ab847..fff3fc3e59 100644 --- a/pyrefly/lib/lsp/wasm/hover.rs +++ b/pyrefly/lib/lsp/wasm/hover.rs @@ -42,6 +42,8 @@ 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::wasm::signature_help::is_constructor_call; +use crate::lsp::wasm::signature_help::override_constructor_return_type; use crate::state::lsp::DefinitionMetadata; use crate::state::lsp::FindDefinitionItemWithDocstring; use crate::state::lsp::FindPreference; @@ -529,53 +531,13 @@ pub fn get_hover( .or_else(find_callee_range_at_position); if let Some(callee_range) = callee_range_opt { - // Determine if this is a constructor call vs direct __init__ call or regular method call - let is_constructor_call = transaction - .get_module_info(handle) - .map(|module| { - let contents = module.contents(); - let start = callee_range.start().to_usize(); - let end = callee_range.end().to_usize(); - contents - .get(start..end) - .map(|callee_text| { - if callee_text.ends_with(".__init__") { - return false; - } - if let Some(last_dot_pos) = callee_text.rfind('.') { - let after_dot = &callee_text[last_dot_pos + 1..]; - if !after_dot.is_empty() - && after_dot.chars().all(|c| c.is_alphanumeric() || c == '_') - && (after_dot.chars().next().unwrap().is_alphabetic() - || after_dot.starts_with('_')) - { - return false; - } - } - true - }) - .unwrap_or(false) - }) - .unwrap_or(false); + let is_constructor = transaction + .get_answers(handle) + .and_then(|ans| ans.get_type_trace(callee_range)) + .is_some_and(is_constructor_call); - // Override the return type if this is a constructor call - if is_constructor_call { - if let Some(mut callable) = type_.clone().to_callable() - && callable.ret.is_none() - { - // Check if first parameter is self or cls - if let Params::List(ref params_list) = callable.params { - if let Some( - Param::Pos(name, self_type, _) | Param::PosOnly(Some(name), self_type, _), - ) = params_list.items().first() - { - if name.as_str() == "self" || name.as_str() == "cls" { - callable.ret = self_type.clone(); - type_ = Type::Callable(Box::new(callable)); - } - } - } - } + if is_constructor && let Some(new_type) = override_constructor_return_type(type_.clone()) { + type_ = new_type; } } diff --git a/pyrefly/lib/lsp/wasm/signature_help.rs b/pyrefly/lib/lsp/wasm/signature_help.rs index 32a1222b9a..dc75c6f926 100644 --- a/pyrefly/lib/lsp/wasm/signature_help.rs +++ b/pyrefly/lib/lsp/wasm/signature_help.rs @@ -37,6 +37,34 @@ use crate::types::callable::Param; use crate::types::callable::Params; use crate::types::types::Type; +pub(crate) fn is_constructor_call(callee_type: Type) -> bool { + matches!(callee_type, Type::ClassDef(_)) + || matches!(callee_type, Type::Type(inner) if matches!(inner.as_ref(), Type::ClassType(_) | Type::ClassDef(_))) +} + +pub(crate) fn override_constructor_return_type(type_: Type) -> Option { + let mut callable = type_.clone().to_callable()?; + if !callable.ret.is_none() { + return None; + } + + let mut should_override = false; + if let Params::List(ref params_list) = callable.params + && let Some(Param::Pos(name, self_type, _) | Param::PosOnly(Some(name), self_type, _)) = + params_list.items().first() + && (name.as_str() == "self" || name.as_str() == "cls") + { + callable.ret = self_type.clone(); + should_override = true; + } + + if should_override { + Some(Type::Callable(Box::new(callable))) + } else { + None + } +} + /// The currently active argument in a function call for signature help. #[derive(Debug)] pub(crate) enum ActiveArgument { @@ -285,34 +313,8 @@ impl Transaction<'_> { let type_ = type_.deterministic_printing(); // Display the return type as the class instance type instead of None - let display_type = if is_constructor_call - && let Some(mut callable) = type_.clone().to_callable() - && callable.ret.is_none() - { - // Check if first parameter is self or cls - let should_override = if let Params::List(ref params_list) = callable.params { - if let Some( - Param::Pos(name, self_type, _) | Param::PosOnly(Some(name), self_type, _), - ) = params_list.items().first() - { - if name.as_str() == "self" || name.as_str() == "cls" { - callable.ret = self_type.clone(); - true - } else { - false - } - } else { - false - } - } else { - false - }; - - if should_override { - Type::Callable(Box::new(callable)) - } else { - type_ - } + let display_type = if is_constructor_call { + override_constructor_return_type(type_.clone()).unwrap_or(type_) } else { type_ }; @@ -371,36 +373,10 @@ impl Transaction<'_> { let parameter_docs = self.parameter_documentation_for_callee(handle, callee_range); let function_docstring = self.function_docstring_for_callee(handle, callee_range); - // Determine if this is a constructor call vs direct __init__ call or regular method call let is_constructor_call = self - .get_module_info(handle) - .map(|module| { - let contents = module.contents(); - let start = callee_range.start().to_usize(); - let end = callee_range.end().to_usize(); - contents - .get(start..end) - .map(|callee_text| { - if callee_text.ends_with(".__init__") { - return false; - } - if let Some(last_dot_pos) = callee_text.rfind('.') { - let after_dot = &callee_text[last_dot_pos + 1..]; - if !after_dot.is_empty() - && after_dot - .chars() - .all(|c| c.is_alphanumeric() || c == '_') - && (after_dot.chars().next().unwrap().is_alphabetic() - || after_dot.starts_with('_')) - { - return false; - } - } - true - }) - .unwrap_or(false) - }) - .unwrap_or(false); + .get_answers(handle) + .and_then(|ans| ans.get_type_trace(callee_range)) + .is_some_and(is_constructor_call); let signatures = callables .into_iter() diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index d60535ad2b..ddab560af7 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -916,12 +916,9 @@ Person() "#; let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report); assert!( - report.contains("-> Person"), - "Expected constructor hover to show -> Person, got: {report}" - ); - assert!( - !report.contains("-> None"), - "Constructor hover should not show -> None, got: {report}" + report + .contains("def Person(\n self: Person,\n name: str,\n age: int\n) -> Person"), + "Expected constructor hover to show complete signature with -> Person, got: {report}" ); }