Support signature help

SamChou19815 · facebook-github-bot · commit 0f0f773a4745 · 2025-06-09T15:52:26.000-07:00
Summary: This diff implements signature help. We can generate signature help by inspecting the type of the callee when it can be converted into a `Type::Callable`. In addition, for overloads, we will use the information recorded in the last diff to show the signature help of the likely chosen overload, as well as all other options.

Reviewed By: kinto0

Differential Revision: D76222222

fbshipit-source-id: 51569c32fb9450faa3721d3d89d1024da4f2be7a
diff --git a/pyrefly/lib/alt/answers.rs b/pyrefly/lib/alt/answers.rs
@@ -557,7 +557,6 @@ impl Answers {
     }
 
     /// Returns all the overload, and the index of a chosen one
-    #[allow(dead_code)]
     pub fn get_all_overload_trace(
         &self,
         range: TextRange,
diff --git a/pyrefly/lib/commands/lsp.rs b/pyrefly/lib/commands/lsp.rs
@@ -87,6 +87,9 @@ use lsp_types::RegistrationParams;
 use lsp_types::RelatedFullDocumentDiagnosticReport;
 use lsp_types::RelativePattern;
 use lsp_types::ServerCapabilities;
+use lsp_types::SignatureHelp;
+use lsp_types::SignatureHelpOptions;
+use lsp_types::SignatureHelpParams;
 use lsp_types::TextDocumentSyncCapability;
 use lsp_types::TextDocumentSyncKind;
 use lsp_types::TextEdit;
@@ -119,6 +122,7 @@ use lsp_types::request::HoverRequest;
 use lsp_types::request::InlayHintRequest;
 use lsp_types::request::References;
 use lsp_types::request::RegisterCapability;
+use lsp_types::request::SignatureHelpRequest;
 use lsp_types::request::UnregisterCapability;
 use lsp_types::request::WorkspaceConfiguration;
 use path_absolutize::Absolutize;
@@ -572,6 +576,10 @@ pub fn run_lsp(
                 Some(OneOf::Left(true))
             }
         },
+        signature_help_provider: Some(SignatureHelpOptions {
+            trigger_characters: Some(vec!["(".to_owned(), ",".to_owned()]),
+            ..Default::default()
+        }),
         hover_provider: Some(HoverProviderCapability::Simple(true)),
         inlay_hint_provider: Some(OneOf::Left(true)),
         document_symbol_provider: Some(OneOf::Left(true)),
@@ -813,6 +821,14 @@ impl Server {
                     ide_transaction_manager.save(transaction);
                 } else if let Some(params) = as_request::<References>(&x) {
                     self.references(x.id, ide_transaction_manager, params);
+                } else if let Some(params) = as_request::<SignatureHelpRequest>(&x) {
+                    let transaction =
+                        ide_transaction_manager.non_commitable_transaction(&self.state);
+                    self.send_response(new_response(
+                        x.id,
+                        Ok(self.signature_help(&transaction, params)),
+                    ));
+                    ide_transaction_manager.save(transaction);
                 } else if let Some(params) = as_request::<HoverRequest>(&x) {
                     let default_response = Hover {
                         contents: HoverContents::Array(Vec::new()),
@@ -1491,6 +1507,18 @@ impl Server {
         });
     }
 
+    fn signature_help(
+        &self,
+        transaction: &Transaction<'_>,
+        params: SignatureHelpParams,
+    ) -> Option<SignatureHelp> {
+        let uri = &params.text_document_position_params.text_document.uri;
+        let handle = self.make_handle_if_enabled(uri)?;
+        let info = transaction.get_module_info(&handle)?;
+        let position = position_to_text_size(&info, params.text_document_position_params.position);
+        transaction.get_signature_help_at(&handle, position)
+    }
+
     fn hover(&self, transaction: &Transaction<'_>, params: HoverParams) -> Option<Hover> {
         let uri = &params.text_document_position_params.text_document.uri;
         let handle = self.make_handle_if_enabled(uri)?;
diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs
@@ -12,7 +12,12 @@ use itertools::Itertools;
 use lsp_types::CompletionItem;
 use lsp_types::CompletionItemKind;
 use lsp_types::DocumentSymbol;
+use lsp_types::ParameterInformation;
+use lsp_types::ParameterLabel;
+use lsp_types::SignatureHelp;
+use lsp_types::SignatureInformation;
 use pyrefly_util::gas::Gas;
+use pyrefly_util::prelude::SliceExt;
 use pyrefly_util::prelude::VecExt;
 use pyrefly_util::task_heap::Cancelled;
 use pyrefly_util::visit::Visit;
@@ -55,8 +60,11 @@ use crate::state::require::Require;
 use crate::state::state::CancellableTransaction;
 use crate::state::state::Transaction;
 use crate::sys_info::SysInfo;
+use crate::types::callable::Param;
+use crate::types::callable::Params;
 use crate::types::lsp::source_range_to_range;
 use crate::types::module::Module;
+use crate::types::types::BoundMethodType;
 use crate::types::types::Type;
 
 const INITIAL_GAS: Gas = Gas::new(20);
@@ -321,6 +329,120 @@ impl<'a> Transaction<'a> {
         }
     }
 
