Address code review

Author: lionel-

crates/ark/src/lsp/ark_file.rs

@@ -90,21 +90,15 @@ impl ArkFile {
         db: &dyn ArkDb,
         point: tree_sitter::Point,
     ) -> anyhow::Result<lsp_types::Position> {
-        let line_col = biome_line_index::LineCol {
-            line: point.row as u32,
-            col: point.column as u32,
-        };
-        to_proto::position_from_line_col(line_col, self.line_index(db), self.encoding)
+        lsp_position_from_tree_sitter_point(point, self.line_index(db), self.encoding)
     }
 
     pub(crate) fn lsp_range_from_tree_sitter_range(
         &self,
         db: &dyn ArkDb,
         range: tree_sitter::Range,
     ) -> anyhow::Result<lsp_types::Range> {
-        let start = self.lsp_position_from_tree_sitter_point(db, range.start_point)?;
-        let end = self.lsp_position_from_tree_sitter_point(db, range.end_point)?;
-        Ok(lsp_types::Range::new(start, end))
+        lsp_range_from_tree_sitter_range(range, self.line_index(db), self.encoding)
     }
 
     pub(crate) fn tree_sitter_range_from_lsp_range(
@@ -126,6 +120,31 @@ impl ArkFile {
     }
 }
 
+/// Free functions over `LineIndex` + `PositionEncoding`, so anything holding
+/// those two (an `ArkFile` plus its `db`, or a `DocumentContext`) can convert
+/// without each carrying its own copy of the logic.
+pub(crate) fn lsp_position_from_tree_sitter_point(
+    point: tree_sitter::Point,
+    line_index: &biome_line_index::LineIndex,
+    encoding: PositionEncoding,
+) -> anyhow::Result<lsp_types::Position> {
+    let line_col = biome_line_index::LineCol {
+        line: point.row as u32,
+        col: point.column as u32,
+    };
+    to_proto::position_from_line_col(line_col, line_index, encoding)
+}
+
+pub(crate) fn lsp_range_from_tree_sitter_range(
+    range: tree_sitter::Range,
+    line_index: &biome_line_index::LineIndex,
+    encoding: PositionEncoding,
+) -> anyhow::Result<lsp_types::Range> {
+    let start = lsp_position_from_tree_sitter_point(range.start_point, line_index, encoding)?;
+    let end = lsp_position_from_tree_sitter_point(range.end_point, line_index, encoding)?;
+    Ok(lsp_types::Range::new(start, end))
+}
+
 #[cfg(test)]
 pub(crate) fn test_ark_file(code: &str) -> (oak_db::OakDatabase, ArkFile) {
     use aether_path::FilePath;

crates/ark/src/lsp/completions/completion_item.rs

@@ -36,6 +36,7 @@ use tower_lsp::lsp_types::Range;
 use tower_lsp::lsp_types::TextEdit;
 use tree_sitter::Node;
 
+use crate::lsp::ark_file::lsp_position_from_tree_sitter_point;
 use crate::lsp::completions::function_context::ArgumentsStatus;
 use crate::lsp::completions::function_context::FunctionContext;
 use crate::lsp::completions::function_context::FunctionRefUsage;
@@ -644,7 +645,8 @@ fn completion_item_from_dot_dot_dot(
 
     item.kind = Some(CompletionItemKind::FIELD);
 
-    let position = context.lsp_position_from_tree_sitter_point(context.point)?;
+    let position =
+        lsp_position_from_tree_sitter_point(context.point, context.line_index, context.encoding)?;
 
     let range = Range {
         start: position,

crates/ark/src/lsp/completions/function_context.rs

@@ -10,6 +10,8 @@ use tower_lsp::lsp_types;
 use tower_lsp::lsp_types::Range;
 use tree_sitter::Node;
 
+use crate::lsp::ark_file::lsp_position_from_tree_sitter_point;
+use crate::lsp::ark_file::lsp_range_from_tree_sitter_range;
 use crate::lsp::document_context::DocumentContext;
 use crate::lsp::traits::node::NodeExt;
 use crate::treesitter::node_find_parent_call;
@@ -61,8 +63,11 @@ impl FunctionContext {
             // We shouldn't ever attempt to instantiate a FunctionContext or
             // function-flavored CompletionItem in this degenerate case, but we
             // return a dummy FunctionContext just to be safe.
-            let node_end = document_context
-                .lsp_position_from_tree_sitter_point(completion_node.range().end_point)?;
+            let node_end = lsp_position_from_tree_sitter_point(
+                completion_node.range().end_point,
+                document_context.line_index,
+                document_context.encoding,
+            )?;
 
             return Ok(Self {
                 name: String::new(),
@@ -107,11 +112,17 @@ impl FunctionContext {
         Ok(Self {
             name,
             range: match function_name_node {
-                Some(node) => document_context.lsp_range_from_tree_sitter_range(node.range())?,
+                Some(node) => lsp_range_from_tree_sitter_range(
+                    node.range(),
+                    document_context.line_index,
+                    document_context.encoding,
+                )?,
                 None => {
                     // Create a zero-width range at the end of the effective_function_node
-                    let node_end = document_context.lsp_position_from_tree_sitter_point(
+                    let node_end = lsp_position_from_tree_sitter_point(
                         effective_function_node.range().end_point,
+                        document_context.line_index,
+                        document_context.encoding,
                     )?;
                     lsp_types::Range::new(node_end, node_end)
                 },

crates/ark/src/lsp/document_context.rs

@@ -5,9 +5,7 @@
 //
 //
 
-use aether_lsp_utils::proto::to_proto;
 use aether_lsp_utils::proto::PositionEncoding;
-use tower_lsp::lsp_types;
 use tree_sitter::Node;
 use tree_sitter::Point;
 
@@ -108,26 +106,6 @@ impl<'a> DocumentContext<'a> {
             trigger,
         }
     }
-
-    pub fn lsp_position_from_tree_sitter_point(
-        &self,
-        point: tree_sitter::Point,
-    ) -> anyhow::Result<lsp_types::Position> {
-        let line_col = biome_line_index::LineCol {
-            line: point.row as u32,
-            col: point.column as u32,
-        };
-        to_proto::position_from_line_col(line_col, self.line_index, self.encoding)
-    }
-
-    pub fn lsp_range_from_tree_sitter_range(
-        &self,
-        range: tree_sitter::Range,
-    ) -> anyhow::Result<lsp_types::Range> {
-        let start = self.lsp_position_from_tree_sitter_point(range.start_point)?;
-        let end = self.lsp_position_from_tree_sitter_point(range.end_point)?;
-        Ok(lsp_types::Range::new(start, end))
-    }
 }
 
 /// Owns a `db` + `ArkFile` so unit tests can build a `DocumentContext` the same