make sure to remove all echos

Author: giacomocavalieri

compiler-core/src/language_server/code_action.rs

@@ -5467,7 +5467,8 @@ pub struct RemoveEcho<'a> {
     module: &'a Module,
     params: &'a CodeActionParams,
     edits: TextEdits<'a>,
-    echo_span_to_delete: Option<SrcSpan>,
+    is_hovering_echo: bool,
+    echo_spans_to_delete: Vec<SrcSpan>,
     // We need to keep a reference to the two latest pipeline assignments we
     // run into to properly delete an echo that's inside a pipeline.
     latest_pipe_step: Option<SrcSpan>,
@@ -5484,7 +5485,8 @@ impl<'a> RemoveEcho<'a> {
             module,
             params,
             edits: TextEdits::new(line_numbers),
-            echo_span_to_delete: None,
+            is_hovering_echo: false,
+            echo_spans_to_delete: vec![],
             latest_pipe_step: None,
             second_to_latest_pipe_step: None,
         }
@@ -5493,14 +5495,18 @@ impl<'a> RemoveEcho<'a> {
     pub fn code_actions(mut self) -> Vec<CodeAction> {
         self.visit_typed_module(&self.module.ast);
 
-        let Some(echo_span_to_delete) = self.echo_span_to_delete else {
+        // We only want to trigger the action if we're over one of the echos in
+        // the module
+        if !self.is_hovering_echo {
             return vec![];
         };
 
-        self.edits.delete(echo_span_to_delete);
+        for span in self.echo_spans_to_delete {
+            self.edits.delete(span);
+        }
 
         let mut action = Vec::with_capacity(1);
-        CodeActionBuilder::new("Remove `echo`")
+        CodeActionBuilder::new("Remove all `echo`s from this module")
             .kind(CodeActionKind::REFACTOR_REWRITE)
             .changes(self.params.text_document.uri.clone(), self.edits.edits)
             .preferred(false)
@@ -5527,15 +5533,15 @@ impl<'ast> ast::visit::Visit<'ast> for RemoveEcho<'ast> {
         // ```
         //
         let echo_range = self.edits.src_span_to_lsp_range(*location);
-        if !within(self.params.range, echo_range) {
-            return;
+        if within(self.params.range, echo_range) {
+            self.is_hovering_echo = true;
         }
 
         if let Some(expression) = expression {
             // If there's an expression we delete everything we find until its
             // start (excluded).
             let span_to_delete = SrcSpan::new(location.start, expression.location().start);
-            self.echo_span_to_delete = Some(span_to_delete);
+            self.echo_spans_to_delete.push(span_to_delete);
         } else {
             // Othwerise we know we're inside a pipeline, we take the closest step
             // that is not echo itself and delete everything from its end until the
@@ -5551,7 +5557,7 @@ impl<'ast> ast::visit::Visit<'ast> for RemoveEcho<'ast> {
                 .or(self.second_to_latest_pipe_step);
             if let Some(step_preceding_echo) = step_preceding_echo {
                 let span_to_delete = SrcSpan::new(step_preceding_echo.end, location.start + 4);
-                self.echo_span_to_delete = Some(span_to_delete);
+                self.echo_spans_to_delete.push(span_to_delete);
             }
         }
 

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

@@ -80,7 +80,7 @@ const INLINE_VARIABLE: &str = "Inline variable";
 const CONVERT_TO_PIPE: &str = "Convert to pipe";
 const INTERPOLATE_STRING: &str = "Interpolate string";
 const FILL_UNUSED_FIELDS: &str = "Fill unused fields";
-const REMOVE_ECHO: &str = "Remove `echo`";
+const REMOVE_ALL_ECHOS_FROM_THIS_MODULE: &str = "Remove all `echo`s from this module";
 
 macro_rules! assert_code_action {
     ($title:expr, $code:literal, $range:expr $(,)?) => {
@@ -204,7 +204,7 @@ pub fn main() {
 #[test]
 fn remove_echo() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   echo 1 + 2
 }",
@@ -215,7 +215,7 @@ fn remove_echo() {
 #[test]
 fn remove_echo_selecting_expression() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   echo 1 + 2
 }",
@@ -226,7 +226,7 @@ fn remove_echo_selecting_expression() {
 #[test]
 fn remove_echo_as_function_arg() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   wibble([], echo 1 + 2)
 }",
@@ -237,7 +237,7 @@ fn remove_echo_as_function_arg() {
 #[test]
 fn remove_echo_in_pipeline_step() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   [1, 2, 3]
   |> echo
@@ -250,7 +250,7 @@ fn remove_echo_in_pipeline_step() {
 #[test]
 fn remove_echo_in_single_line_pipeline_step() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   [1, 2, 3] |> echo |> wibble
 }",
@@ -261,7 +261,7 @@ fn remove_echo_in_single_line_pipeline_step() {
 #[test]
 fn remove_echo_last_in_long_pipeline_step() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   [1, 2, 3]
   |> wibble
@@ -274,7 +274,7 @@ fn remove_echo_last_in_long_pipeline_step() {
 #[test]
 fn remove_echo_last_in_short_pipeline_step() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   [1, 2, 3]
   |> echo
@@ -286,7 +286,7 @@ fn remove_echo_last_in_short_pipeline_step() {
 #[test]
 fn remove_echo_before_pipeline() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   echo [1, 2, 3] |> wibble
 }",
@@ -297,7 +297,7 @@ fn remove_echo_before_pipeline() {
 #[test]
 fn remove_echo_before_pipeline_selecting_step() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   echo [1, 2, 3] |> wibble
 }",
@@ -306,16 +306,29 @@ fn remove_echo_before_pipeline_selecting_step() {
 }
 
 #[test]
-fn remove_echo_removes_closest_one() {
+fn remove_echo_removes_all_echos() {
     assert_code_action!(
-        REMOVE_ECHO,
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
         "pub fn main() {
   echo wibble(echo 1, 2)
 }",
         find_position_of("echo").nth_occurrence(2).to_selection()
     );
 }
 
+#[test]
+fn remove_echo_removes_all_echos_1() {
+    assert_code_action!(
+        REMOVE_ALL_ECHOS_FROM_THIS_MODULE,
+        "pub fn main() {
+  echo 1 |> echo |> echo |> wibble |> echo
+  echo wibble(echo 1, echo 2)
+  echo 1
+}",
+        find_position_of("echo").nth_occurrence(2).to_selection()
+    );
+}
+
 #[test]
 fn split_string() {
     assert_code_action!(

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

@@ -0,0 +1,15 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "pub fn main() {\n  echo wibble(echo 1, 2)\n}"
+---
+----- BEFORE ACTION
+pub fn main() {
+  echo wibble(echo 1, 2)
+              ↑         
+}
+
+
+----- AFTER ACTION
+pub fn main() {
+  wibble(1, 2)
+}

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

@@ -0,0 +1,19 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "pub fn main() {\n  echo 1 |> echo |> echo |> wibble |> echo\n  echo wibble(echo 1, echo 2)\n  echo 1\n}"
+---
+----- BEFORE ACTION
+pub fn main() {
+  echo 1 |> echo |> echo |> wibble |> echo
+            ↑                             
+  echo wibble(echo 1, echo 2)
+  echo 1
+}
+
+
+----- AFTER ACTION
+pub fn main() {
+  1 |> wibble
+  wibble(1, 2)
+  1
+}