Refactor completions (#754)

* Hello CompletionBuilder

* Move completions_from_unique_sources() into the builder

* Move completions_from_composite_sources() into the builder

* Opening move re: using parameter_hints from the builder

* Let's try an extension trait (namespace)

* Use extension trait (search_path)

* Use extension trait (workspace)

* Move pipe root into the builder and make it lazy

* Get pipe root from the builder (not passing as arg)

* Opening move for a single interface for completion sources

* Put unique and composite completion aggregation logic back where it started

* Fully shift to CompletionSource approach for unique sources

* Make CompletionSource trait object-safe by adding &self parameter

Allows us to iterate in a clean way over unique sources

* Adopt CompletionSource trait for composite sources

Finally all completion sources are shifted over to a new, common pattern.

Some return types were changed (to Option<Vec<CompletionItem>>) as a move towards consistency in completion source functions

* Update copyright headers

* More consistent with pattern used by other completion sources

* Consolidate imports

* Be more consistent and intentional around logging for unique sources

* Borrow some ideas from unique for composite sources

* More work on logging, mostly for composite sources

Remove on-entry logging for completion sources, since the 2 main aggregators now log before trying each source

Shift towards debug for logging from a completion source or related helpers

* Make parameter hints lazy

* Remove superficial differences in handling of parameter hints and pipe root

* This hasn't been dead code for a while

* Can have same visiblity as composite

* Rescue existing comments

* Can be private

* Restore / add some hints

* Use a more restricted visibility

* Tweak comment

* The Great Renaming

* CompletionBuilder -> CompletionContext
* context -> document_context (where necessary for disambiguation)

* Get rid of the loops, but use common helpers for each "iteration" / source

* Use `log::trace!`, not `log::debug!`

* Don't import `anyhow::Result`

* Go back to marshalling unique and composite completions via free functions

* Rename to push_completions()

* Encapsulate pipe root error handling in its accessor

* Remove unnecessary re-export of find_pipe_root

Author: jennybc

crates/ark/src/lsp/completions.rs

@@ -5,6 +5,7 @@
 //
 //
 
+mod completion_context;
 mod completion_item;
 mod parameter_hints;
 mod provide;

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

@@ -0,0 +1,54 @@
+//
+// completion_context.rs
+//
+// Copyright (C) 2025 Posit Software, PBC. All rights reserved.
+//
+//
+
+use std::cell::OnceCell;
+
+use crate::lsp::completions::parameter_hints::ParameterHints;
+use crate::lsp::completions::parameter_hints::{self};
+use crate::lsp::completions::sources::composite::pipe::find_pipe_root;
+use crate::lsp::completions::sources::composite::pipe::PipeRoot;
+use crate::lsp::document_context::DocumentContext;
+use crate::lsp::state::WorldState;
+
+pub(crate) struct CompletionContext<'a> {
+    pub(crate) document_context: &'a DocumentContext<'a>,
+    pub(crate) state: &'a WorldState,
+    parameter_hints_cell: OnceCell<ParameterHints>,
+    pipe_root_cell: OnceCell<Option<PipeRoot>>,
+}
+
+impl<'a> CompletionContext<'a> {
+    pub fn new(document_context: &'a DocumentContext, state: &'a WorldState) -> Self {
+        Self {
+            document_context,
+            state,
+            parameter_hints_cell: OnceCell::new(),
+            pipe_root_cell: OnceCell::new(),
+        }
+    }
+
+    pub fn parameter_hints(&self) -> &ParameterHints {
+        self.parameter_hints_cell.get_or_init(|| {
+            parameter_hints::parameter_hints(
+                self.document_context.node,
+                &self.document_context.document.contents,
+            )
+        })
+    }
+
+    pub fn pipe_root(&self) -> Option<PipeRoot> {
+        self.pipe_root_cell
+            .get_or_init(|| match find_pipe_root(self.document_context) {
+                Ok(root) => root,
+                Err(e) => {
+                    log::error!("Error trying to find pipe root: {}", e);
+                    None
+                },
+            })
+            .clone()
+    }
+}

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

