Fix bugs with "Inline variable" code action

CHANGELOG.md

@@ -16,6 +16,14 @@
   for record fields in custom type definitions.
   ([cysabi](https://github.com/cysabi))
 
+- Fixed a bug where the "Inline variable" code action would be offered for
+  function parameters and other invalid cases.
+  ([Surya Rose](https://github.com/GearsDatapacks))
+
+- Fixed a bug where the "Inline variable" code action would not be applied
+  correctly to variables using label shorthand syntax.
+  ([Surya Rose](https://github.com/GearsDatapacks))
+
 ## v1.11.1 - 2025-06-05
 
 ### Compiler

compiler-core/src/ast/tests.rs

@@ -6,7 +6,7 @@ use crate::analyse::TargetSupport;
 use crate::build::{ExpressionPosition, Origin, Target};
 use crate::config::PackageConfig;
 use crate::line_numbers::LineNumbers;
-use crate::type_::error::VariableOrigin;
+use crate::type_::error::{VariableDeclaration, VariableOrigin, VariableSyntax};
 use crate::type_::expression::{FunctionDefinition, Purity};
 use crate::type_::{Deprecation, PRELUDE_MODULE_NAME, Problems};
 use crate::warning::WarningEmitter;
@@ -327,7 +327,10 @@ wibble}"#,
             publicity: Publicity::Private,
             variant: ValueConstructorVariant::LocalVariable {
                 location: SrcSpan { start: 5, end: 11 },
-                origin: VariableOrigin::Variable("wibble".into()),
+                origin: VariableOrigin {
+                    syntax: VariableSyntax::Variable("wibble".into()),
+                    declaration: VariableDeclaration::LetPattern,
+                },
             },
             type_: type_::int(),
         },

compiler-core/src/language_server/code_action.rs

@@ -19,7 +19,7 @@ use crate::{
     strings::to_snake_case,
     type_::{
         self, FieldMap, ModuleValueConstructor, Type, TypeVar, TypedCallArg, ValueConstructor,
-        error::{ModuleSuggestion, VariableOrigin},
+        error::{ModuleSuggestion, VariableDeclaration, VariableOrigin},
         printer::{Names, Printer},
     },
 };
@@ -35,7 +35,7 @@ use super::{
     edits::{add_newlines_after_import, get_import_edit, position_of_first_definition_if_import},
     engine::{overlaps, within},
     files::FileSystemProxy,
-    reference::find_variable_references,
+    reference::{VariableReferenceKind, find_variable_references},
     src_span_to_lsp_range, url_from_path,
 };
 
@@ -5883,11 +5883,10 @@ impl<'a> InlineVariable<'a> {
     }
 
     fn maybe_inline(&mut self, location: SrcSpan) {
-        let variable_location =
-            match find_variable_references(&self.module.ast, location).as_slice() {
-                [only_reference] => only_reference.location,
-                _ => return,
-            };
+        let reference = match find_variable_references(&self.module.ast, location).as_slice() {
+            [only_reference] => *only_reference,
+            _ => return,
+        };
 
         let Some(ast::Statement::Assignment(assignment)) =
             self.module.ast.find_statement(location.start)
@@ -5919,7 +5918,15 @@ impl<'a> InlineVariable<'a> {
             .get(value_location.start as usize..value_location.end as usize)
             .expect("Span is valid");
 
-        self.edits.replace(variable_location, value.into());
+        match reference.kind {
+            VariableReferenceKind::Variable => {
+                self.edits.replace(reference.location, value.into());
+            }
+            VariableReferenceKind::LabelShorthand => {
+                self.edits
+                    .insert(reference.location.end, format!(" {value}"));
+            }
+        }
 
         let mut location = assignment.location;
 
@@ -5961,13 +5968,14 @@ impl<'ast> ast::visit::Visit<'ast> for InlineVariable<'ast> {
             return;
         };
 
-        // We can only inline it if it comes from a regular variable pattern,
-        // not a complex pattern or generated assignment.
-        match origin {
-            VariableOrigin::Variable(_) => {}
-            VariableOrigin::LabelShorthand(_)
-            | VariableOrigin::AssignmentPattern
-            | VariableOrigin::Generated => return,
+        // We can only inline variables assigned by `let` statements, as it
+        //doesn't make sense to do so with any other kind of variable.
+        match origin.declaration {
+            VariableDeclaration::LetPattern => {}
+            VariableDeclaration::UsePattern
+            | VariableDeclaration::ClausePattern
+            | VariableDeclaration::FunctionParameter
+            | VariableDeclaration::Generated => return,
         }
 
         self.maybe_inline(*location);
@@ -5980,12 +5988,14 @@ impl<'ast> ast::visit::Visit<'ast> for InlineVariable<'ast> {
         _type: &'ast Arc<Type>,
         origin: &'ast VariableOrigin,
     ) {
-        // We can only inline it if it is a regular variable pattern
-        match origin {
-            VariableOrigin::Variable(_) => {}
-            VariableOrigin::LabelShorthand(_)
-            | VariableOrigin::AssignmentPattern
-            | VariableOrigin::Generated => return,
+        // We can only inline variables assigned by `let` statements, as it
+        //doesn't make sense to do so with any other kind of variable.
+        match origin.declaration {
+            VariableDeclaration::LetPattern => {}
+            VariableDeclaration::UsePattern
+            | VariableDeclaration::ClausePattern
+            | VariableDeclaration::FunctionParameter
+            | VariableDeclaration::Generated => return,
         }
 
         let range = self.edits.src_span_to_lsp_range(*location);

compiler-core/src/language_server/engine.rs

@@ -19,7 +19,7 @@ use crate::{
     type_::{
         self, Deprecation, ModuleInterface, Type, TypeConstructor, ValueConstructor,
         ValueConstructorVariant,
-        error::{Named, VariableOrigin},
+        error::{Named, VariableSyntax},
         printer::Printer,
     },
 };
@@ -637,12 +637,12 @@ where
             Ok(match reference_for_ast_node(found, &current_module.name) {
                 Some(Referenced::LocalVariable {
                     location, origin, ..
-                }) if location.contains(byte_index) => match origin {
-                    Some(VariableOrigin::Generated) => None,
+                }) if location.contains(byte_index) => match origin.map(|origin| origin.syntax) {
+                    Some(VariableSyntax::Generated) => None,
                     Some(
-                        VariableOrigin::Variable(_)
-                        | VariableOrigin::AssignmentPattern
-                        | VariableOrigin::LabelShorthand(_),
+                        VariableSyntax::Variable { .. }
+                        | VariableSyntax::AssignmentPattern
+                        | VariableSyntax::LabelShorthand(_),
                     )
                     | None => success_response(location),
                 },
@@ -695,12 +695,14 @@ where
                     definition_location,
                     ..
                 }) => {
-                    let rename_kind = match origin {
-                        Some(VariableOrigin::Generated) => return Ok(None),
-                        Some(VariableOrigin::LabelShorthand(_)) => {
+                    let rename_kind = match origin.map(|origin| origin.syntax) {
+                        Some(VariableSyntax::Generated) => return Ok(None),
+                        Some(VariableSyntax::LabelShorthand(_)) => {
                             VariableReferenceKind::LabelShorthand
                         }
-                        Some(VariableOrigin::AssignmentPattern | VariableOrigin::Variable(_))
+                        Some(
+                            VariableSyntax::AssignmentPattern | VariableSyntax::Variable { .. },
+                        )
                         | None => VariableReferenceKind::Variable,
                     };
                     rename_local_variable(module, &lines, &params, definition_location, rename_kind)
@@ -772,12 +774,12 @@ where
                     origin,
                     definition_location,
                     location,
-                }) if location.contains(byte_index) => match origin {
-                    Some(VariableOrigin::Generated) => None,
+                }) if location.contains(byte_index) => match origin.map(|origin| origin.syntax) {
+                    Some(VariableSyntax::Generated) => None,
                     Some(
-                        VariableOrigin::LabelShorthand(_)
-                        | VariableOrigin::AssignmentPattern
-                        | VariableOrigin::Variable(_),
+                        VariableSyntax::LabelShorthand(_)
+                        | VariableSyntax::AssignmentPattern
+                        | VariableSyntax::Variable { .. },
                     )
                     | None => {
                         let variable_references =

compiler-core/src/language_server/reference.rs

@@ -329,11 +329,13 @@ fn find_references_in_module(
     }
 }
 
+#[derive(Debug, Clone, Copy)]
 pub struct VariableReference {
     pub location: SrcSpan,
     pub kind: VariableReferenceKind,
 }
 
+#[derive(Debug, Clone, Copy)]
 pub enum VariableReferenceKind {
     Variable,
     LabelShorthand,

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

@@ -8661,3 +8661,80 @@ pub fn main() {
         find_position_of("add").to_selection()
     );
 }
+
+// https://github.com/gleam-lang/gleam/issues/4660#issuecomment-2932371619
+#[test]
+fn inline_variable_label_shorthand() {
+    assert_code_action!(
+        INLINE_VARIABLE,
+        "
+pub type Example {
+  Example(sum: Int, nil: Nil)
+}
+
+pub fn main() {
+  let sum = 1 + 1
+
+  Example(Nil, sum:)
+}
+",
+        find_position_of("sum = ").to_selection()
+    );
+}
+
+// https://github.com/gleam-lang/gleam/issues/4660
+#[test]
+fn no_inline_variable_action_for_parameter() {
+    assert_no_code_actions!(
+        INLINE_VARIABLE,
+        "
+pub fn main() {
+  let x = fn(something) {
+    something
+  }
+
+  x
+}
+",
+        find_position_of("something")
+            .nth_occurrence(2)
+            .to_selection()
+    );
+}
+
+#[test]
+fn no_inline_variable_action_for_use_pattern() {
+    assert_no_code_actions!(
+        INLINE_VARIABLE,
+        "
+pub fn main() {
+  let x = {
+    use something <- todo
+    something
+  }
+
+  x
+}
+",
+        find_position_of("something").to_selection()
+    );
+}
+
+#[test]
+fn no_inline_variable_action_for_case_pattern() {
+    assert_no_code_actions!(
+        INLINE_VARIABLE,
+        "
+pub fn main() {
+  let x = case todo {
+    something -> something
+  }
+
+  x
+}
+",
+        find_position_of("something")
+            .nth_occurrence(2)
+            .to_selection()
+    );
+}

compiler-core/src/language_server/tests/snapshots/gleam_core__language_server__tests__action__inline_variable_label_shorthand.snap

@@ -0,0 +1,27 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "\npub type Example {\n  Example(sum: Int, nil: Nil)\n}\n\npub fn main() {\n  let sum = 1 + 1\n\n  Example(Nil, sum:)\n}\n"
+---
+----- BEFORE ACTION
+
+pub type Example {
+  Example(sum: Int, nil: Nil)
+}
+
+pub fn main() {
+  let sum = 1 + 1
+      ↑          
+
+  Example(Nil, sum:)
+}
+
+
+----- AFTER ACTION
+
+pub type Example {
+  Example(sum: Int, nil: Nil)
+}
+
+pub fn main() {
+  Example(Nil, sum: 1 + 1)
+}