Repository: gleam-lang/gleam (rust)

Issue:
Do not show errors for wrong labels if the called function/variant constructor doesn't exist
When calling a function (or variant constructor) that doesn't exists like this:
```gleam
pub fn main() {
  wibble(wobble: "hello, Joe!")
  // or Wibble(wobble: "Hello, Joe!")
}
```
The compiler complains about the function not existing:
```text
error: Unknown variable
  ┌─ /src/main.gleam:4:3
  │
4 │   Wibble(wobble: "Hello, Joe!")
  │   ^^^^^^

The custom type variant constructor `Wibble` is not in scope here.
```
But it also shows an error for each of the used labels:
```text
error: Unexpected labelled argument
  ┌─ /src/main.gleam:4:10
  │
4 │   Wibble(wobble: "Hello, Joe!")
  │          ^^^^^^^^^^^^^^^^^^^^^

This argument has been given a label but the constructor does
not expect any. Please remove the label `wobble`.
```
I think it doesn't make a lot of sense to show this error when the function itself doesn't exist and I'd like to remove it. What do you think?

Pull request description:
No longer show errors on labelled arguments of non existing functions
This PR closes #4422 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 60396f0d8..ee8630f71 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -90,10 +90,15 @@
 
   ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
 
+
 - Compilation of binary operators is now fault tolerant and won't stop at the
   first type error.
   ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
 
+- The compiler no longer shows errors for a function's labels if the called
+  function itself doesn't exist.
+  ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
+
 ### Build tool
 
 - Include a type annotation for the `main` function generated by `gleam new`.
