Fix generate function code action for unsupported targets

Author: GearsDatapacks

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+### Bug fixes
+
+- Fixed a bug where the "Generate function" code action would be incorrectly
+  offered when calling a function unsupported by the current target, leading to
+  invalid code if the code action was accepted.
+  ([Surya Rose](https://github.com/GearsDatapacks))
+
 ### Compiler
 
 - Patterns aliasing a string prefix have been optimised to generate faster code

compiler-core/src/ast/typed.rs

@@ -1182,4 +1182,7 @@ pub enum InvalidExpression {
         module_name: EcoString,
         label: EcoString,
     },
+    UnknownVariable {
+        name: EcoString,
+    },
 }

compiler-core/src/language_server/code_action.rs

@@ -5305,15 +5305,13 @@ impl<'a> GenerateFunction<'a> {
 
     fn try_save_function_to_generate(
         &mut self,
-        function_name_location: SrcSpan,
+        name: &'a EcoString,
         function_type: &Arc<Type>,
         given_arguments: Option<&'a [TypedCallArg]>,
     ) {
-        let candidate_name = code_at(self.module, function_name_location);
-        match (candidate_name, function_type.fn_types()) {
-            (_, None) => (),
-            (name, _) if !is_valid_lowercase_name(name) => (),
-            (name, Some((arguments_types, return_type))) => {
+        match function_type.fn_types() {
+            None => {}
+            Some((arguments_types, return_type)) => {
                 self.function_to_generate = Some(FunctionToGenerate {
                     name,
                     arguments_types,
@@ -5366,7 +5364,10 @@ impl<'ast> ast::visit::Visit<'ast> for GenerateFunction<'ast> {
                 Some(InvalidExpression::ModuleSelect { module_name, label }) => {
                     self.try_save_function_from_other_module(module_name, label, type_, None)
                 }
-                None => self.try_save_function_to_generate(*location, type_, None),
+                Some(InvalidExpression::UnknownVariable { name }) => {
+                    self.try_save_function_to_generate(name, type_, None)
+                }
+                None => {}
             }
         }
 
@@ -5404,10 +5405,10 @@ impl<'ast> ast::visit::Visit<'ast> for GenerateFunction<'ast> {
                 }
                 TypedExpr::Invalid {
                     type_,
-                    location,
-                    extra_information: _,
+                    extra_information: Some(InvalidExpression::UnknownVariable { name }),
+                    location: _,
                 } => {
-                    return self.try_save_function_to_generate(*location, type_, Some(arguments));
+                    return self.try_save_function_to_generate(name, type_, Some(arguments));
                 }
                 _ => {}
             }

compiler-core/src/language_server/tests/action.rs

@@ -10727,3 +10727,20 @@ pub fn main() -> Nil {
         find_position_of("function").to_selection()
     );
 }
+
+// https://github.com/gleam-lang/gleam/issues/4904
+#[test]
+fn no_code_action_to_generate_function_on_unsupported_target() {
+    assert_no_code_actions!(
+        GENERATE_FUNCTION,
+        r#"
+pub fn main() {
+  wibble()
+}
+
+@external(javascript, "./ffi.mjs", "wibble")
+fn wibble() -> Nil
+"#,
+        find_position_of("wibble").to_selection()
+    );
+}

compiler-core/src/type_/expression.rs

@@ -713,6 +713,18 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
         }
     }
 
+    fn error_expr_with_information(
+        &mut self,
+        location: SrcSpan,
+        extra_information: Option<InvalidExpression>,
+    ) -> TypedExpr {
+        TypedExpr::Invalid {
+            location,
+            type_: self.new_unbound_var(),
+            extra_information,
+        }
+    }
+
     fn infer_iter_statements<StatementsIter: Iterator<Item = UntypedStatement>>(
         &mut self,
         location: SrcSpan,
@@ -1713,8 +1725,15 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
         match self.infer_or_error(expression) {
             Ok(result) => result,
             Err(error) => {
+                let information = match &error {
+                    Error::UnknownVariable { name, .. } => {
+                        Some(InvalidExpression::UnknownVariable { name: name.clone() })
+                    }
+                    _ => None,
+                };
+
                 self.problems.error(error);
-                self.error_expr(location)
+                self.error_expr_with_information(location, information)
             }
         }
     }
@@ -4542,11 +4561,16 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
             ReferenceRegistration::Register
         };
 
-        match self.infer_var(argument_name, argument_location, references) {
+        match self.infer_var(argument_name.clone(), argument_location, references) {
             Ok(result) => result,
             Err(error) => {
                 self.problems.error(error);
-                self.error_expr(argument_location)
+                self.error_expr_with_information(
+                    argument_location,
+                    Some(InvalidExpression::UnknownVariable {
+                        name: argument_name,
+                    }),
+                )
             }
         }
     }

compiler-core/src/type_/pipe.rs

@@ -55,18 +55,7 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> {
         let end = expressions.last().location().end;
         let mut expressions = expressions.into_iter();
         let first = expressions.next().expect("Empty pipeline in typer");
-        let first_location = first.location();
-        let first = match expr_typer.infer_or_error(first) {
-            Ok(inferred) => inferred,
-            Err(e) => {
-                expr_typer.problems.error(e);
-                TypedExpr::Invalid {
-                    location: first_location,
-                    type_: expr_typer.new_unbound_var(),
-                    extra_information: None,
-                }
-            }
-        };
+        let first = expr_typer.infer(first);
 
         Self::new(expr_typer, size, first, end).infer_expressions(expressions)
     }
@@ -140,20 +129,7 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> {
                     location,
                     ..
                 } => {
-                    let fun = match self.expr_typer.infer_or_error(*fun) {
-                        Ok(fun) => fun,
-                        Err(e) => {
-                            // In case we cannot infer the function we'll
-                            // replace it with an invalid expression with an
-                            // unbound type to keep going!
-                            self.expr_typer.problems.error(e);
-                            TypedExpr::Invalid {
-                                location,
-                                type_: self.expr_typer.new_unbound_var(),
-                                extra_information: None,
-                            }
-                        }
-                    };
+                    let fun = self.expr_typer.infer(*fun);
 
                     match fun.type_().fn_types() {
                         // Rewrite as right(..args)(left)
@@ -352,19 +328,7 @@ impl<'a, 'b, 'c> PipeTyper<'a, 'b, 'c> {
     /// b is the `function` argument.
     fn infer_apply_pipe(&mut self, function: UntypedExpr) -> TypedExpr {
         let function_location = function.location();
-        let function = Box::new(match self.expr_typer.infer_or_error(function) {
-            Ok(function) => function,
-            Err(error) => {
-                // If we cannot infer the function we put an invalid expression
-                // in its place so we can still keep going with the other steps.
-                self.expr_typer.problems.error(error);
-                TypedExpr::Invalid {
-                    location: function_location,
-                    type_: self.expr_typer.new_unbound_var(),
-                    extra_information: None,
-                }
-            }
-        });
+        let function = Box::new(self.expr_typer.infer(function));
 
         self.expr_typer.purity = self
             .expr_typer