Add symbols imported in packages to workspace

crates/ark/src/lsp/diagnostics.rs

@@ -30,6 +30,7 @@ use crate::lsp::encoding::convert_tree_sitter_range_to_lsp_range;
 use crate::lsp::indexer;
 use crate::lsp::inputs::library::Library;
 use crate::lsp::inputs::package::Package;
+use crate::lsp::inputs::source_root::SourceRoot;
 use crate::lsp::state::WorldState;
 use crate::lsp::traits::node::NodeExt;
 use crate::lsp::traits::rope::RopeExt;
@@ -62,6 +63,9 @@ pub struct DiagnosticContext<'a> {
     // The set of packages that are currently installed.
     pub installed_packages: HashSet<String>,
 
+    /// Reference to source root, if any.
+    pub root: &'a Option<SourceRoot>,
+
     /// Reference to the library for looking up package exports.
     pub library: &'a Library,
 
@@ -84,13 +88,14 @@ impl Default for DiagnosticsConfig {
 }
 
 impl<'a> DiagnosticContext<'a> {
-    pub fn new(contents: &'a Rope, library: &'a Library) -> Self {
+    pub fn new(contents: &'a Rope, root: &'a Option<SourceRoot>, library: &'a Library) -> Self {
         Self {
             contents,
             document_symbols: Vec::new(),
             session_symbols: HashSet::new(),
             workspace_symbols: HashSet::new(),
             installed_packages: HashSet::new(),
+            root,
             library,
             library_symbols: BTreeMap::new(),
             in_formula: false,
@@ -145,7 +150,7 @@ pub(crate) fn generate_diagnostics(doc: Document, state: WorldState) -> Vec<Diag
         return diagnostics;
     }
 
-    let mut context = DiagnosticContext::new(&doc.contents, &state.library);
+    let mut context = DiagnosticContext::new(&doc.contents, &state.root, &state.library);
 
     // Add a 'root' context for the document.
     context.document_symbols.push(HashMap::new());
@@ -155,9 +160,45 @@ pub(crate) fn generate_diagnostics(doc: Document, state: WorldState) -> Vec<Diag
         indexer::IndexEntryData::Function { name, arguments: _ } => {
             context.workspace_symbols.insert(name.to_string());
         },
+        indexer::IndexEntryData::Variable { name } => {
+            context.workspace_symbols.insert(name.to_string());
+        },
         _ => {},
     });
 
+    // If this is a package, add imported symbols to workspace
+    if let Some(SourceRoot::Package(root)) = &state.root {
+        // Add symbols from `importFrom()` directives
+        for import in &root.namespace.imports {
+            context.workspace_symbols.insert(import.clone());
+        }
+
+        // Add symbols from `import()` directives
+        for bulk_import in &root.namespace.bulk_imports {
+            if let Some(pkg) = state.library.get(bulk_import) {
+                for export in &pkg.namespace.exports {
+                    context.workspace_symbols.insert(export.clone());
+                }
+            }
+        }
+    }
+
+    // Simple workaround to include testthat exports in test files. I think the
+    // general principle would be that (a) files in `tests/testthat/` include
+    // `testthat.R` as a preamble (note that people modify that file e.g. to add
+    // more `library()` calls), and (b) all helper files are included in a
+    // test-specific workspace (which is effectively the case currently as we
+    // don't special-case how workspace inclusion works for packages). We might
+    // want to provide a mechanism for test packages to declare this sort of
+    // test files setup.
+    if doc.testthat {
+        if let Some(pkg) = state.library.get("testthat") {
+            for export in &pkg.namespace.exports {
+                context.workspace_symbols.insert(export.clone());
+            }
+        }
+    }
+
     // Add per-environment session symbols
     for scope in state.console_scopes.iter() {
         for name in scope.iter() {

crates/ark/src/lsp/diagnostics_syntax.rs

@@ -314,7 +314,7 @@ mod tests {
     fn text_diagnostics(text: &str) -> Vec<Diagnostic> {
         let document = Document::new(text, None);
         let library = Library::default();
-        let context = DiagnosticContext::new(&document.contents, &library);
+        let context = DiagnosticContext::new(&document.contents, &None, &library);
         let diagnostics = syntax_diagnostics(document.ast.root_node(), &context).unwrap();
         diagnostics
     }

crates/ark/src/lsp/documents.rs

@@ -48,6 +48,10 @@ pub struct Document {
 
     // Configuration of the document, such as indentation settings.
     pub config: DocumentConfig,
+
+    /// Whether the document is a testthat file. This property should not be
+    /// here, it's just a short term stopgap.
+    pub testthat: bool,
 }
 
 impl std::fmt::Debug for Document {
@@ -80,6 +84,7 @@ impl Document {
             version,
             ast,
             config: Default::default(),
+            testthat: false,
         }
     }
 

crates/ark/src/lsp/inputs/library.rs

@@ -68,7 +68,7 @@ impl Library {
 
     fn load_package(&self, name: &str) -> anyhow::Result<Option<Package>> {
         for lib_path in self.library_paths.iter() {
-            match Package::load(&lib_path, name) {
+            match Package::load_from_library(&lib_path, name) {
                 Ok(Some(pkg)) => return Ok(Some(pkg)),
                 Ok(None) => (),
                 Err(err) => lsp::log_warn!("Can't load package: {err:?}"),

crates/ark/src/lsp/inputs/package.rs

@@ -23,15 +23,12 @@ pub struct Package {
 }
 
 impl Package {
-    /// Attempts to load a package from the given path and name.
-    pub fn load(lib_path: &std::path::Path, name: &str) -> anyhow::Result<Option<Self>> {
-        let package_path = lib_path.join(name);
-
+    /// Load a package from a given path.
+    pub fn load(package_path: &std::path::Path) -> anyhow::Result<Option<Self>> {
         let description_path = package_path.join("DESCRIPTION");
         let namespace_path = package_path.join("NAMESPACE");
 
-        // Only consider libraries that have a folder named after the
-        // requested package and that contains a description file
+        // Only consider directories that contain a description file
         if !description_path.is_file() {
             return Ok(None);
         }
@@ -41,12 +38,6 @@ impl Package {
         let description_contents = fs::read_to_string(&description_path)?;
         let description = Description::parse(&description_contents)?;
 
-        if description.name != name {
-            return Err(anyhow::anyhow!(
-                "`Package` field in `DESCRIPTION` doesn't match folder name '{name}'"
-            ));
-        }
-
         let namespace_contents = fs::read_to_string(&namespace_path)?;
         let namespace = Namespace::parse(&namespace_contents)?;
 
@@ -56,4 +47,25 @@ impl Package {
             namespace,
         }))
     }
+
+    /// Load a package from the given library path and name.
+    pub fn load_from_library(
+        lib_path: &std::path::Path,
+        name: &str,
+    ) -> anyhow::Result<Option<Self>> {
+        let package_path = lib_path.join(name);
+
+        // For library packages, ensure the invariant that the package name
+        // matches the folder name
+        if let Some(pkg) = Self::load(&package_path)? {
+            if pkg.description.name != name {
+                return Err(anyhow::anyhow!(
+                    "`Package` field in `DESCRIPTION` doesn't match folder name '{name}'"
+                ));
+            }
+            Ok(Some(pkg))
+        } else {
+            Ok(None)
+        }
+    }
 }