@@ -319,6 +324,26 @@
 
   ([Matias Carlander](https://github.com/matiascr))
 
+- The "Generate function" code action now uses labels or variable names to improve
+  the names of generated arguments. For example:
+
+  ```gleam
+  pub fn main() {
+    let language = English
+    greet(language, name: "Louis")
+  }
+  ```
+
+  Will generate the following function:
+
+  ```gleam
+  pub fn greet(language: String, name name: String) -> a {
+    todo
+  }
+  ```
+
+  ([Surya Rose](https://github.com/GearsDatapacks))
+
 ### Formatter
 
 ### Container images
diff --git a/compiler-core/src/language_server/code_action.rs b/compiler-core/src/language_server/code_action.rs
index e072903c4..b2fc5cee0 100644
--- a/compiler-core/src/language_server/code_action.rs
+++ b/compiler-core/src/language_server/code_action.rs
@@ -4716,6 +4716,7 @@ pub struct GenerateFunction<'a> {
 struct FunctionToGenerate<'a> {
     name: &'a str,
     arguments: Vec<(Option<EcoString>, Arc<Type>)>,
+    given_arguments: Option<&'a [TypedCallArg]>,
     return_type: Arc<Type>,
     previous_function_end: Option<u32>,
 }
@@ -4741,6 +4742,7 @@ impl<'a> GenerateFunction<'a> {
         let Some(FunctionToGenerate {
             name,
             arguments,
+            given_arguments,
             previous_function_end: Some(insert_at),
             return_type,
         }) = self.function_to_generate
@@ -4752,8 +4754,31 @@ impl<'a> GenerateFunction<'a> {
         let mut printer = Printer::new(&self.module.ast.names);
         let args = arguments
             .iter()
-            .map(|(arg_label, arg_type)| {
-                let arg_name = name_generator.generate_name_from_type(arg_type);
+            .enumerate()
+            .map(|(index, (arg_label, arg_type))| {
+                let arg_name = if let Some(label) = arg_label {
+                    name_generator.add_used_name(label.clone());
+                    label.clone()
+                } else {
+                    let given_argument = given_arguments
+                        .and_then(|arguments| arguments.get(index).map(|argument| &argument.value));
+
+                    match given_argument {
+                        // If the argument is a record, we can't use it as an argument name.
+                        // Similarly, we don't want to base the variable name off a
+                        // compiler-generated variable like `_pipe`.
+                        Some(TypedExpr::Var {
+                            name, constructor, ..
+                        }) if !constructor.variant.is_record()
+                            && !constructor.variant.is_generated_variable() =>
+                        {
+                            name_generator.add_used_name(name.clone());
+                            name.clone()
+                        }
+                        _ => 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}")
@@ -4784,6 +4809,7 @@ impl<'a> GenerateFunction<'a> {
         function_name_location: SrcSpan,
         function_type: &Arc<Type>,
         labels: HashMap<usize, EcoString>,
+        given_arguments: Option<&'a [TypedCallArg]>,
     ) {
         let name_range = function_name_location.start as usize..function_name_location.end as usize;
         let candidate_name = self.module.code.get(name_range);
@@ -4800,6 +4826,7 @@ impl<'a> GenerateFunction<'a> {
                 self.function_to_generate = Some(FunctionToGenerate {
                     name,
                     arguments,
+                    given_arguments,
                     return_type,
                     previous_function_end: self.last_visited_function_end,
                 })
@@ -4817,7 +4844,7 @@ 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) {
-            self.try_save_function_to_generate(*location, type_, HashMap::new());
+            self.try_save_function_to_generate(*location, type_, HashMap::new(), None);
         }
 
         ast::visit::visit_typed_expr_invalid(self, location, type_);
@@ -4842,7 +4869,12 @@ impl<'ast> ast::visit::Visit<'ast> for GenerateFunction<'ast> {
                     .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);
+                self.try_save_function_to_generate(
+                    fun.location(),
+                    &fun.type_(),
+                    labels,
+                    Some(args),
+                );
             }
         } else {
             ast::visit::visit_typed_expr_call(self, location, type_, fun, args);
diff --git a/compiler-core/src/type_.rs b/compiler-core/src/type_.rs
index 44b4083f9..09be25eb8 100644
--- a/compiler-core/src/type_.rs
+++ b/compiler-core/src/type_.rs
@@ -775,6 +775,22 @@ impl ValueConstructorVariant {
         matches!(self, Self::LocalVariable { .. })
     }
 
+    /// Returns `true` if the variant is a local variable generated by the compiler.
+    #[must_use]
+    pub fn is_generated_variable(&self) -> bool {
+        match self {
+            ValueConstructorVariant::LocalVariable {
+                origin: VariableOrigin::Generated,
+                ..
+            } => true,
+            ValueConstructorVariant::LocalVariable { .. } => false,
+            ValueConstructorVariant::ModuleConstant { .. }
+            | ValueConstructorVariant::LocalConstant { .. }
+            | ValueConstructorVariant::ModuleFn { .. }
+            | ValueConstructorVariant::Record { .. } => false,
+        }
+    }
+
     /// Returns `true` if the value constructor variant is [`ModuleFn`].
     ///
     /// [`ModuleFn`]: ValueConstructorVariant::ModuleFn
diff --git a/compiler-core/src/type_/expression.rs b/compiler-core/src/type_/expression.rs
index 50e2e267b..c5cc5ebbf 100644
--- a/compiler-core/src/type_/expression.rs
+++ b/compiler-core/src/type_/expression.rs
@@ -3708,13 +3708,23 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
             .map_err(|e| convert_get_value_constructor_error(e, location, None))
             .and_then(|field_map| {
                 match field_map {
-                    // The fun has a field map so labelled arguments may be present and need to be reordered.
+                    // The fun has a field map so labelled arguments may be
+                    // present and need to be reordered.
                     Some(field_map) => field_map.reorder(&mut args, location),
 
-                    // The fun has no field map and so we error if arguments have been labelled
+                    // The fun has no field map and so we error if arguments
+                    // have been labelled.
+                    // There's an exception to this rule: if the function itself
+                    // doesn't exist (that is it's an `Invalid` expression), then
+                    // we don't want to error on any labels that might have been
+                    // used as it would be quite noisy. Once the function is
+                    // known to be a valid function we can make sure that there's
+                    // no labelled arguments if it doesn't actually have a field map.
+                    None if fun.is_invalid() => Ok(()),
                     None => assert_no_labelled_arguments(&args),
                 }
             });
+
         if let Err(e) = field_map {
             match e {
                 Error::IncorrectArity {


Test patch:
diff --git a/compiler-core/src/language_server/tests/action.rs b/compiler-core/src/language_server/tests/action.rs
index 66397eb35..d4354b2ec 100644
--- a/compiler-core/src/language_server/tests/action.rs
+++ b/compiler-core/src/language_server/tests/action.rs
@@ -6312,6 +6312,35 @@ pub fn main() {
     );
 }
 
+#[test]
+fn generate_function_generates_argument_names_from_labels() {
+    assert_code_action!(
+        GENERATE_FUNCTION,
+        "
+pub fn main() {
+  add(1, addend: 10)
+}
+",
+        find_position_of("add").to_selection()
+    );
+}
+
+#[test]
+fn generate_function_generates_argument_names_from_variables() {
+    assert_code_action!(
+        GENERATE_FUNCTION,
+        "
+pub fn main() {
+  let wibble = 10
+  let wobble = 20
+
+  wubble(wibble, wobble, 14)
+}
+",
+        find_position_of("wubble").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_generates_argument_names_from_labels.snap b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_generates_argument_names_from_labels.snap
new file mode 100644
index 000000000..1297f113f
--- /dev/null
+++ b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_generates_argument_names_from_labels.snap
@@ -0,0 +1,21 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "\npub fn main() {\n  add(1, addend: 10)\n}\n"
+---
+----- BEFORE ACTION
+
+pub fn main() {
+  add(1, addend: 10)
+  ↑                 
+}
+
+
+----- AFTER ACTION
+
+pub fn main() {
+  add(1, addend: 10)
+}
+
+fn add(int: Int, addend addend: Int) -> a {
+  todo
+}
diff --git a/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_generates_argument_names_from_variables.snap b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_generates_argument_names_from_variables.snap
new file mode 100644
index 000000000..377e3e562
--- /dev/null
+++ b/compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__generate_function_generates_argument_names_from_variables.snap
@@ -0,0 +1,27 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "\npub fn main() {\n  let wibble = 10\n  let wobble = 20\n\n  wubble(wibble, wobble, 14)\n}\n"
+---
+----- BEFORE ACTION
+
+pub fn main() {
+  let wibble = 10
+  let wobble = 20
+
+  wubble(wibble, wobble, 14)
+  ↑                         
+}
+
+
+----- AFTER ACTION
+
+pub fn main() {
+  let wibble = 10
+  let wobble = 20
+
+  wubble(wibble, wobble, 14)
+}
+
+fn wubble(wibble: Int, wobble: Int, int: Int) -> a {
+  todo
+}
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
index bdc7fadda..0b9e81426 100644
--- 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
@@ -16,6 +16,6 @@ pub fn main() {
   wibble(2, n: 1)
 }
 
-fn wibble(int: Int, n int_2: Int) -> a {
+fn wibble(int: Int, n n: Int) -> a {
   todo
 }
diff --git a/compiler-core/src/type_/tests/errors.rs b/compiler-core/src/type_/tests/errors.rs
index f87703f26..ecb5a15dd 100644
--- a/compiler-core/src/type_/tests/errors.rs
+++ b/compiler-core/src/type_/tests/errors.rs
@@ -2965,3 +2965,29 @@ pub fn main() {
 "#
     );
 }
+
+#[test]
+fn function_that_does_not_exist_does_not_produce_error_for_labelled_args() {
+    assert_module_error!(
+        r#"
+pub fn main() {
+  // We only want to error on `wibble` since it doesn't exist, we don't want
+  // an error on the label at this point!
+  wibble(label: 1)
+}
+"#
+    );
+}
+
+#[test]
+fn constructor_that_does_not_exist_does_not_produce_error_for_labelled_args() {
+    assert_module_error!(
+        r#"
+pub fn main() {
+  // We only want to error on `Wibble` since it doesn't exist, we don't want
+  // an error on the label at this point!
+  Wibble(label: 1)
+}
+"#
+    );
+}
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__constructor_that_does_not_exist_does_not_produce_error_for_labelled_args.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__constructor_that_does_not_exist_does_not_produce_error_for_labelled_args.snap
new file mode 100644
index 000000000..ab0ea3c56
--- /dev/null
+++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__constructor_that_does_not_exist_does_not_produce_error_for_labelled_args.snap
@@ -0,0 +1,21 @@
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub fn main() {\n  // We only want to error on `Wibble` since it doesn't exist, we don't want\n  // an error on the label at this point!\n  Wibble(label: 1)\n}\n"
+---
+----- SOURCE CODE
+
+pub fn main() {
+  // We only want to error on `Wibble` since it doesn't exist, we don't want
+  // an error on the label at this point!
+  Wibble(label: 1)
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:5:3
+  │
+5 │   Wibble(label: 1)
+  │   ^^^^^^
+
+The custom type variant constructor `Wibble` is not in scope here.
diff --git a/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__function_that_does_not_exist_does_not_produce_error_for_labelled_args.snap b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__function_that_does_not_exist_does_not_produce_error_for_labelled_args.snap
new file mode 100644
index 000000000..eae907896
--- /dev/null
+++ b/compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__errors__function_that_does_not_exist_does_not_produce_error_for_labelled_args.snap
@@ -0,0 +1,21 @@
+---
+source: compiler-core/src/type_/tests/errors.rs
+expression: "\npub fn main() {\n  // We only want to error on `wibble` since it doesn't exist, we don't want\n  // an error on the label at this point!\n  wibble(label: 1)\n}\n"
+---
+----- SOURCE CODE
+
+pub fn main() {
+  // We only want to error on `wibble` since it doesn't exist, we don't want
+  // an error on the label at this point!
+  wibble(label: 1)
+}
+
+
+----- ERROR
+error: Unknown variable
+  ┌─ /src/one/two.gleam:5:3
+  │
+5 │   wibble(label: 1)
+  │   ^^^^^^
+
+The name `wibble` is not in scope here.