+    pub fn get_signature_help_at(
+        &self,
+        handle: &Handle,
+        position: TextSize,
+    ) -> Option<SignatureHelp> {
+        let mod_module = self.get_ast(handle)?;
+        fn visit(x: &Expr, find: TextSize, res: &mut Option<(TextRange, TextRange, usize)>) {
+            if let Expr::Call(call) = x
+                && call.arguments.range.contains_inclusive(find)
+            {
+                for (i, arg) in call.arguments.args.as_ref().iter().enumerate() {
+                    if arg.range().contains_inclusive(find) {
+                        visit(arg, find, res);
+                        if res.is_some() {
+                            return;
+                        }
+                        *res = Some((call.func.range(), call.arguments.range, i));
+                    }
+                }
+                if res.is_none() {
+                    *res = Some((
+                        call.func.range(),
+                        call.arguments.range,
+                        call.arguments.len(),
+                    ));
+                }
+            } else {
+                x.recurse(&mut |x| visit(x, find, res));
+            }
+        }
+        let mut res = None;
+        mod_module.visit(&mut |x| visit(x, position, &mut res));
+        let (callee_range, call_args_range, arg_index) = res?;
+        let answers = self.get_answers(handle)?;
+        if let Some((overloads, chosen_overload_index)) =
+            answers.get_all_overload_trace(call_args_range)
+        {
+            let signatures = overloads.into_map(|callable| {
+                Self::create_signature_information(Type::Callable(Box::new(callable)), arg_index)
+            });
+            Some(SignatureHelp {
+                signatures,
+                active_signature: chosen_overload_index.map(|i| i as u32),
+                active_parameter: Some(arg_index as u32),
+            })
+        } else {
+            answers
+                .get_type_trace(callee_range)
+                .map(|callee_type| SignatureHelp {
+                    signatures: vec![Self::create_signature_information(
+                        callee_type.arc_clone(),
+                        arg_index,
+                    )],
+                    active_signature: Some(0),
+                    active_parameter: Some(arg_index as u32),
+                })
+        }
+    }
+
+    fn create_signature_information(type_: Type, arg_index: usize) -> SignatureInformation {
+        let type_ = type_.deterministic_printing();
+        let label = format!("{}", type_);
+        let (parameters, active_parameter) = if let Some(params) =
+            Self::normalize_singleton_function_type_for_signature_help(type_)
+        {
+            let active_parameter = if arg_index < params.len() {
+                Some(arg_index as u32)
+            } else {
+                None
+            };
+            (
+                Some(params.map(|param| ParameterInformation {
+                    label: ParameterLabel::Simple(format!("{}", param)),
+                    documentation: None,
+                })),
+                active_parameter,
+            )
+        } else {
+            (None, None)
+        };
+        SignatureInformation {
+            label,
+            documentation: None,
+            parameters,
+            active_parameter,
+        }
+    }
+
+    fn normalize_singleton_function_type_for_signature_help(type_: Type) -> Option<Vec<Param>> {
+        let callable = match type_ {
+            Type::Callable(callable) => Some(*callable),
+            Type::Function(function) => Some(function.signature),
+            Type::BoundMethod(bound_method) => match bound_method.func {
+                BoundMethodType::Function(function) => Some(function.signature),
+                BoundMethodType::Forall(forall) => Some(forall.body.signature),
+                BoundMethodType::Overload(_) => None,
+            },
+            _ => None,
+        }?;
+        // We will drop the self parameter for signature help
+        if let Params::List(params_list) = callable.params {
+            if let Some(Param::PosOnly(Some(name), _, _) | Param::Pos(name, _, _)) =
+                params_list.items().first()
+                && name.as_str() == "self"
+            {
+                let mut params = params_list.into_items();
+                params.remove(0);
+                return Some(params);
+            }
+            return Some(params_list.into_items());
+        }
+        None
+    }
+
     fn resolve_named_import(
         &self,
         handle: &Handle,
diff --git a/pyrefly/lib/test/lsp/lsp_interaction_util.rs b/pyrefly/lib/test/lsp/lsp_interaction_util.rs
@@ -26,6 +26,7 @@ use lsp_types::CompletionOptions;
 use lsp_types::HoverProviderCapability;
 use lsp_types::OneOf;
 use lsp_types::ServerCapabilities;
+use lsp_types::SignatureHelpOptions;
 use lsp_types::TextDocumentSyncCapability;
 use lsp_types::TextDocumentSyncKind;
 use lsp_types::Url;
@@ -300,6 +301,10 @@ fn get_initialize_responses(find_refs: bool) -> Vec<Message> {
             } else {
                 None
             },
+            signature_help_provider: Some(SignatureHelpOptions {
+                trigger_characters: Some(vec!["(".to_owned(), ",".to_owned()]),
+                ..Default::default()
+            }),
             hover_provider: Some(HoverProviderCapability::Simple(true)),
             inlay_hint_provider: Some(OneOf::Left(true)),
             document_symbol_provider: Some(OneOf::Left(true)),
diff --git a/pyrefly/lib/test/lsp/mod.rs b/pyrefly/lib/test/lsp/mod.rs
@@ -17,3 +17,4 @@ mod inlay_hint;
 mod local_find_refs;
 mod lsp_interaction;
 mod lsp_interaction_util;
+mod signature_help;
diff --git a/pyrefly/lib/test/lsp/signature_help.rs b/pyrefly/lib/test/lsp/signature_help.rs
diff --git a/pyrefly/lib/types/callable.rs b/pyrefly/lib/types/callable.rs

Original file line number	Diff line number	Diff line change
`@@ -557,7 +557,6 @@ impl Answers {`
`557`	`557`	`}`
`558`	`558`
`559`	`559`	`/// Returns all the overload, and the index of a chosen one`
`560`		`- #[allow(dead_code)]`
`561`	`560`	`pub fn get_all_overload_trace(`
`562`	`561`	`&self,`
`563`	`562`	`range: TextRange,`