Don't make parameters of variables defined within the scope of the extracted function

Author: GearsDatapacks

compiler-core/src/language_server/code_action.rs

@@ -8505,7 +8505,7 @@ impl<'a> ExtractFunction<'a> {
 
         let location = SrcSpan::new(first.location().start, last.location().end);
 
-        let referenced_variables = referenced_variables_for_statements(&statements);
+        let referenced_variables = referenced_variables_for_statements(&statements, location);
 
         let code = code_at(self.module, location);
 
@@ -8582,15 +8582,16 @@ impl<'ast> ast::visit::Visit<'ast> for ExtractFunction<'ast> {
 }
 
 fn referenced_variables(expression: &TypedExpr) -> Vec<(EcoString, Arc<Type>)> {
-    let mut references = ReferencedVariables::new();
+    let mut references = ReferencedVariables::new(expression.location());
     references.visit_typed_expr(expression);
     references.variables
 }
 
 fn referenced_variables_for_statements(
     statements: &[&TypedStatement],
+    location: SrcSpan,
 ) -> Vec<(EcoString, Arc<Type>)> {
-    let mut references = ReferencedVariables::new();
+    let mut references = ReferencedVariables::new(location);
     for statement in statements {
         references.visit_typed_statement(*statement);
     }
@@ -8599,19 +8600,19 @@ fn referenced_variables_for_statements(
 
 struct ReferencedVariables {
     variables: Vec<(EcoString, Arc<Type>)>,
-    defined_variables: HashSet<EcoString>,
+    location: SrcSpan,
 }
 
 impl ReferencedVariables {
-    fn new() -> Self {
+    fn new(location: SrcSpan) -> Self {
         Self {
             variables: Vec::new(),
-            defined_variables: HashSet::new(),
+            location,
         }
     }
 
-    fn register(&mut self, name: &EcoString, type_: &Arc<Type>) {
-        if self.defined_variables.contains(name) {
+    fn register(&mut self, name: &EcoString, type_: &Arc<Type>, definition_location: SrcSpan) {
+        if self.location.contains_span(definition_location) {
             return;
         }
 
@@ -8633,8 +8634,8 @@ impl<'ast> ast::visit::Visit<'ast> for ReferencedVariables {
         name: &'ast EcoString,
     ) {
         match &constructor.variant {
-            type_::ValueConstructorVariant::LocalVariable { .. } => {
-                self.register(name, &constructor.type_);
+            type_::ValueConstructorVariant::LocalVariable { location, .. } => {
+                self.register(name, &constructor.type_, *location);
             }
             type_::ValueConstructorVariant::ModuleConstant { .. }
             | type_::ValueConstructorVariant::LocalConstant { .. }

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

@@ -10264,3 +10264,38 @@ pub fn do_things(a, b) {
         find_position_of("let").select_until(find_position_of("* b\n").under_char('\n'))
     );
 }
+
+#[test]
+fn extract_function_which_use_variables_defined_in_the_extracted_span() {
+    assert_code_action!(
+        EXTRACT_FUNCTION,
+        "
+pub fn do_things(a, b) {
+  let new_a = 10 + a
+  let new_b = 10 + b
+  let result = new_a * new_b
+  result + 3
+}
+",
+        find_position_of("let").select_until(find_position_of("* new_b\n").under_char('\n'))
+    );
+}
+
+#[test]
+fn extract_function_which_use_variables_shadowed_in_an_inner_scope() {
+    assert_code_action!(
+        EXTRACT_FUNCTION,
+        "
+pub fn do_things(a, b) {
+  let first_part = {
+    let a = a + 10
+    let b = b + 10
+    a * b
+  }
+  let result = first_part + a * b
+  result + 3
+}
+",
+        find_position_of("let").select_until(find_position_of("+ a * b\n").under_char('\n'))
+    );
+}

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

@@ -0,0 +1,29 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "\npub fn do_things(a, b) {\n  let new_a = 10 + a\n  let new_b = 10 + b\n  let result = new_a * new_b\n  result + 3\n}\n"
+---
+----- BEFORE ACTION
+
+pub fn do_things(a, b) {
+  let new_a = 10 + a
+  ▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+  let new_b = 10 + b
+▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+  let result = new_a * new_b
+▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+  result + 3
+}
+
+
+----- AFTER ACTION
+
+pub fn do_things(a, b) {
+  function(a, b)
+  result + 3
+}
+
+fn function(a: Int, b: Int) -> Int {
+  let new_a = 10 + a
+  let new_b = 10 + b
+  let result = new_a * new_b
+}

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

@@ -0,0 +1,38 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "\npub fn do_things(a, b) {\n  let first_part = {\n    let a = a + 10\n    let b = b + 10\n    a * b\n  }\n  let result = first_part + a * b\n  result + 3\n}\n"
+---
+----- BEFORE ACTION
+
+pub fn do_things(a, b) {
+  let first_part = {
+  ▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+    let a = a + 10
+▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+    let b = b + 10
+▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+    a * b
+▔▔▔▔▔▔▔▔▔
+  }
+▔▔▔
+  let result = first_part + a * b
+▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔▔
+  result + 3
+}
+
+
+----- AFTER ACTION
+
+pub fn do_things(a, b) {
+  function(a, b)
+  result + 3
+}
+
+fn function(a: Int, b: Int) -> Int {
+  let first_part = {
+    let a = a + 10
+    let b = b + 10
+    a * b
+  }
+  let result = first_part + a * b
+}