More completion refactoring around how completions are marshalled (#755)

* Refactor how completion items are marshalled

* It can be nice to see completion text and type

* Inline logging

* This is already being tracked in #764

* We're definitely not doing this and the logging is super annoying

* Rescue this comment

* Streamline completion item tracking

* Go back to keying on label. If we have future concerns about two different CompletionItems having the same label, new logging will alert us and we can dig deeper.
* Rename CompletionWithSource to CompletionItemWithSource and only track the first source that contributes the item.
* Transfer completion ownership to the finalizer

* Let the compiler infer the type

Co-authored-by: Davis Vaughan <[email protected]>

* Trace not debug

Co-authored-by: Davis Vaughan <[email protected]>

* Remove comment

---------

Co-authored-by: Davis Vaughan <[email protected]>
Co-authored-by: Davis Vaughan <[email protected]>

Author: jennybc

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

@@ -12,14 +12,21 @@ use crate::lsp::completions::sources::composite;
 use crate::lsp::completions::sources::unique;
 use crate::lsp::document_context::DocumentContext;
 use crate::lsp::state::WorldState;
+use crate::treesitter::node_text;
+use crate::treesitter::NodeTypeExt;
 
 // Entry point for completions.
 // Must be within an `r_task()`.
 pub(crate) fn provide_completions(
     document_context: &DocumentContext,
     state: &WorldState,
 ) -> anyhow::Result<Vec<CompletionItem>> {
-    log::info!("provide_completions()");
+    log::info!(
+        "provide_completions() - Completion node text: '{node_text}', Node type: '{node_type:?}'",
+        node_text = node_text(&document_context.node, &document_context.document.contents)
+            .unwrap_or_default(),
+        node_type = document_context.node.node_type()
+    );
 
     let completion_context = CompletionContext::new(document_context, state);
 

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

@@ -33,7 +33,7 @@ where
     S: CompletionSource,
 {
     let source_name = source.name();
-    log::trace!("Trying completions from source: {}", source_name);
+    log::info!("Trying completions from source: {}", source_name);
 
     if let Some(completions) = source.provide_completions(completion_context)? {
         log::info!(
@@ -46,17 +46,3 @@ where
         Ok(None)
     }
 }
-
-pub fn push_completions<S>(
-    source: S,
-    completion_context: &CompletionContext,
-    completions: &mut Vec<CompletionItem>,
-) -> anyhow::Result<()>
-where
-    S: CompletionSource,
-{
-    if let Some(mut additional_completions) = collect_completions(source, completion_context)? {
-        completions.append(&mut additional_completions);
-    }
-    Ok(())
-}

crates/ark/src/lsp/completions/sources/composite.rs

@@ -14,25 +14,33 @@ mod snippets;
 mod subset;
 mod workspace;
 
-use std::collections::HashSet;
+use std::collections::HashMap;
 
 use stdext::*;
 use tower_lsp::lsp_types::CompletionItem;
 use tower_lsp::lsp_types::CompletionItemKind;
 use tree_sitter::Node;
 
 use crate::lsp::completions::completion_context::CompletionContext;
-use crate::lsp::completions::sources::push_completions;
+use crate::lsp::completions::sources::collect_completions;
+use crate::lsp::completions::sources::CompletionSource;
 use crate::treesitter::NodeType;
 use crate::treesitter::NodeTypeExt;
 
+// Locally useful data structure for tracking completions and their source
+#[derive(Clone, Default)]
+struct CompletionItemWithSource {
+    item: CompletionItem,
+    source: String,
+}
+
 /// Gets completions from all composite sources, with deduplication and sorting
 pub(crate) fn get_completions(
     completion_context: &CompletionContext,
 ) -> anyhow::Result<Option<Vec<CompletionItem>>> {
     log::info!("Getting completions from composite sources");
 
-    let mut completions: Vec<CompletionItem> = vec![];
+    let mut completions = HashMap::new();
 
     // Call, pipe, and subset completions should show up no matter what when
     // the user requests completions. This allows them to "tab" their way
@@ -50,7 +58,6 @@ pub(crate) fn get_completions(
     // For the rest of the general completions, we require an identifier to
     // begin showing anything.
     if is_identifier_like(completion_context.document_context.node) {
-        // Consulted settings.json
         push_completions(keyword::KeywordSource, completion_context, &mut completions)?;
 
         push_completions(
@@ -78,16 +85,64 @@ pub(crate) fn get_completions(
         )?;
     }
 
-    // Remove duplicates
-    let mut uniques = HashSet::new();
-    completions.retain(|x| uniques.insert(x.label.clone()));
+    // Simplify to plain old CompletionItems and sort them
+    let completions = finalize_completions(completions);
+
+    Ok(Some(completions))
+}
 
-    // Sort completions by providing custom 'sort' text to be used when
-    // ordering completion results. we use some placeholders at the front
-    // to 'bin' different completion types differently; e.g. we place parameter
-    // completions at the front, followed by variable completions (like pipe
-    // completions and subset completions), followed by anything else.
-    for item in &mut completions {
+fn push_completions<S>(
+    source: S,
+    completion_context: &CompletionContext,
+    completions: &mut HashMap<String, CompletionItemWithSource>,
+) -> anyhow::Result<()>
+where
+    S: CompletionSource,
+{
+    let source_name = source.name();
+
+    if let Some(source_completions) = collect_completions(source, completion_context)? {
+        for item in source_completions {
+            if let Some(existing) = completions.get(&item.label) {
+                log::trace!(
+                    "Completion with label '{}' already exists (first contributed by source: {}, now also from: {})",
+                    item.label,
+                    existing.source,
+                    source_name
+                );
+            } else {
+                completions.insert(item.label.clone(), CompletionItemWithSource {
+                    item,
+                    source: source_name.to_string(),
+                });
+            }
+        }
+    }
+
+    Ok(())
+}
+
+/// Produce plain old CompletionItems and sort them
+fn finalize_completions(
+    completions: HashMap<String, CompletionItemWithSource>,
+) -> Vec<CompletionItem> {
+    let mut items: Vec<CompletionItem> = completions
+        .into_values()
+        .map(|completion_with_source| completion_with_source.item)
+        .collect();
+
+    sort_completions(&mut items);
+
+    items
+}
+
+// Sort completions by providing custom 'sort' text to be used when
+// ordering completion results. we use some placeholders at the front
+// to 'bin' different completion types differently; e.g. we place parameter
+// completions at the front, followed by variable completions (like pipe
+// completions and subset completions), followed by anything else.
+fn sort_completions(completions: &mut Vec<CompletionItem>) {
+    for item in completions {
         // Start with existing `sort_text` if one exists
         let sort_text = item.sort_text.take();
         let sort_text = match sort_text {
@@ -115,8 +170,6 @@ pub(crate) fn get_completions(
             }
         }
     }
-
-    Ok(Some(completions))
 }
 
 fn is_identifier_like(x: Node) -> bool {