@@ -8,7 +8,6 @@
 use std::fs::DirEntry;
 
 use anyhow::bail;
-use anyhow::Result;
 use harp::r_symbol;
 use harp::utils::is_symbol_valid;
 use harp::utils::r_env_binding_is_active;
@@ -52,23 +51,23 @@ use crate::treesitter::NodeTypeExt;
 pub(super) fn completion_item(
     label: impl AsRef<str>,
     data: CompletionData,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     Ok(CompletionItem {
         label: label.as_ref().to_string(),
         data: Some(serde_json::to_value(data)?),
         ..Default::default()
     })
 }
 
-pub(super) fn completion_item_from_file(entry: DirEntry) -> Result<CompletionItem> {
+pub(super) fn completion_item_from_file(entry: DirEntry) -> anyhow::Result<CompletionItem> {
     let name = entry.file_name().to_string_lossy().to_string();
     let mut item = completion_item(name, CompletionData::File { path: entry.path() })?;
 
     item.kind = Some(CompletionItemKind::FILE);
     Ok(item)
 }
 
-pub(super) fn completion_item_from_directory(entry: DirEntry) -> Result<CompletionItem> {
+pub(super) fn completion_item_from_directory(entry: DirEntry) -> anyhow::Result<CompletionItem> {
     let mut name = entry.file_name().to_string_lossy().to_string();
     name.push('/');
 
@@ -84,7 +83,7 @@ pub(super) fn completion_item_from_directory(entry: DirEntry) -> Result<Completi
     Ok(item)
 }
 
-pub(super) fn completion_item_from_direntry(entry: DirEntry) -> Result<CompletionItem> {
+pub(super) fn completion_item_from_direntry(entry: DirEntry) -> anyhow::Result<CompletionItem> {
     let is_dir = entry
         .file_type()
         .map(|value| value.is_dir())
@@ -99,7 +98,7 @@ pub(super) fn completion_item_from_direntry(entry: DirEntry) -> Result<Completio
 pub(super) fn completion_item_from_assignment(
     node: &Node,
     context: &DocumentContext,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     let lhs = node.child_by_field_name("lhs").into_result()?;
     let rhs = node.child_by_field_name("rhs").into_result()?;
 
@@ -143,7 +142,7 @@ pub(super) fn completion_item_from_assignment(
 pub(super) unsafe fn completion_item_from_package(
     package: &str,
     append_colons: bool,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     let mut item = completion_item(package.to_string(), CompletionData::Package {
         name: package.to_string(),
     })?;
@@ -170,8 +169,8 @@ pub(super) unsafe fn completion_item_from_package(
 pub(super) fn completion_item_from_function(
     name: &str,
     package: Option<&str>,
-    parameter_hints: ParameterHints,
-) -> Result<CompletionItem> {
+    parameter_hints: &ParameterHints,
+) -> anyhow::Result<CompletionItem> {
     let label = format!("{}", name);
     let mut item = completion_item(label, CompletionData::Function {
         name: name.to_string(),
@@ -219,7 +218,7 @@ fn item_details(package: Option<&str>) -> CompletionItemLabelDetails {
 }
 
 // TODO
-pub(super) unsafe fn completion_item_from_dataset(name: &str) -> Result<CompletionItem> {
+pub(super) unsafe fn completion_item_from_dataset(name: &str) -> anyhow::Result<CompletionItem> {
     let mut item = completion_item(name.to_string(), CompletionData::Unknown)?;
     item.kind = Some(CompletionItemKind::STRUCT);
     Ok(item)
@@ -229,7 +228,7 @@ pub(super) unsafe fn completion_item_from_data_variable(
     name: &str,
     owner: &str,
     enquote: bool,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     let mut item = completion_item(name.to_string(), CompletionData::DataVariable {
         name: name.to_string(),
         owner: owner.to_string(),
@@ -253,8 +252,8 @@ pub(super) unsafe fn completion_item_from_object(
     envir: SEXP,
     package: Option<&str>,
     promise_strategy: PromiseStrategy,
-    parameter_hints: ParameterHints,
-) -> Result<CompletionItem> {
+    parameter_hints: &ParameterHints,
+) -> anyhow::Result<CompletionItem> {
     if r_typeof(object) == PROMSXP {
         return completion_item_from_promise(
             name,
@@ -303,8 +302,8 @@ pub(super) unsafe fn completion_item_from_promise(
     envir: SEXP,
     package: Option<&str>,
     promise_strategy: PromiseStrategy,
-    parameter_hints: ParameterHints,
-) -> Result<CompletionItem> {
+    parameter_hints: &ParameterHints,
+) -> anyhow::Result<CompletionItem> {
     if r_promise_is_forced(object) {
         // Promise has already been evaluated before.
         // Generate completion item from underlying value.
@@ -352,7 +351,7 @@ pub(super) unsafe fn completion_item_from_promise(
     Ok(item)
 }
 
-pub(super) fn completion_item_from_active_binding(name: &str) -> Result<CompletionItem> {
+pub(super) fn completion_item_from_active_binding(name: &str) -> anyhow::Result<CompletionItem> {
     // We never want to force active bindings, so we return a fairly
     // generic completion item
     let mut item = completion_item(name, CompletionData::Object {
@@ -373,8 +372,8 @@ pub(super) unsafe fn completion_item_from_namespace(
     name: &str,
     namespace: SEXP,
     package: &str,
-    parameter_hints: ParameterHints,
-) -> Result<CompletionItem> {
+    parameter_hints: &ParameterHints,
+) -> anyhow::Result<CompletionItem> {
     // First, look in the namespace itself.
     if let Some(item) = completion_item_from_symbol(
         name,
@@ -410,7 +409,7 @@ pub(super) unsafe fn completion_item_from_lazydata(
     name: &str,
     env: SEXP,
     package: &str,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     // Important to use `Simple` here, as lazydata bindings are calls to `lazyLoadDBfetch()`
     // but we don't want to force them during completion generation because they often take a
     // long time to load.
@@ -419,7 +418,8 @@ pub(super) unsafe fn completion_item_from_lazydata(
     // Lazydata objects are never functions, so this doesn't really matter
     let parameter_hints = ParameterHints::Enabled;
 
-    match completion_item_from_symbol(name, env, Some(package), promise_strategy, parameter_hints) {
+    match completion_item_from_symbol(name, env, Some(package), promise_strategy, &parameter_hints)
+    {
         Some(item) => item,
         None => {
             // Should be impossible, but we'll be extra safe
@@ -433,8 +433,8 @@ pub(super) unsafe fn completion_item_from_symbol(
     envir: SEXP,
     package: Option<&str>,
     promise_strategy: PromiseStrategy,
-    parameter_hints: ParameterHints,
-) -> Option<Result<CompletionItem>> {
+    parameter_hints: &ParameterHints,
+) -> Option<anyhow::Result<CompletionItem>> {
     let symbol = r_symbol!(name);
 
     match r_env_binding_is_active(envir, symbol) {
@@ -475,7 +475,7 @@ pub(super) unsafe fn completion_item_from_symbol(
 pub(super) fn completion_item_from_scope_parameter(
     parameter: &str,
     _context: &DocumentContext,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     let mut item = completion_item(parameter, CompletionData::ScopeParameter {
         name: parameter.to_string(),
     })?;
@@ -488,7 +488,7 @@ pub(super) fn completion_item_from_parameter(
     parameter: &str,
     callee: &str,
     context: &DocumentContext,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     if parameter == "..." {
         return completion_item_from_dot_dot_dot(callee, context);
     }
@@ -522,7 +522,7 @@ pub(super) fn completion_item_from_parameter(
 fn completion_item_from_dot_dot_dot(
     callee: &str,
     context: &DocumentContext,
-) -> Result<CompletionItem> {
+) -> anyhow::Result<CompletionItem> {
     // Special behavior for `...` arguments, where we want to show them
     // in quick suggestions (to show help docs for them), but not actually
     // insert any text for them if the user selects them. Can't use an

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

@@ -1,3 +1,10 @@
+//
+// parameter_hints.rs
+//
+// Copyright (C) 2025 Posit Software, PBC. All rights reserved.
+//
+//
+
 use ropey::Rope;
 use tree_sitter::Node;
 

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

@@ -1,35 +1,34 @@
 //
 // provide.rs
 //
-// Copyright (C) 2023 Posit Software, PBC. All rights reserved.
+// Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
 //
 //
 
-use anyhow::Result;
 use tower_lsp::lsp_types::CompletionItem;
 
-use crate::lsp::completions::parameter_hints::parameter_hints;
-use crate::lsp::completions::sources::completions_from_composite_sources;
-use crate::lsp::completions::sources::completions_from_unique_sources;
+use crate::lsp::completions::completion_context::CompletionContext;
+use crate::lsp::completions::sources::composite;
+use crate::lsp::completions::sources::unique;
 use crate::lsp::document_context::DocumentContext;
 use crate::lsp::state::WorldState;
 
 // Entry point for completions.
 // Must be within an `r_task()`.
 pub(crate) fn provide_completions(
-    context: &DocumentContext,
+    document_context: &DocumentContext,
     state: &WorldState,
-) -> Result<Vec<CompletionItem>> {
+) -> anyhow::Result<Vec<CompletionItem>> {
     log::info!("provide_completions()");
 
-    let parameter_hints = parameter_hints(context.node, &context.document.contents);
+    let completion_context = CompletionContext::new(document_context, state);
 
-    if let Some(completions) = completions_from_unique_sources(context, parameter_hints)? {
+    // Try unique sources first
+    if let Some(completions) = unique::get_completions(&completion_context)? {
         return Ok(completions);
-    };
+    }
 
     // At this point we aren't in a "unique" completion case, so just return a
-    // set of reasonable completions based on loaded packages, the open
-    // document, the current workspace, and any call related arguments
-    completions_from_composite_sources(context, state, parameter_hints)
+    // set of reasonable completions from composite sources
+    Ok(composite::get_completions(&completion_context)?.unwrap_or_default())
 }

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

@@ -6,7 +6,6 @@
 //
 
 use anyhow::bail;
-use anyhow::Result;
 use stdext::*;
 use tower_lsp::lsp_types::CompletionItem;
 use tower_lsp::lsp_types::Documentation;
@@ -16,7 +15,7 @@ use tower_lsp::lsp_types::MarkupKind;
 use crate::lsp::completions::types::CompletionData;
 use crate::lsp::help::RHtmlHelp;
 
-pub fn resolve_completion(item: &mut CompletionItem) -> Result<bool> {
+pub fn resolve_completion(item: &mut CompletionItem) -> anyhow::Result<bool> {
     let Some(data) = item.data.clone() else {
         bail!("Completion '{}' has no associated data", item.label);
     };
@@ -46,7 +45,10 @@ pub fn resolve_completion(item: &mut CompletionItem) -> Result<bool> {
     }
 }
 
-fn resolve_package_completion_item(item: &mut CompletionItem, package: &str) -> Result<bool> {
+fn resolve_package_completion_item(
+    item: &mut CompletionItem,
+    package: &str,
+) -> anyhow::Result<bool> {
     let topic = join!(package, "-package");
     let help = unwrap!(RHtmlHelp::from_topic(topic.as_str(), Some(package))?, None => {
         return Ok(false);
@@ -68,7 +70,7 @@ fn resolve_function_completion_item(
     item: &mut CompletionItem,
     name: &str,
     package: Option<&str>,
-) -> Result<bool> {
+) -> anyhow::Result<bool> {
     let help = unwrap!(RHtmlHelp::from_function(name, package)?, None => {
         return Ok(false);
     });
@@ -90,7 +92,7 @@ fn resolve_parameter_completion_item(
     item: &mut CompletionItem,
     name: &str,
     function: &str,
-) -> Result<bool> {
+) -> anyhow::Result<bool> {
     // Get help for this function.
     let help = unwrap!(RHtmlHelp::from_function(function, None)?, None => {
         return Ok(false);