Repository: gleam-lang/gleam (rust)

Issue:
"Generate function" code action doesn't take labels into account
At the moment if I have a function that doesn't exist and is also using labels like this: `wibble(wobble: 1)`, the "Generate function" code action will generate a function that has no labels and so will be invalid. It would be nice if it took labels into account as well. One think I'm not sure how to deal with is what to do when the labels are not right:
- If there's duplicate labels should we generate an invalid function with duplicate labels? Or should the LS not prompt any action at all?
- If there's an unlabelled argument after labelled ones should the language server generate an invalid function with labelled arguments in the wrong place, a valid function but without the wrong-positioned labels, or should it refuse to generate anything at all?


Pull request description:
"Generate function" no longer ignores labels
This pr fixes #4226

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a02bbf17d..cd5e1e183 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -145,6 +145,9 @@
 - Fixed a bug where division and remainder operators would not work correctly
   in guards on the JavaScript target.
   ([Surya Rose](https://github.com/GearsDatapacks))
+- Fixed a bug where the "Generate function" code action would ignore the
+  provided labels
+  ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
 
 ## v1.8.1 - 2025-02-11
 
diff --git a/compiler-core/src/ast/typed.rs b/compiler-core/src/ast/typed.rs
index 16e772d40..bf4e087bf 100644
--- a/compiler-core/src/ast/typed.rs
+++ b/compiler-core/src/ast/typed.rs
@@ -876,6 +876,13 @@ impl TypedExpr {
             },
         }
     }
+
+    pub(crate) fn is_invalid(&self) -> bool {
+        match self {
+            TypedExpr::Invalid { .. } => true,
+            _ => false,
+        }
+    }
 }
 
 impl<'a> From<&'a TypedExpr> for Located<'a> {
diff --git a/compiler-core/src/language_server/code_action.rs b/compiler-core/src/language_server/code_action.rs
index 038aea9dd..13f578e62 100644
--- a/compiler-core/src/language_server/code_action.rs
+++ b/compiler-core/src/language_server/code_action.rs
@@ -4081,7 +4081,7 @@ pub struct GenerateFunction<'a> {
 
 struct FunctionToGenerate<'a> {
     name: &'a str,
-    arguments_types: Vec<Arc<Type>>,
+    arguments: Vec<(Option<EcoString>, Arc<Type>)>,
     return_type: Arc<Type>,
     previous_function_end: Option<u32>,
 }
