Move imports to top of module

compiler-core/src/language_server/code_action/move_imports_to_top.rs

@@ -1,12 +1,13 @@
 use crate::{
-    ast::{self, visit::Visit as _},
+    ast::{self, visit::Visit as _, Import, SrcSpan},
     build,
     line_numbers::LineNumbers,
 };
 use ecow::EcoString;
+use itertools::Itertools;
 use lsp_types::{CodeAction, CodeActionKind, CodeActionParams, Position, Range, TextEdit};
 
-use super::{super::src_span_to_lsp_range, CodeActionBuilder};
+use super::{super::engine::overlaps, super::src_span_to_lsp_range, CodeActionBuilder};
 
 #[derive(Debug)]
 /// Code action to move all the imports to the top of the module.
@@ -16,51 +17,35 @@ pub struct MoveImportsToTop<'a> {
     params: &'a CodeActionParams,
     module: &'a ast::TypedModule,
 
-    past_top_imports: bool,
-    edits: Vec<TextEdit>,
+    is_over_import: bool,
+    imports: Vec<&'a Import<EcoString>>,
+    earlier_definition_end: Option<u32>,
 }
 
-impl<'ast> ast::visit::Visit<'ast> for MoveImportsToTop<'_> {
+impl<'ast> ast::visit::Visit<'ast> for MoveImportsToTop<'ast> {
     fn visit_typed_definition(&mut self, def: &'ast ast::TypedDefinition) {
         match def {
-            ast::Definition::Import(import) if self.past_top_imports => {
-                tracing::info!("found not top level import");
-                self.edits.push(TextEdit {
-                    range: src_span_to_lsp_range(import.location, &self.line_numbers),
-                    new_text: "".into(),
-                });
-
-                let import_start = import.location.start as usize;
-                let import_end = import.location.end as usize;
-                let import_text = self
-                    .code
-                    .get(import_start..import_end)
-                    .expect("import")
-                    .into();
-
-                self.edits.push(TextEdit {
-                    range: Range {
-                        start: Position {
-                            line: 0,
-                            character: 0,
-                        },
-                        end: Position {
-                            line: 0,
-                            character: 0,
-                        },
-                    },
-                    new_text: import_text,
-                });
-            }
-            ast::Definition::Import(import_) => {
-                tracing::info!("found top level import", import_);
+            ast::Definition::Import(import) => {
+                if overlaps(
+                    self.params.range,
+                    src_span_to_lsp_range(import.location, &self.line_numbers),
+                ) {
+                    self.is_over_import = true;
+                }
+                self.imports.push(import)
             }
             ast::Definition::Function(_)
             | ast::Definition::TypeAlias(_)
             | ast::Definition::CustomType(_)
             | ast::Definition::ModuleConstant(_) => {
-                tracing::info!("found definition after top level imports");
-                self.past_top_imports = true
+                let def_location = def.location();
+                match self.earlier_definition_end {
+                    Some(end) if def_location.end < end => {
+                        self.earlier_definition_end = Some(def_location.end)
+                    }
+                    Some(_) => (),
+                    None => self.earlier_definition_end = Some(def_location.end),
+                }
             }
         }
     }
@@ -73,25 +58,75 @@ impl<'a> MoveImportsToTop<'a> {
             code: &module.code,
             params,
             module: &module.ast,
-            past_top_imports: false,
-            edits: vec![],
+            imports: vec![],
+            is_over_import: false,
+            earlier_definition_end: None,
         }
     }
 
     pub fn code_actions(mut self) -> Vec<CodeAction> {
         self.visit_typed_module(self.module);
-        if self.edits.is_empty() {
+
+        if self.imports.is_empty() || !self.is_over_import {
+            return vec![];
+        }
+
+        let Some(end) = self.earlier_definition_end else {
             return vec![];
+        };
+
+        let edits = self
+            .imports
+            .iter()
+            .filter(|import| import.location.start > end)
+            .flat_map(|import| self.move_import_edits(import))
+            .collect_vec();
+
+        if edits.is_empty() {
+            vec![]
+        } else {
+            vec![
+                CodeActionBuilder::new("Move all imports to the top of the module")
+                    .kind(CodeActionKind::REFACTOR_REWRITE)
+                    .changes(self.params.text_document.uri.clone(), edits)
+                    .preferred(false)
+                    .build(),
+            ]
         }
+    }
 
-        self.edits.sort_by_key(|edit| edit.range.start);
+    fn move_import_edits(&self, import: &Import<EcoString>) -> Vec<TextEdit> {
+        let import_text = self
+            .code
+            .get(import.location.start as usize..import.location.end as usize)
+            .expect("import location");
 
         vec![
-            CodeActionBuilder::new("Move all imports to the top of the module")
-                .kind(CodeActionKind::SOURCE_ORGANIZE_IMPORTS)
-                .changes(self.params.text_document.uri.clone(), self.edits)
-                .preferred(true)
-                .build(),
+            TextEdit {
+                range: src_span_to_lsp_range(
+                    SrcSpan {
+                        start: import.location.start,
+                        // This way we will take care of any eventual new line
+                        // after the import.
+                        end: import.location.end + 1,
+                    },
+                    &self.line_numbers,
+                ),
+                new_text: "".into(),
+            },
+            TextEdit {
+                range: Range {
+                    start: Position {
+                        line: 0,
+                        character: 0,
+                    },
+                    end: Position {
+                        line: 0,
+                        character: 0,
+                    },
+                },
+                new_text: format!("{import_text}\n"),
+            },
         ]
     }
 }

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

