Skip to content

Commit 1e3b986

Browse files
authored
Merge pull request #870 from posit-dev/feature/static-imports
Static diagnostics for `library()` and `require()` calls
2 parents c2c981a + 9f8277e commit 1e3b986

File tree

15 files changed

+1323
-52
lines changed

15 files changed

+1323
-52
lines changed

crates/ark/src/lsp/diagnostics.rs

Lines changed: 370 additions & 10 deletions
Large diffs are not rendered by default.

crates/ark/src/lsp/diagnostics_syntax.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,10 +309,12 @@ mod tests {
309309
use crate::lsp::diagnostics::DiagnosticContext;
310310
use crate::lsp::diagnostics_syntax::syntax_diagnostics;
311311
use crate::lsp::documents::Document;
312+
use crate::lsp::inputs::library::Library;
312313

313314
fn text_diagnostics(text: &str) -> Vec<Diagnostic> {
314315
let document = Document::new(text, None);
315-
let context = DiagnosticContext::new(&document.contents);
316+
let library = Library::default();
317+
let context = DiagnosticContext::new(&document.contents, &library);
316318
let diagnostics = syntax_diagnostics(document.ast.root_node(), &context).unwrap();
317319
diagnostics
318320
}

crates/ark/src/lsp/indexer.rs

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use stdext::unwrap;
1919
use stdext::unwrap::IntoResult;
2020
use tower_lsp::lsp_types::Range;
2121
use tree_sitter::Node;
22+
use tree_sitter::Query;
2223
use walkdir::DirEntry;
2324
use walkdir::WalkDir;
2425

@@ -29,7 +30,7 @@ use crate::lsp::traits::rope::RopeExt;
2930
use crate::treesitter::BinaryOperatorType;
3031
use crate::treesitter::NodeType;
3132
use crate::treesitter::NodeTypeExt;
32-
use crate::treesitter::TSQuery;
33+
use crate::treesitter::TsQuery;
3334

3435
#[derive(Clone, Debug)]
3536
pub enum IndexEntryData {
@@ -347,23 +348,27 @@ fn index_r6_class_methods(
347348
entries: &mut Vec<IndexEntry>,
348349
) -> anyhow::Result<()> {
349350
// Tree-sitter query to match individual methods in R6Class public/private lists
350-
let query_str = r#"
351-
(argument
352-
name: (identifier) @access
353-
value: (call
354-
function: (identifier) @_list_fn
355-
arguments: (arguments
356-
(argument
357-
name: (identifier) @method_name
358-
value: (function_definition) @method_fn
359-
)
360-
)
361-
)
362-
(#match? @access "public|private")
363-
(#eq? @_list_fn "list")
364-
)
365-
"#;
366-
let mut ts_query = TSQuery::new(query_str)?;
351+
static R6_METHODS_QUERY: LazyLock<Query> = LazyLock::new(|| {
352+
let query_str = r#"
353+
(argument
354+
name: (identifier) @access
355+
value: (call
356+
function: (identifier) @_list_fn
357+
arguments: (arguments
358+
(argument
359+
name: (identifier) @method_name
360+
value: (function_definition) @method_fn
361+
)
362+
)
363+
)
364+
(#match? @access "public|private")
365+
(#eq? @_list_fn "list")
366+
)
367+
"#;
368+
let language = &tree_sitter_r::LANGUAGE.into();
369+
Query::new(language, query_str).expect("Failed to compile R6 methods query")
370+
});
371+
let mut ts_query = TsQuery::from_query(&*R6_METHODS_QUERY);
367372

368373
// We'll switch from Rope to String in the near future so let's not
369374
// worry about this conversion now

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

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
//
2+
// library.rs
3+
//
4+
// Copyright (C) 2025 by Posit Software, PBC
5+
//
6+
7+
use std::collections::HashMap;
8+
use std::path::PathBuf;
9+
use std::sync::Arc;
10+
use std::sync::RwLock;
11+
12+
use super::package::Package;
13+
use crate::lsp;
14+
15+
/// Lazily manages a list of known R packages by name
16+
#[derive(Default, Clone, Debug)]
17+
pub struct Library {
18+
/// Paths to library directories, i.e. what `base::libPaths()` returns.
19+
pub library_paths: Arc<Vec<PathBuf>>,
20+
21+
packages: Arc<RwLock<HashMap<String, Option<Arc<Package>>>>>,
22+
}
23+
24+
impl Library {
25+
pub fn new(library_paths: Vec<PathBuf>) -> Self {
26+
Self {
27+
packages: Arc::new(RwLock::new(HashMap::new())),
28+
library_paths: Arc::new(library_paths),
29+
}
30+
}
31+
32+
/// Get a package by name, loading and caching it if necessary.
33+
/// Returns `None` if the package can't be found or loaded.
34+
pub fn get(&self, name: &str) -> Option<Arc<Package>> {
35+
// Try to get from cache first (could be `None` if we already tried to
36+
// load a non-existent or broken package)
37+
if let Some(entry) = self.packages.read().unwrap().get(name) {
38+
return entry.clone();
39+
}
40+
41+
// Not cached, try to load
42+
let pkg = match self.load_package(name) {
43+
Ok(Some(pkg)) => Some(Arc::new(pkg)),
44+
Ok(None) => None,
45+
Err(err) => {
46+
lsp::log_error!("Can't load R package: {err:?}");
47+
None
48+
},
49+
};
50+
51+
self.packages
52+
.write()
53+
.unwrap()
54+
.insert(name.to_string(), pkg.clone());
55+
56+
pkg
57+
}
58+
59+
/// Insert a package in the library for testing purposes.
60+
#[cfg(test)]
61+
pub fn insert(self, name: &str, package: Package) -> Self {
62+
self.packages
63+
.write()
64+
.unwrap()
65+
.insert(name.to_string(), Some(Arc::new(package)));
66+
self
67+
}
68+
69+
fn load_package(&self, name: &str) -> anyhow::Result<Option<Package>> {
70+
for lib_path in self.library_paths.iter() {
71+
match Package::load(&lib_path, name)? {
72+
Some(pkg) => return Ok(Some(pkg)),
73+
None => (),
74+
}
75+
}
76+
77+
Ok(None)
78+
}
79+
}
80+
81+
#[cfg(test)]
82+
mod tests {
83+
use std::fs::File;
84+
use std::fs::{self};
85+
use std::io::Write;
86+
87+
use tempfile::TempDir;
88+
89+
use super::*;
90+
91+
// Helper to create a temporary package directory with DESCRIPTION and NAMESPACE
92+
fn create_temp_package(
93+
pkg_name: &str,
94+
description: &str,
95+
namespace: &str,
96+
) -> (TempDir, PathBuf) {
97+
let temp_dir = TempDir::new().unwrap();
98+
let pkg_dir = temp_dir.path().join(pkg_name);
99+
fs::create_dir(&pkg_dir).unwrap();
100+
101+
let desc_path = pkg_dir.join("DESCRIPTION");
102+
let mut desc_file = File::create(&desc_path).unwrap();
103+
desc_file.write_all(description.as_bytes()).unwrap();
104+
105+
let ns_path = pkg_dir.join("NAMESPACE");
106+
let mut ns_file = File::create(&ns_path).unwrap();
107+
ns_file.write_all(namespace.as_bytes()).unwrap();
108+
109+
(temp_dir, pkg_dir)
110+
}
111+
112+
#[test]
113+
fn test_load_and_cache_package() {
114+
let pkg_name = "mypkg";
115+
let description = r#"
116+
Package: mypkg
117+
Version: 1.0
118+
"#;
119+
let namespace = r#"
120+
export(foo)
121+
export(bar)
122+
importFrom(pkg, baz)
123+
"#;
124+
125+
let (temp_dir, _pkg_dir) = create_temp_package(pkg_name, description, namespace);
126+
127+
// Library should point to the temp_dir as its only library path
128+
let lib = Library::new(vec![temp_dir.path().to_path_buf()]);
129+
130+
// First access loads from disk
131+
let pkg = lib.get(pkg_name).unwrap();
132+
assert_eq!(pkg.description.name, "mypkg");
133+
134+
// Second access uses cache (note that we aren't testing that we are
135+
// indeed caching, just exercising the cache code path)
136+
assert!(lib.get(pkg_name).is_some());
137+
138+
// Negative cache: missing package
139+
assert!(lib.get("notapkg").is_none());
140+
// Now cached as absent
141+
assert!(lib.get("notapkg").is_none());
142+
143+
// Namespace is parsed
144+
assert_eq!(pkg.namespace.exports, vec!["bar", "foo"]);
145+
assert_eq!(pkg.namespace.imports, vec!["baz"]);
146+
}
147+
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
//
2+
// mod.rs
3+
//
4+
// Copyright (C) 2025 by Posit Software, PBC
5+
//
6+
//
7+
8+
pub mod library;
9+
pub mod package;
10+
pub mod package_description;
11+
pub mod package_namespace;
12+
pub mod source_root;

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
//
2+
// package.rs
3+
//
4+
// Copyright (C) 2025 by Posit Software, PBC
5+
//
6+
//
7+
8+
use std::fs;
9+
use std::path::PathBuf;
10+
11+
use crate::lsp::inputs::package_description::Description;
12+
use crate::lsp::inputs::package_namespace::Namespace;
13+
14+
/// Represents an R package and its metadata relevant for static analysis.
15+
#[derive(Clone, Debug)]
16+
pub struct Package {
17+
/// Path to the directory that contains `DESCRIPTION``. Can
18+
/// be an installed package or a package source.
19+
pub path: PathBuf,
20+
21+
pub description: Description,
22+
pub namespace: Namespace,
23+
}
24+
25+
impl Package {
26+
/// Attempts to load a package from the given path and name.
27+
pub fn load(lib_path: &std::path::Path, name: &str) -> anyhow::Result<Option<Self>> {
28+
let package_path = lib_path.join(name);
29+
30+
let description_path = package_path.join("DESCRIPTION");
31+
let namespace_path = package_path.join("NAMESPACE");
32+
33+
// Only consider libraries that have a folder named after the
34+
// requested package and that contains a description file
35+
if !description_path.is_file() {
36+
return Ok(None);
37+
}
38+
39+
// This fails if there is no `Package` field, so we're never loading
40+
// folders like bookdown projects as package
41+
let description_contents = fs::read_to_string(&description_path)?;
42+
let description = Description::parse(&description_contents)?;
43+
44+
if description.name != name {
45+
return Err(anyhow::anyhow!(
46+
"`Package` field in `DESCRIPTION` doesn't match folder name '{name}'"
47+
));
48+
}
49+
50+
let namespace = if namespace_path.is_file() {
51+
let namespace_contents = fs::read_to_string(&namespace_path)?;
52+
Namespace::parse(&namespace_contents)?
53+
} else {
54+
tracing::info!("Package `{name}` doesn't contain a NAMESPACE file, using defaults");
55+
Namespace::default()
56+
};
57+
58+
Ok(Some(Package {
59+
path: package_path,
60+
description,
61+
namespace,
62+
}))
63+
}
64+
}

0 commit comments

Comments
 (0)