Add INDEX names to set of exported symbols

Author: lionel-

crates/ark/src/lsp/diagnostics.rs

@@ -1850,4 +1850,40 @@ foo
             assert_eq!(diagnostics.len(), 1);
         });
     }
+
+    #[test]
+    fn test_penguins_symbol_no_diagnostic() {
+        r_task(|| {
+            let palmerpenguins_dir = crate::lsp::inputs::package::temp_palmerpenguin();
+            let palmerpenguins_pkg = Package::load_from_folder(palmerpenguins_dir.path())
+                .unwrap()
+                .unwrap();
+            let library = Library::new(vec![]).insert("penguins", palmerpenguins_pkg);
+
+            // Simulate a world state with the penguins package installed and attached
+            let mut state = DEFAULT_STATE.clone();
+            state.library = library;
+            state.console_scopes = vec![vec!["library".to_string()]];
+
+            let code = r#"
+                library(penguins)
+                penguins
+                path_to_file
+                penguins_raw
+            "#;
+            let document = Document::new(code, None);
+            let diagnostics = generate_diagnostics(document, state.clone());
+            assert!(diagnostics.is_empty());
+
+            let code = r#"
+                penguins
+                path_to_file
+                penguins_raw
+                library(penguins)
+            "#;
+            let document = Document::new(code, None);
+            let diagnostics = generate_diagnostics(document, state);
+            assert_eq!(diagnostics.len(), 3);
+        })
+    }
 }

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

@@ -9,6 +9,7 @@ use std::fs;
 use std::path::PathBuf;
 
 use crate::lsp::inputs::package_description::Description;
+use crate::lsp::inputs::package_index::Index;
 use crate::lsp::inputs::package_namespace::Namespace;
 
 /// Represents an R package and its metadata relevant for static analysis.
@@ -22,17 +23,28 @@ pub struct Package {
     pub namespace: Namespace,
 
     // List of symbols exported via NAMESPACE `export()` directives and via
-    // `DocType{data}`. Note the latter should only apply to packages with
-    // `LazyData: true` but currently applies to all packages, as a stopgap
-    // to prevent spurious diagnostics (we accept false negatives to avoid
-    // annoying false positives).
+    // documented symbols listed in INDEX. The latter is a stopgap to ensure we
+    // support exported datasets and prevent spurious diagnostics (we accept
+    // false negatives to avoid annoying false positives).
     pub exported_symbols: Vec<String>,
 }
 
 impl Package {
-    pub fn new(path: PathBuf, description: Description, namespace: Namespace) -> Self {
+    pub fn new(
+        path: PathBuf,
+        description: Description,
+        namespace: Namespace,
+        index: Index,
+    ) -> Self {
         // Compute exported symbols. Start from explicit NAMESPACE exports.
-        let exported_symbols = namespace.exports.clone();
+        let mut exported_symbols = namespace.exports.clone();
+
+        // Add all documented symbols. This should cover documented datasets.
+        exported_symbols.extend(index.names.iter().cloned());
+
+        // Sort and deduplicate (we expect lots of duplicates)
+        exported_symbols.sort();
+        exported_symbols.dedup();
 
         Self {
             path,
@@ -44,7 +56,7 @@ impl Package {
 
     #[cfg(test)]
     pub fn from_parts(path: PathBuf, description: Description, namespace: Namespace) -> Self {
-        Self::new(path, description, namespace)
+        Self::new(path, description, namespace, Index::default())
     }
 
     /// Load a package from a given path.
@@ -73,10 +85,22 @@ impl Package {
             Namespace::default()
         };
 
+        let index = match Index::load_from_folder(package_path) {
+            Ok(index) => index,
+            Err(err) => {
+                tracing::warn!(
+                    "Can't load INDEX file from `{path}`: {err:?}",
+                    path = package_path.to_string_lossy()
+                );
+                Index::default()
+            },
+        };
+
         Ok(Some(Self::new(
             package_path.to_path_buf(),
             description,
             namespace,
+            index,
         )))
     }
 
@@ -95,9 +119,98 @@ impl Package {
                     "`Package` field in `DESCRIPTION` doesn't match folder name '{name}'"
                 ));
             }
+
             Ok(Some(pkg))
         } else {
             Ok(None)
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::lsp::inputs::package_description::Description;
+    use crate::lsp::inputs::package_index::Index;
+    use crate::lsp::inputs::package_namespace::Namespace;
+
+    fn new_package(name: &str, ns: Namespace, index: Index) -> Package {
+        Package::new(
+            std::path::PathBuf::from("/fake"),
+            Description {
+                name: name.to_string(),
+                ..Description::default()
+            },
+            ns,
+            index,
+        )
+    }
+
+    #[test]
+    fn exported_symbols_are_sorted_and_unique() {
+        let mut ns = Namespace::default();
+        ns.exports = vec!["b".to_string(), "a".to_string(), "a".to_string()];
+
+        let mut index = Index::default();
+        index.names = vec!["c".to_string(), "a".to_string(), "a".to_string()];
+
+        let pkg = new_package("foo", ns, index);
+        assert_eq!(pkg.exported_symbols, vec!["a", "b", "c"]);
+    }
+
+    #[test]
+    fn exported_symbols_empty_when_none() {
+        let ns = Namespace::default();
+        let idx = Index::default();
+        let pkg = new_package("foo", ns, idx);
+        assert!(pkg.exported_symbols.is_empty());
+    }
+
+    #[test]
+    fn load_from_folder_reads_description_namespace_and_index() {
+        let dir = temp_palmerpenguin();
+
+        let pkg = Package::load_from_folder(dir.path()).unwrap().unwrap();
+
+        // Should include all exports and all index names, sorted and deduped
+        assert_eq!(pkg.exported_symbols, vec![
+            "path_to_file",
+            "penguins",
+            "penguins_raw"
+        ]);
+        assert_eq!(pkg.description.name, "penguins");
+    }
+}
+
+#[cfg(test)]
+pub(crate) fn temp_palmerpenguin() -> tempfile::TempDir {
+    let dir = tempfile::tempdir().unwrap();
+
+    // Write DESCRIPTION
+    let description = "\
+Package: penguins
+Version: 1.0
+";
+    fs::write(dir.path().join("DESCRIPTION"), description).unwrap();
+
+    // Write NAMESPACE
+    let namespace = "\
+export(path_to_file)
+export(penguins)
+";
+    fs::write(dir.path().join("NAMESPACE"), namespace).unwrap();
+
+    // Write INDEX
+    let index = "\
+path_to_file            Get file path to 'penguins.csv' and
+                    'penguins_raw.csv' files
+penguins                Size measurements for adult foraging penguins
+                    near Palmer Station, Antarctica
+penguins_raw            Penguin size, clutch, and blood isotope data
+                    for foraging adults near Palmer Station,
+                    Antarctica
+";
+    fs::write(dir.path().join("INDEX"), index).unwrap();
+
+    dir
+}

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

@@ -48,6 +48,17 @@ pub struct Description {
     pub fields: Dcf,
 }
 
+impl Default for Description {
+    fn default() -> Self {
+        Description {
+            name: String::new(),
+            version: String::new(),
+            depends: Vec::new(),
+            fields: Dcf::default(),
+        }
+    }
+}
+
 impl Description {
     /// Parse a DESCRIPTION file in DCF format
     pub fn parse(contents: &str) -> anyhow::Result<Self> {

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

@@ -7,6 +7,7 @@
 
 use std::path::Path;
 
+#[derive(Default, Clone, Debug)]
 pub struct Index {
     pub names: Vec<String>,
 }