Function signature helper now respects original definition generic names when arguments are unbound

Author: scristobal

CHANGELOG.md

@@ -204,7 +204,7 @@
   ([Hari Mohan](https://github.com/seafoamteal))
 
 - The language server now supports renaming, go to definition, hover, and
-   finding references from expressions in case clause guards.
+  finding references from expressions in case clause guards.
   ([Surya Rose](https://github.com/GearsDatapacks))
 
 - The language server now supports `textDocument/foldingRange`, enabling
@@ -227,6 +227,33 @@
 
   ([Aayush Tripathi](https://github.com/aayush-tripathi))
 
+- The function signature helper now displays original function definition
+  generic names when arguments are unbound. For example, in this code:
+
+  ```gleam
+  pub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }
+
+  pub fn main() {
+      wibble( )
+          // ↑
+  }
+  ```
+
+  will show a signature help
+
+  ```gleam
+  wibble(something, fn() -> something, anything)
+
+  ```
+
+  instead of
+
+  ```gleam
+  wibble(a, fn() -> a, b) -> Nil
+  ```
+
+  ([Samuel Cristobal](https://github.com/scristobal))
+
 ### Formatter
 
 - The formatter no longer wraps multiple tuple or field access into a block.

compiler-core/src/analyse.rs

@@ -346,6 +346,10 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
             self.check_for_type_leaks(value)
         }
 
+        // Resolve deferred type variable aliases now that all unification is
+        // done and link chains are stable.
+        env.resolve_deferred_type_variable_aliases();
+
         let Environment {
             module_types: types,
             module_types_constructors: types_constructors,

compiler-core/src/type_/environment.rs

@@ -86,6 +86,13 @@ pub struct Environment<'a> {
 
     pub names: Names,
 
+    /// Deferred type variable aliases: `(source_generic_id, instantiated_type)`.
+    /// During `instantiate`, when a generic type variable is replaced with a new
+    /// unbound var, we record the pair here. After type checking, we resolve
+    /// these by following link chains in the types to find the final unbound var
+    /// IDs and register the original names for them.
+    pub deferred_type_variable_aliases: Vec<(u64, Arc<Type>)>,
+
     /// Wether we ran into an `echo` or not while analysing the current module.
     pub echo_found: bool,
 
@@ -143,6 +150,7 @@ impl<'a> Environment<'a> {
             local_variable_usages: vec![HashMap::new()],
             target_support,
             names,
+            deferred_type_variable_aliases: Vec::new(),
             module_type_aliases: HashMap::new(),
             echo_found: false,
             references: ReferenceTracker::new(),
@@ -258,6 +266,38 @@ impl Environment<'_> {
         self.previous_id
     }
 
+    /// Resolve deferred type variable aliases. During `instantiate`, generic
+    /// type variables are replaced with new unbound vars, but those vars may
+    /// later get linked to other vars during unification. This method follows
+    /// the link chains to find the final unbound var IDs and registers the
+    /// original names for them.
+    pub fn resolve_deferred_type_variable_aliases(&mut self) {
+        for (source_id, type_) in self.deferred_type_variable_aliases.drain(..) {
+            if let Some(alias) = self.names.get_type_variable(source_id).cloned() {
+                let final_id = Self::resolve_type_var_id(&type_);
+                if let Some(id) = final_id {
+                    // Only register if the target doesn't already have a name.
+                    // This avoids overwriting user-provided names (e.g. from
+                    // annotations) with names from instantiated type parameters.
+                    if self.names.get_type_variable(id).is_none() {
+                        self.names.type_variable_in_scope(id, alias);
+                    }
+                }
+            }
+        }
+    }
+
+    /// Follow link chains in a type to find the final unbound or generic var ID.
+    fn resolve_type_var_id(type_: &Type) -> Option<u64> {
+        match type_ {
+            Type::Var { type_ } => match type_.borrow().deref() {
+                TypeVar::Unbound { id } | TypeVar::Generic { id } => Some(*id),
+                TypeVar::Link { type_ } => Self::resolve_type_var_id(type_),
+            },
+            Type::Named { .. } | Type::Fn { .. } | Type::Tuple { .. } => None,
+        }
+    }
+
     /// Create a new unbound type that is a specific type, we just don't
     /// know which one yet.
     ///
@@ -609,6 +649,12 @@ impl Environment<'_> {
                                 // Check this in the hydrator, i.e. is it a created type
                                 let v = self.new_unbound_var();
                                 let _ = ids.insert(*id, v.clone());
+
+                                // Record this pairing so that after type checking
+                                // we can resolve any link chains and register the
+                                // original name for the final unbound variable.
+                                self.deferred_type_variable_aliases.push((*id, v.clone()));
+
                                 return v;
                             }
                         }

language-server/src/engine.rs

@@ -1129,15 +1129,21 @@ Unused labelled fields:
         &mut self,
         params: lsp_types::SignatureHelpParams,
     ) -> Response<Option<SignatureHelp>> {
-        self.respond(
-            |this| match this.node_at_position(&params.text_document_position_params) {
+        self.respond(|this| {
+            let Some(module) =
+                this.module_for_uri(&params.text_document_position_params.text_document.uri)
+            else {
+                return Ok(None);
+            };
+
+            match this.node_at_position(&params.text_document_position_params) {
                 Some((_lines, Located::Expression { expression, .. })) => {
-                    Ok(signature_help::for_expression(expression))
+                    Ok(signature_help::for_expression(expression, module))
                 }
                 Some((_lines, _located)) => Ok(None),
                 None => Ok(None),
-            },
-        )
+            }
+        })
     }
 
     fn module_node_at_position(

language-server/src/signature_help.rs

@@ -11,10 +11,11 @@ use lsp_types::{
 
 use gleam_core::{
     ast::{CallArg, ImplicitCallArgOrigin, TypedExpr},
-    type_::{FieldMap, ModuleValueConstructor, Type, pretty::Printer},
+    build::Module,
+    type_::{FieldMap, ModuleValueConstructor, Type, printer::Printer},
 };
 
-pub fn for_expression(expr: &TypedExpr) -> Option<SignatureHelp> {
+pub fn for_expression(expr: &TypedExpr, module: &Module) -> Option<SignatureHelp> {
     // If we're inside a function call we can provide signature help,
     // otherwise we don't want anything to pop up.
     let TypedExpr::Call { fun, arguments, .. } = expr else {
@@ -27,7 +28,13 @@ pub fn for_expression(expr: &TypedExpr) -> Option<SignatureHelp> {
         // help.
         TypedExpr::Var {
             constructor, name, ..
-        } => signature_help(name.clone(), fun, arguments, constructor.field_map()),
+        } => signature_help(
+            name.clone(),
+            fun,
+            arguments,
+            constructor.field_map(),
+            module,
+        ),
 
         // If we're making a qualified call to another module's function
         // then we want to show its type, documentation and the exact name
@@ -50,7 +57,7 @@ pub fn for_expression(expr: &TypedExpr) -> Option<SignatureHelp> {
                 | ModuleValueConstructor::Fn { field_map, .. } => field_map.into(),
             };
             let name = format!("{module_alias}.{label}").into();
-            signature_help(name, fun, arguments, field_map)
+            signature_help(name, fun, arguments, field_map, module)
         }
 
         // If the function being called is an invalid node we don't want to
@@ -87,7 +94,7 @@ pub fn for_expression(expr: &TypedExpr) -> Option<SignatureHelp> {
         | TypedExpr::BitArray { .. }
         | TypedExpr::RecordUpdate { .. }
         | TypedExpr::NegateBool { .. }
-        | TypedExpr::NegateInt { .. } => signature_help("fn".into(), fun, arguments, None),
+        | TypedExpr::NegateInt { .. } => signature_help("fn".into(), fun, arguments, None, module),
     }
 }
 
@@ -109,6 +116,7 @@ fn signature_help(
     fun: &TypedExpr,
     supplied_arguments: &[CallArg<TypedExpr>],
     field_map: Option<&FieldMap>,
+    module: &Module,
 ) -> Option<SignatureHelp> {
     let (arguments, return_) = fun.type_().fn_types()?;
 
@@ -127,7 +135,7 @@ fn signature_help(
         None => HashMap::new(),
     };
 
-    let printer = Printer::new();
+    let printer = Printer::new(&module.ast.names);
     let (label, parameters) =
         print_signature_help(printer, fun_name, arguments, return_, &index_to_label);
 
@@ -240,7 +248,7 @@ fn active_parameter_index(
 /// `ParameterInformation` for all its arguments.
 ///
 fn print_signature_help(
-    mut printer: Printer,
+    mut printer: Printer<'_>,
     function_name: EcoString,
     arguments: Vec<Arc<Type>>,
     return_: Arc<Type>,
@@ -256,7 +264,7 @@ fn print_signature_help(
             signature.push_str(label);
             signature.push_str(": ");
         }
-        signature.push_str(&printer.pretty_print(argument, 0));
+        signature.push_str(&printer.print_type(argument));
         let arg_end = signature.len();
         let label = ParameterLabel::LabelOffsets([arg_start as u32, arg_end as u32]);
 
@@ -272,6 +280,6 @@ fn print_signature_help(
     }
 
     signature.push_str(") -> ");
-    signature.push_str(&printer.pretty_print(&return_, 0));
+    signature.push_str(&printer.print_type(&return_));
     (signature, parameter_informations)
 }

language-server/src/tests/signature_help.rs

@@ -451,3 +451,45 @@ pub fn main() {
         find_position_of(r#"Pokemon(name: "Jirachi",)"#).under_last_char()
     );
 }
+
+#[test]
+pub fn help_for_use_function_call_uses_generic_names_when_missing_all_arguments() {
+    assert_signature_help!(
+        r#"
+pub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }
+pub fn main() {
+    wibble( )
+}
+"#,
+        find_position_of("wibble( ").under_last_char()
+    );
+}
+
+#[test]
+pub fn help_for_use_function_call_uses_concrete_types_when_possible_or_generic_names_when_unbound()
+{
+    assert_signature_help!(
+        r#"
+pub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }
+pub fn main() {
+    wibble(1, )
+}
+"#,
+        find_position_of("wibble(1, )").under_last_char()
+    );
+}
+
+#[test]
+pub fn help_for_use_function_call_uses_concrete_types_when_late_ubound() {
+    assert_signature_help!(
+        r#"
+fn identity(x: a) -> a { x }
+
+fn main() {
+  let a = Ok(10)
+  identity( a)
+}
+"#,
+        find_position_of("identity( ").under_last_char()
+    );
+}

language-server/src/tests/snapshots/gleam_language_server__tests__action__generate_function_capture.snap

@@ -1,6 +1,7 @@
 ---
-source: compiler-core/src/language_server/tests/action.rs
+source: language-server/src/tests/action.rs
 expression: "\nfn map(list: List(a), f: fn(a) -> b) -> List(b) {\n  todo\n}\n\npub fn main() {\n  map([1, 2, 3], add(_, 1))\n}\n"
+snapshot_kind: text
 ---
 ----- BEFORE ACTION
 
@@ -24,6 +25,6 @@ pub fn main() {
   map([1, 2, 3], add(_, 1))
 }
 
-fn add(int: Int, int_2: Int) -> a {
+fn add(int: Int, int_2: Int) -> b {
   todo
 }

language-server/src/tests/snapshots/gleam_language_server__tests__signature_help__help_for_use_function_call_uses_concrete_types_when_late_ubound.snap

@@ -0,0 +1,18 @@
+---
+source: language-server/src/tests/signature_help.rs
+expression: "\nfn identity(x: a) -> a { x }\n\nfn main() {\n  let a = Ok(10)\n  identity( a)\n}\n"
+snapshot_kind: text
+---
+fn identity(x: a) -> a { x }
+
+fn main() {
+  let a = Ok(10)
+  identity( a)
+           ↑  
+}
+
+
+----- Signature help -----
+identity(Result(Int, b)) -> Result(Int, b)
+
+No documentation

language-server/src/tests/snapshots/gleam_language_server__tests__signature_help__help_for_use_function_call_uses_concrete_types_when_possible_or_generic_names_when_unbound.snap

@@ -0,0 +1,17 @@
+---
+source: language-server/src/tests/signature_help.rs
+expression: "\npub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }\npub fn main() {\n    wibble(1, )\n}\n"
+snapshot_kind: text
+---
+pub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }
+pub fn main() {
+    wibble(1, )
+              ↑
+}
+
+
+----- Signature help -----
+wibble(Int, fn() -> Int, anything) -> Nil
+            ▔▔▔▔▔▔▔▔▔▔▔
+
+No documentation

language-server/src/tests/snapshots/gleam_language_server__tests__signature_help__help_for_use_function_call_uses_generic_names_when_missing_all_arguments.snap

@@ -0,0 +1,17 @@
+---
+source: language-server/src/tests/signature_help.rs
+expression: "\npub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }\npub fn main() {\n    wibble( )\n}\n"
+snapshot_kind: text
+---
+pub fn wibble(x: something, y: fn() -> something, z: anything) { Nil }
+pub fn main() {
+    wibble( )
+           ↑ 
+}
+
+
+----- Signature help -----
+wibble(something, fn() -> something, anything) -> Nil
+       ▔▔▔▔▔▔▔▔▔
+
+No documentation