@@ -53,6 +53,7 @@ fn engine_response(src: &str, line: u32) -> engine::Response<Option<Vec<lsp_type
 const REMOVE_UNUSED_IMPORTS: &str = "Remove unused imports";
 const REMOVE_REDUNDANT_TUPLES: &str = "Remove redundant tuples";
 const CONVERT_TO_CASE: &str = "Convert to case";
+const MOVE_IMPORTS_UP: &str = "Move all imports to the top of the module";
 
 fn apply_first_code_action_with_title(src: &str, line: u32, title: &str) -> String {
     let response = engine_response(src, line)
@@ -102,11 +103,22 @@ fn apply_code_edit(
             panic!("Unknown url {}", change_url)
         }
         for edit in change {
-            let start = line_numbers.byte_index(edit.range.start.line, edit.range.start.character)
-                as i32
-                - offset;
-            let end = line_numbers.byte_index(edit.range.end.line, edit.range.end.character) as i32
-                - offset;
+            let edit_start =
+                line_numbers.byte_index(edit.range.start.line, edit.range.start.character) as i32;
+            let start = if offset > edit_start {
+                0
+            } else {
+                edit_start - offset
+            };
+
+            let edit_end =
+                line_numbers.byte_index(edit.range.end.line, edit.range.end.character) as i32;
+            let end = if offset > edit_end {
+                0
+            } else {
+                edit_end - offset
+            };
+
             let range = (start as usize)..(end as usize);
             offset += end - start;
             offset -= edit.new_text.len() as i32;
@@ -751,6 +763,69 @@ fn test_convert_outer_let_assert_to_case() {
     ));
 }
 
+#[test]
+fn move_imports_to_the_top_of_the_module_does_not_pop_up_if_no_imports_can_be_moved() {
+    let code = r#"import error
+pub fn main() {
+  error.is_ok()
+}"#;
+
+    assert!(engine_response(code, 0)
+        .result
+        .expect("ok response")
+        .is_none());
+}
+
+#[test]
+fn move_imports_to_the_top_of_the_module_does_not_pop_up_if_not_over_an_import() {
+    let code = r#"pub fn main() {}
+
+const wibble = 1
+
+import list
+"#;
+
+    assert!(engine_response(code, 0)
+        .result
+        .expect("ok response")
+        .is_none());
+
+    assert!(engine_response(code, 1)
+        .result
+        .expect("ok response")
+        .is_none());
+
+    assert!(engine_response(code, 2)
+        .result
+        .expect("ok response")
+        .is_none());
+}
+
+#[test]
+fn move_imports_to_the_top_of_the_module_moves_all_imports_when_on_import_below_a_definition_1() {
+    let code = r#"import list
+
+pub fn main() {}
+
+import result
+import map
+"#;
+
+    insta::assert_snapshot!(apply_first_code_action_with_title(code, 4, MOVE_IMPORTS_UP));
+}
+
+#[test]
+fn move_imports_to_the_top_of_the_module_moves_all_imports_when_on_import_below_a_definition_2() {
+    let code = r#"import list
+
+pub fn main() {}
+
+import result
+"#;
+
+    insta::assert_snapshot!(apply_first_code_action_with_title(code, 5, MOVE_IMPORTS_UP));
+}
+
 /* TODO: implement qualified unused location
 #[test]
 fn test_remove_unused_qualified_action() {

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

@@ -0,0 +1,9 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "apply_first_code_action_with_title(code, 4, MOVE_IMPORTS_UP)"
+---
+import map
+import result
+import list
+
+pub fn main() {}

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

@@ -0,0 +1,8 @@
+---
+source: compiler-core/src/language_server/tests/action.rs
+expression: "apply_first_code_action_with_title(code, 5, MOVE_IMPORTS_UP)"
+---
+import result
+import list
+
+pub fn main() {}