From 03395ed41768972d6763cba428b536d1d35d6fa0 Mon Sep 17 00:00:00 2001
From: Giacomo Cavalieri <giacomo.cavalieri@icloud.com>
Date: Sat, 21 Dec 2024 10:58:32 +0100
Subject: [PATCH] Fix bug extracting module selects

---
 .../src/language_server/code_action.rs        | 26 +++++++------------
 .../src/language_server/tests/action.rs       | 15 +++++++++++
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/compiler-core/src/language_server/code_action.rs b/compiler-core/src/language_server/code_action.rs
index 512bfa44f60..9e07b10492a 100644
--- a/compiler-core/src/language_server/code_action.rs
+++ b/compiler-core/src/language_server/code_action.rs
@@ -2711,8 +2711,16 @@ impl<'ast> ast::visit::Visit<'ast> for ExtractVariable<'ast> {
         if self.position != Some(ExtractVariablePosition::TopLevelStatement)
             && within(self.params.range, expr_range)
         {
-            self.selected_expression = Some(expr_location);
-            self.statement_before_selected_expression = self.latest_statement;
+            match expr {
+                // We don't extract variables, they're already good.
+                // And we don't extract module selects by themselves but always
+                // want to consider those as part of a function call.
+                TypedExpr::Var { .. } | TypedExpr::ModuleSelect { .. } => (),
+                _ => {
+                    self.selected_expression = Some(expr_location);
+                    self.statement_before_selected_expression = self.latest_statement;
+                }
+            }
         }
 
         let previous_position = self.position;
@@ -2744,20 +2752,6 @@ impl<'ast> ast::visit::Visit<'ast> for ExtractVariable<'ast> {
         self.position = previous_position;
     }
 
-    // We don't want to offer the action if the cursor is over a variable
-    // already!
-    fn visit_typed_expr_var(
-        &mut self,
-        location: &'ast SrcSpan,
-        _constructor: &'ast type_::ValueConstructor,
-        _name: &'ast EcoString,
-    ) {
-        let var_range = self.edits.src_span_to_lsp_range(*location);
-        if within(self.params.range, var_range) {
-            self.selected_expression = None;
-        }
-    }
-
     // We don't want to offer the action if the cursor is over some invalid
     // piece of code.
     fn visit_typed_expr_invalid(&mut self, location: &'ast SrcSpan, _type_: &'ast Arc<Type>) {
diff --git a/compiler-core/src/language_server/tests/action.rs b/compiler-core/src/language_server/tests/action.rs
index 0dd082402af..b5fd5f464ac 100644
--- a/compiler-core/src/language_server/tests/action.rs
+++ b/compiler-core/src/language_server/tests/action.rs
@@ -3985,6 +3985,21 @@ fn do_not_extract_top_level_expression_in_let_statement() {
     );
 }
 
+#[test]
+fn do_not_extract_top_level_module_call() {
+    let src = r#"
+import list
+pub fn main() {
+  list.map([1, 2, 3], todo)
+}"#;
+
+    assert_no_code_actions!(
+        EXTRACT_VARIABLE,
+        TestProject::for_source(src).add_module("list", "pub fn map(l, f) { todo }"),
+        find_position_of("map").to_selection()
+    );
+}
+
 #[test]
 fn expand_function_capture() {
     assert_code_action!(