@@ -4106,7 +4106,7 @@ impl<'a> GenerateFunction<'a> {
 
         let Some(FunctionToGenerate {
             name,
-            arguments_types,
+            arguments,
             previous_function_end: Some(insert_at),
             return_type,
         }) = self.function_to_generate
@@ -4116,18 +4116,19 @@ impl<'a> GenerateFunction<'a> {
 
         let mut name_generator = NameGenerator::new();
         let mut printer = Printer::new(&self.module.ast.names);
-        let args = if let [arg_type] = arguments_types.as_slice() {
-            let arg_name = name_generator.generate_name_from_type(arg_type);
-            format!("{arg_name}: {}", printer.print_type(arg_type))
-        } else {
-            arguments_types
-                .iter()
-                .map(|arg_type| {
-                    let arg_name = name_generator.generate_name_from_type(arg_type);
-                    format!("{arg_name}: {}", printer.print_type(arg_type))
-                })
-                .join(", ")
-        };
+        let args = arguments
+            .iter()
+            .map(|(arg_label, arg_type)| {
+                let arg_name = name_generator.generate_name_from_type(arg_type);
+                let pretty_type = printer.print_type(arg_type);
+                if let Some(arg_label) = arg_label {
+                    format!("{arg_label} {arg_name}: {pretty_type}")
+                } else {
+                    format!("{arg_name}: {pretty_type}")
+                }
+            })
+            .join(", ");
+
         let return_type = printer.print_type(&return_type);
 
         self.edits.insert(
@@ -4143,6 +4144,34 @@ impl<'a> GenerateFunction<'a> {
             .push_to(&mut action);
         action
     }
+
+    fn try_save_function_to_generate(
+        &mut self,
+        function_name_location: SrcSpan,
+        function_type: &Arc<Type>,
+        labels: HashMap<usize, EcoString>,
+    ) {
+        let name_range = function_name_location.start as usize..function_name_location.end as usize;
+        let candidate_name = self.module.code.get(name_range);
+        match (candidate_name, function_type.fn_types()) {
+            (None, _) | (_, None) => (),
+            (Some(name), _) if !is_valid_lowercase_name(name) => (),
+            (Some(name), Some((arguments_types, return_type))) => {
+                let arguments = arguments_types
+                    .iter()
+                    .enumerate()
+                    .map(|(i, arg)| (labels.get(&i).cloned(), arg.clone()))
+                    .collect_vec();
+
+                self.function_to_generate = Some(FunctionToGenerate {
+                    name,
+                    arguments,
+                    return_type,
+                    previous_function_end: self.last_visited_function_end,
+                })
+            }
+        }
+    }
 }
 
 impl<'ast> ast::visit::Visit<'ast> for GenerateFunction<'ast> {
@@ -4154,24 +4183,62 @@ impl<'ast> ast::visit::Visit<'ast> for GenerateFunction<'ast> {
     fn visit_typed_expr_invalid(&mut self, location: &'ast SrcSpan, type_: &'ast Arc<Type>) {
         let invalid_range = self.edits.src_span_to_lsp_range(*location);
         if within(self.params.range, invalid_range) {
-            let name_range = location.start as usize..location.end as usize;
-            let candidate_name = self.module.code.get(name_range);
-            match (candidate_name, type_.fn_types()) {
-                (None, _) | (_, None) => return,
-                (Some(name), _) if !is_valid_lowercase_name(name) => return,
-                (Some(name), Some((arguments_types, return_type))) => {
-                    self.function_to_generate = Some(FunctionToGenerate {
-                        name,
-                        arguments_types,
-                        return_type,
-                        previous_function_end: self.last_visited_function_end,
-                    })
-                }
-            }
+            self.try_save_function_to_generate(*location, type_, HashMap::new());
         }
 
         ast::visit::visit_typed_expr_invalid(self, location, type_);
     }
+
+    fn visit_typed_expr_call(
+        &mut self,
+        location: &'ast SrcSpan,
+        type_: &'ast Arc<Type>,
+        fun: &'ast TypedExpr,
+        args: &'ast [TypedCallArg],
+    ) {
+        // If the function being called is invalid we need to generate a
+        // function that has the proper labels.
+        let fun_range = self.edits.src_span_to_lsp_range(fun.location());
+
+        if within(self.params.range, fun_range) && fun.is_invalid() {
+            if labels_are_correct(args) {
+                let labels = args
+                    .iter()
+                    .enumerate()
+                    .filter_map(|(i, arg)| arg.label.as_ref().map(|label| (i, label.clone())))
+                    .collect();
+
+                self.try_save_function_to_generate(fun.location(), &fun.type_(), labels);
+            }
+        } else {
+            ast::visit::visit_typed_expr_call(self, location, type_, fun, args);
+        }
+    }
+}
+
+#[must_use]
+/// Checks the labels in the given arguments are correct: that is there's no
+/// duplicate labels and all labelled arguments come after the unlabelled ones.
+fn labels_are_correct(args: &[TypedCallArg]) -> bool {
+    let mut labelled_arg_found = false;
+    let mut used_labels = HashSet::new();
+
+    for arg in args {
+        match &arg.label {
+            // Labels are invalid if there's duplicate ones or if an unlabelled
+            // argument comes after a labelled one.
+            Some(label) if used_labels.contains(label) => return false,
+            None if labelled_arg_found => return false,
+            // Otherwise we just add the label to the used ones.
+            Some(label) => {
+                labelled_arg_found = true;
+                let _ = used_labels.insert(label);
+            }
+            None => continue,
+        }
+    }
+
+    true
 }
 
 struct NameGenerator {


Test patch:
diff --git a/compiler-core/src/language_server/tests/action.rs b/compiler-core/src/language_server/tests/action.rs
index fa6d2f304..e4c969c81 100644
--- a/compiler-core/src/language_server/tests/action.rs
+++ b/compiler-core/src/language_server/tests/action.rs
@@ -4850,6 +4850,45 @@ pub fn main() {
     );
 }
 
+#[test]
+fn generate_function_takes_labels_into_account() {
+    assert_code_action!(
+        GENERATE_FUNCTION,
+        "
+pub fn main() {
+  wibble(2, n: 1)
+}
+",
+        find_position_of("wibble").to_selection()
+    );
+}
+
+#[test]
+fn generate_function_does_not_trigger_if_labels_are_in_the_wrong_order() {
+    assert_no_code_actions!(
+        GENERATE_FUNCTION,
+        "
+pub fn main() {
+  wibble(n: 2, 1)
+}
+",
+        find_position_of("wibble").to_selection()
+    );
+}
+
+#[test]
+fn generate_function_does_not_trigger_if_there_are_repeated_labels() {
+    assert_no_code_actions!(
+        GENERATE_FUNCTION,
+        "
+pub fn main() {
+  wibble(n: 2, n: 1)
+}
+",
+        find_position_of("wibble").to_selection()
+    );
+}
+
 #[test]
 fn pattern_match_on_argument_generates_unique_names_even_with_labels() {
     assert_code_action!(
diff --git a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_takes_labels_into_account.snap b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_takes_labels_into_account.snap
new file mode 100644
index 000000000..bdc7fadda
--- /dev/null
+++ b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_takes_labels_into_account.snap
@@ -0,0 +1,21 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "\npub fn main() {\n  wibble(2, n: 1)\n}\n"
+---
+----- BEFORE ACTION
+
+pub fn main() {
+  wibble(2, n: 1)
+  ↑              
+}
+
+
+----- AFTER ACTION
+
+pub fn main() {
+  wibble(2, n: 1)
+}
+
+fn wibble(int: Int, n int_2: Int) -> a {
+  todo
+}