From 691e216219e7dde1a40f484b13caba994eabbd47 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 15:11:24 +0200 Subject: [PATCH 01/13] Add `bquote()` refs test --- Cargo.lock | 2 + crates/oak_ide/Cargo.toml | 2 + .../oak_ide/tests/integration/base_sources.rs | 74 +++++++++++++++++++ .../tests/integration/find_references.rs | 25 +++++++ crates/oak_ide/tests/integration/main.rs | 1 + 5 files changed, 104 insertions(+) create mode 100644 crates/oak_ide/tests/integration/base_sources.rs diff --git a/Cargo.lock b/Cargo.lock index 9ba7b9a772..178c25b2af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2571,8 +2571,10 @@ dependencies = [ "oak_package_metadata", "oak_scan", "oak_semantic", + "oak_source", "salsa", "stdext", + "tempfile", "url", ] diff --git a/crates/oak_ide/Cargo.toml b/crates/oak_ide/Cargo.toml index 2df1e5b6dc..a18ce4873a 100644 --- a/crates/oak_ide/Cargo.toml +++ b/crates/oak_ide/Cargo.toml @@ -28,5 +28,7 @@ aether_path.workspace = true oak_package_metadata.workspace = true oak_scan.workspace = true oak_semantic = { workspace = true, features = ["testing"] } +oak_source.workspace = true salsa.workspace = true stdext.workspace = true +tempfile.workspace = true diff --git a/crates/oak_ide/tests/integration/base_sources.rs b/crates/oak_ide/tests/integration/base_sources.rs new file mode 100644 index 0000000000..c6a70eac8a --- /dev/null +++ b/crates/oak_ide/tests/integration/base_sources.rs @@ -0,0 +1,74 @@ +//! Regression test against the real, on-disk base R source cache. +//! +//! `Package::resolve()` has no `NAMESPACE` to gate `base` (see +//! `crates/oak_db/src/package_resolve.rs`), so resolving any bare name +//! against it sweeps every file in the package (around 160) and semantically +//! indexes each one. The other `oak_ide` tests install tiny synthetic +//! packages that can't exercise that sweep at the scale it actually runs at. +//! This test points at base's real source tree instead, fetched through the +//! same `oak_source` cache the kernel warms in the background, so a +//! regression that makes per-file indexing much more expensive shows up here +//! as a slow test rather than only as a slow LSP request. +//! +//! Ignored by default: the first run needs network access to populate the +//! shared cache under `/oak/source/v1/r/`. Later runs reuse it, so +//! run it explicitly with `cargo test -- --ignored` (or via `just test +//! --run-ignored all`) once that cache is warm. + +use std::fs; + +use oak_db::Db; +use oak_db::OakDatabase; +use oak_scan::DbScan; +use oak_source::SourceCache; + +use crate::support::offset; +use crate::support::range; +use crate::support::ranges; +use crate::support::upsert; + +const R_VERSION: &str = "4.5.2"; + +#[test] +fn test_bquote_hole_reference_against_real_base() { + let source = SourceCache::open().unwrap(); + let r_root = source + .get_r(R_VERSION) + .or_else(|| source.insert_r(R_VERSION)) + .expect("base R source archive unavailable"); + let base_r_dir = r_root.join("base").join("R"); + + let lib = tempfile::tempdir().unwrap(); + fs::create_dir_all(lib.path().join("base")).unwrap(); + fs::write( + lib.path().join("base").join("DESCRIPTION"), + format!("Package: base\nVersion: {R_VERSION}\n"), + ) + .unwrap(); + + let mut db = OakDatabase::new(); + db.set_library_paths(&[lib.path().to_path_buf()]); + let base = db.package_by_name("base").unwrap(); + db.set_package_sources(base, &base_r_dir); + + // Same repro as `find_references::test_bquote_hole_nested_in_quoted_call`, + // but `bquote` now resolves through the real `base` package instead of no + // package at all, so this forces the full-package sweep. + let script = "foo <- function() {}\nbquote(foo(.(foo)))\n"; + let file = upsert(&mut db, "test.R", script); + + let hole_use = script.rfind("foo").unwrap() as u32; + let refs = oak_ide::find_references(&db, file, offset(0), true); + assert_eq!(ranges(&refs), vec![ + range(0, 3), + range(hole_use, hole_use + 3) + ]); + + // `bquote` itself isn't locally scoped, so it resolves through the + // `base` import layer and forces `Package::resolve()` to sweep every + // file in the package looking for a top-level `bquote` binding. Its own + // (library, excluded) definition site doesn't come back, just the call. + let bquote_offset = script.find("bquote").unwrap() as u32; + let refs = oak_ide::find_references(&db, file, offset(bquote_offset), true); + assert_eq!(ranges(&refs), vec![range(bquote_offset, bquote_offset + 6)]); +} diff --git a/crates/oak_ide/tests/integration/find_references.rs b/crates/oak_ide/tests/integration/find_references.rs index 15958f3581..aa5277106d 100644 --- a/crates/oak_ide/tests/integration/find_references.rs +++ b/crates/oak_ide/tests/integration/find_references.rs @@ -214,6 +214,31 @@ fn test_on_exit_body_use_has_lazy_view() { ]); } +#[test] +fn test_bquote_hole_nested_in_quoted_call() { + // `foo(...)` is itself inside the quoted `expr`, so its call name is not a + // live use; only the escaped `.(foo)` argument is. Regression test for a + // reported LSP crash on this shape; it does not reproduce at this layer. + let source = "foo <- function() {}\nbquote(foo(.(foo)))\n"; + let mut db = OakDatabase::new(); + let file = upsert(&mut db, "test.R", source); + + let hole_use = source.rfind("foo").unwrap() as u32; + + let refs = find_references(&db, file, offset(0), true); + assert_eq!(ranges(&refs), vec![ + range(0, 3), + range(hole_use, hole_use + 3) + ]); + + // Same result when the cursor starts on the hole's `foo` itself. + let refs = find_references(&db, file, offset(hole_use), true); + assert_eq!(ranges(&refs), vec![ + range(0, 3), + range(hole_use, hole_use + 3) + ]); +} + // --- Boundary cursor --- #[test] diff --git a/crates/oak_ide/tests/integration/main.rs b/crates/oak_ide/tests/integration/main.rs index 7b541187ac..6cddf5b5ed 100644 --- a/crates/oak_ide/tests/integration/main.rs +++ b/crates/oak_ide/tests/integration/main.rs @@ -1,5 +1,6 @@ mod support; +mod base_sources; mod find_references; mod goto_definition; mod rename; From dce4a74dcabb7134c84d729e5ea3623282742b33 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 15:29:33 +0200 Subject: [PATCH 02/13] Flush after logging on segfault --- crates/ark/src/traps.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ark/src/traps.rs b/crates/ark/src/traps.rs index 3fa5e94d52..16b51e4950 100644 --- a/crates/ark/src/traps.rs +++ b/crates/ark/src/traps.rs @@ -39,4 +39,7 @@ pub extern "C-unwind" fn backtrace_handler(signum: libc::c_int) { // capture the current thread's backtrace let bt = std::backtrace::Backtrace::force_capture(); log::error!("{}\n{}", header, bt); + + log::logger().flush(); + std::thread::sleep(std::time::Duration::from_millis(250)); } From 564917ac66970b0f3f2f30fcb120479b7a85dcf0 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 16:45:52 +0200 Subject: [PATCH 03/13] Fix logging in internal crates --- crates/ark/build.rs | 19 +++++++++++++++++++ crates/ark/src/logger.rs | 31 ++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/ark/build.rs b/crates/ark/build.rs index 17c1991f89..ecf8c003ff 100644 --- a/crates/ark/build.rs +++ b/crates/ark/build.rs @@ -17,6 +17,25 @@ fn main() { println!("cargo:rerun-if-changed=src"); println!("cargo:rerun-if-changed=resources"); + // Every first-party crate directory name. Used by `logger::internal_crates()` + // to `ark`'s `RUST_LOG` level to all our crates. + let workspace_crates_dir = Path::new(&std::env::var("CARGO_MANIFEST_DIR").unwrap()) + .join("..") + .canonicalize() + .unwrap(); + println!("cargo:rerun-if-changed={}", workspace_crates_dir.display()); + + let internal_crates: Vec = std::fs::read_dir(&workspace_crates_dir) + .unwrap() + .filter_map(|entry| entry.ok()) + .filter(|entry| entry.path().is_dir()) + .filter_map(|entry| entry.file_name().into_string().ok()) + .collect(); + println!( + "cargo:rustc-env=ARK_INTERNAL_CRATES={}", + internal_crates.join(",") + ); + // Attempt to use `git rev-parse HEAD` to get the current git hash. If this // fails, we'll just use the string "" to indicate that the git hash // could not be determined.. diff --git a/crates/ark/src/logger.rs b/crates/ark/src/logger.rs index 6b0cc054ef..f45620a9ec 100644 --- a/crates/ark/src/logger.rs +++ b/crates/ark/src/logger.rs @@ -18,6 +18,13 @@ use tracing_subscriber::Layer; use crate::logger_hprof; +/// Every first-party crate's directory name under `crates/`, baked in by +/// `build.rs` so [`internal_crates()`] doesn't need a hardcoded list that +/// goes stale as crates are added. +fn internal_crates() -> impl Iterator { + env!("ARK_INTERNAL_CRATES").split(',') +} + pub fn init(log_file: Option<&str>, profile_file: Option<&str>) { static ONCE: Once = Once::new(); @@ -35,7 +42,7 @@ pub fn init(log_file: Option<&str>, profile_file: Option<&str>) { .and_then(|c| c.get(1)) .map(|c| c.as_str()) { - for pkg in ["amalthea", "harp", "stdext"] { + for pkg in internal_crates().filter(|&pkg| pkg != "ark") { if let Ok(directive) = format!("{pkg}={level}").parse() { env_filter = env_filter.add_directive(directive); } @@ -111,3 +118,25 @@ fn non_blocking(file: Option<&str>, cell: &OnceCell) -> BoxMakeWrit BoxMakeWriter::new(std::io::stderr) } } + +#[cfg(test)] +mod tests { + use super::internal_crates; + + #[test] + fn test_internal_crates_includes_path_crates() { + let crates: Vec<&str> = internal_crates().collect(); + assert!(crates.contains(&"oak_db")); + assert!(crates.contains(&"oak_semantic")); + assert!(crates.contains(&"amalthea")); + assert!(crates.contains(&"harp")); + assert!(crates.contains(&"stdext")); + } + + #[test] + fn test_internal_crates_excludes_git_dependencies() { + let crates: Vec<&str> = internal_crates().collect(); + assert!(!crates.contains(&"aether_factory")); + assert!(!crates.contains(&"aether_syntax")); + } +} From be5ee8763429c9548c500dd23c648e36fb521ea7 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 19:08:48 +0200 Subject: [PATCH 04/13] Protect against stack overflows --- Cargo.lock | 33 +++++++ Cargo.toml | 1 + crates/oak_db/Cargo.toml | 1 + crates/oak_db/src/file.rs | 60 ++++++++++++ .../oak_ide/tests/integration/base_sources.rs | 92 ++++++++++++------- 5 files changed, 156 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 178c25b2af..b7eaa2dada 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -388,6 +388,15 @@ version = "1.0.103" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" +[[package]] +name = "ar_archive_writer" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4087686b4b0a3427190bae57a1d9a478dbb2d40c5dc1bd6e2b6d797913bdd348" +dependencies = [ + "object", +] + [[package]] name = "ark" version = "0.1.252" @@ -2552,6 +2561,7 @@ dependencies = [ "rustc-hash", "salsa", "salsa-macros", + "stacker", "stdext", "tempfile", "url", @@ -2897,6 +2907,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "psm" +version = "0.1.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "645dbe486e346d9b5de3ef16ede18c26e6c70ad97418f4874b8b1889d6e761ea" +dependencies = [ + "ar_archive_writer", + "cc", +] + [[package]] name = "quote" version = "1.0.46" @@ -3620,6 +3640,19 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" +[[package]] +name = "stacker" +version = "0.1.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "640c8cdd92b6b12f5bcb1803ca3bbf5ab96e5e6b6b96b9ab77dabe9e880b3190" +dependencies = [ + "cc", + "cfg-if", + "libc", + "psm", + "windows-sys 0.61.2", +] + [[package]] name = "static_assertions" version = "1.1.0" diff --git a/Cargo.toml b/Cargo.toml index 3338a7f440..ecd27cf3d9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -111,6 +111,7 @@ serde_with = "3.19.0" sha2 = "0.11.0" smallvec = "1.15.1" socket2 = "0.6.3" +stacker = "0.1.21" stdext = { path = "crates/stdext" } streaming-iterator = "0.1.9" strum = "0.28.0" diff --git a/crates/oak_db/Cargo.toml b/crates/oak_db/Cargo.toml index 6b9423326d..15d2dfd6c1 100644 --- a/crates/oak_db/Cargo.toml +++ b/crates/oak_db/Cargo.toml @@ -29,6 +29,7 @@ oak_semantic = { workspace = true, features = ["salsa"] } rustc-hash.workspace = true salsa.workspace = true salsa-macros.workspace = true +stacker.workspace = true stdext.workspace = true url.workspace = true diff --git a/crates/oak_db/src/file.rs b/crates/oak_db/src/file.rs index 04205f27f8..389583c20b 100644 --- a/crates/oak_db/src/file.rs +++ b/crates/oak_db/src/file.rs @@ -317,7 +317,27 @@ fn root_by_path(db: &dyn Db, path: &FilePath) -> Option { .map(|(_, r)| r) } +/// Guard against stack overflow when `semantic_index` recurses across files. +const STACK_RED_ZONE: usize = 1024 * 1024; +const STACK_GROW_BY: usize = 8 * 1024 * 1024; + fn build_semantic_index(file: File, db: &dyn Db) -> SemanticIndex { + #[cfg(test)] + let _depth = recursion_depth::enter(); + + if matches!(stacker::remaining_stack(), Some(left) if left < STACK_RED_ZONE) { + log::trace!( + "Deep cross-file recursion building semantic index for {}, growing the stack", + file.path(db) + ); + } + + stacker::maybe_grow(STACK_RED_ZONE, STACK_GROW_BY, || { + build_semantic_index_inner(file, db) + }) +} + +fn build_semantic_index_inner(file: File, db: &dyn Db) -> SemanticIndex { let parsed = file.parse(db); let resolver = SalsaImportsResolver::new(db, file); let index = oak_semantic::build_index(&parsed.tree(), resolver); @@ -369,3 +389,43 @@ fn semantic_index_cycle_result(db: &dyn Db, _id: salsa::Id, file: File) -> Seman let parsed = file.parse(db); oak_semantic::build_index(&parsed.tree(), oak_semantic::NoopImportsResolver) } + +/// Test-only recorder for the deepest `build_semantic_index` nesting. +#[cfg(test)] +pub(crate) mod recursion_depth { + use std::cell::Cell; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; + + thread_local! { + static CURRENT: Cell = const { Cell::new(0) }; + } + static MAX: AtomicUsize = AtomicUsize::new(0); + + pub(crate) fn reset() { + MAX.store(0, Ordering::Relaxed); + } + pub(crate) fn max() -> usize { + MAX.load(Ordering::Relaxed) + } + + /// Bumps the running depth on entry and records the peak. The returned guard + /// decrements on drop. + pub(crate) fn enter() -> Guard { + let depth = CURRENT.with(|current| { + let depth = current.get() + 1; + current.set(depth); + depth + }); + MAX.fetch_max(depth, Ordering::Relaxed); + Guard + } + + pub(crate) struct Guard; + + impl Drop for Guard { + fn drop(&mut self) { + CURRENT.with(|current| current.set(current.get() - 1)); + } + } +} diff --git a/crates/oak_ide/tests/integration/base_sources.rs b/crates/oak_ide/tests/integration/base_sources.rs index c6a70eac8a..1629e7c1b8 100644 --- a/crates/oak_ide/tests/integration/base_sources.rs +++ b/crates/oak_ide/tests/integration/base_sources.rs @@ -1,21 +1,32 @@ //! Regression test against the real, on-disk base R source cache. //! -//! `Package::resolve()` has no `NAMESPACE` to gate `base` (see -//! `crates/oak_db/src/package_resolve.rs`), so resolving any bare name -//! against it sweeps every file in the package (around 160) and semantically -//! indexes each one. The other `oak_ide` tests install tiny synthetic -//! packages that can't exercise that sweep at the scale it actually runs at. -//! This test points at base's real source tree instead, fetched through the -//! same `oak_source` cache the kernel warms in the background, so a -//! regression that makes per-file indexing much more expensive shows up here -//! as a slow test rather than only as a slow LSP request. +//! `Package::resolve()` has no `NAMESPACE` to gate a base-priority package +//! (see `crates/oak_db/src/package_resolve.rs`), so resolving any bare name +//! against one sweeps every file in it and semantically indexes each one. +//! `File::resolve()` walks the default search path in priority order +//! (`stats, graphics, grDevices, utils, datasets, methods, base`, see +//! `crates/oak_db/src/search.rs`) and stops at the first package that binds +//! the name, but only after that package's full sweep completes. `bquote` +//! only lives in `base`, the *last* layer, so resolving it sweeps all six +//! other default packages first. //! -//! Ignored by default: the first run needs network access to populate the -//! shared cache under `/oak/source/v1/r/`. Later runs reuse it, so -//! run it explicitly with `cargo test -- --ignored` (or via `just test -//! --run-ignored all`) once that cache is warm. +//! The other `oak_ide` tests install tiny synthetic packages that can't +//! exercise that walk at the scale it actually runs at. This test points at +//! every default-search-path package's real source tree instead, fetched +//! through the same `oak_source` cache the kernel warms in the background, +//! so a regression that makes the walk much more expensive shows up here as +//! a slow test rather than only as a slow LSP request. +//! +//! The first run needs network access to populate the shared cache under +//! `/oak/source/v1/r/`. Later runs reuse it. +//! +//! `test_bquote_hole_reference_against_real_search_path` is `#[ignore]`d: it +//! reproduces an open bug (the walk hangs well past a minute rather than +//! finishing in the few seconds file count alone would predict) rather than +//! asserting a fix, so it can't run unattended in normal `just test`. use std::fs; +use std::path::Path; use oak_db::Db; use oak_db::OakDatabase; @@ -29,31 +40,49 @@ use crate::support::upsert; const R_VERSION: &str = "4.5.2"; +/// R's default search path, `stats` (highest priority) through `base` +/// (lowest), see `crate::search::DEFAULT_SEARCH_PATH_PACKAGES` in `oak_db`. +const DEFAULT_SEARCH_PATH_PACKAGES: [&str; 7] = + ["stats", "graphics", "grDevices", "utils", "datasets", "methods", "base"]; + +/// Register every default-search-path package as a library package backed by +/// its real source directory under `r_root`. +fn install_default_search_path(db: &mut OakDatabase, r_root: &Path) { + let lib = tempfile::tempdir().unwrap(); + for name in DEFAULT_SEARCH_PATH_PACKAGES { + fs::create_dir_all(lib.path().join(name)).unwrap(); + fs::write( + lib.path().join(name).join("DESCRIPTION"), + format!("Package: {name}\nVersion: {R_VERSION}\n"), + ) + .unwrap(); + } + // `set_library_paths` only reads `lib` once, up front, to discover + // package directories, so it's fine to drop the `TempDir` handle (and + // its auto-cleanup) right after. + db.set_library_paths(&[lib.keep()]); + + for name in DEFAULT_SEARCH_PATH_PACKAGES { + let pkg = db.package_by_name(name).unwrap(); + db.set_package_sources(pkg, &r_root.join(name).join("R")); + } +} + #[test] -fn test_bquote_hole_reference_against_real_base() { +#[ignore] +fn test_bquote_hole_reference_against_real_search_path() { let source = SourceCache::open().unwrap(); let r_root = source .get_r(R_VERSION) .or_else(|| source.insert_r(R_VERSION)) .expect("base R source archive unavailable"); - let base_r_dir = r_root.join("base").join("R"); - - let lib = tempfile::tempdir().unwrap(); - fs::create_dir_all(lib.path().join("base")).unwrap(); - fs::write( - lib.path().join("base").join("DESCRIPTION"), - format!("Package: base\nVersion: {R_VERSION}\n"), - ) - .unwrap(); let mut db = OakDatabase::new(); - db.set_library_paths(&[lib.path().to_path_buf()]); - let base = db.package_by_name("base").unwrap(); - db.set_package_sources(base, &base_r_dir); + install_default_search_path(&mut db, &r_root); // Same repro as `find_references::test_bquote_hole_nested_in_quoted_call`, - // but `bquote` now resolves through the real `base` package instead of no - // package at all, so this forces the full-package sweep. + // but `bquote` now resolves through the real default search path instead + // of no package at all, so this forces the full walk-and-sweep. let script = "foo <- function() {}\nbquote(foo(.(foo)))\n"; let file = upsert(&mut db, "test.R", script); @@ -65,9 +94,10 @@ fn test_bquote_hole_reference_against_real_base() { ]); // `bquote` itself isn't locally scoped, so it resolves through the - // `base` import layer and forces `Package::resolve()` to sweep every - // file in the package looking for a top-level `bquote` binding. Its own - // (library, excluded) definition site doesn't come back, just the call. + // default search path, sweeping `stats`, `graphics`, `grDevices`, + // `utils`, `datasets`, and `methods` (all empty) before reaching `base`. + // Its own (library, excluded) definition site doesn't come back, just + // the call. let bquote_offset = script.find("bquote").unwrap() as u32; let refs = oak_ide::find_references(&db, file, offset(bquote_offset), true); assert_eq!(ranges(&refs), vec![range(bquote_offset, bquote_offset + 6)]); From d0c79051a8f2ca1c5f01ee3bc5c22a8e0b77011f Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 19:12:57 +0200 Subject: [PATCH 05/13] Make `cross_file_layers()` a cached query --- crates/oak_db/src/file_imports.rs | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/crates/oak_db/src/file_imports.rs b/crates/oak_db/src/file_imports.rs index 1e0d3608fb..97fb0a5042 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -46,6 +46,7 @@ pub enum ImportLayer { /// flow-ordered set. /// /// [`SalsaImportsResolver`]: crate::imports::SalsaImportsResolver +#[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] pub(crate) struct CrossFileLayers { pub above: Vec, pub below: Vec, @@ -55,17 +56,19 @@ impl CrossFileLayers { /// Flatten to a single lookup-ordered layer list, splicing the file's own /// `library()` attaches into the band between the definition/namespace /// layers (which outrank them) and the rest of the search path. - pub(crate) fn splice_own_attaches(self, own: Vec) -> Vec { - let CrossFileLayers { mut above, below } = self; - above.reserve(own.len() + below.len()); - above.extend(own); - above.extend(below); - above + pub(crate) fn splice_own_attaches(&self, own: Vec) -> Vec { + let mut out = Vec::with_capacity(self.above.len() + own.len() + self.below.len()); + + out.extend(self.above.iter().cloned()); + out.extend(own); + out.extend(self.below.iter().cloned()); + + out } } /// The point in a package's load at which a file views its collation siblings. -#[derive(Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub(crate) enum CollationView { /// Deferred (a function body, or end-of-file): the code runs after the /// whole collation has loaded, so every sibling is visible. @@ -171,6 +174,12 @@ impl File { /// The cross-file layers this file sees at load time, excluding its own /// attaches (see [`CrossFileLayers`]). Never reads the file's own semantic /// index, so it's safe to call while that index is being built. + /// + /// Tracked and keyed on `(self, view)`. Resolving one file's effects reads + /// this once per annotated call, and each rebuild walks every collation + /// predecessor's `attached_packages`, so recomputing it per call would be + /// O(predecessors) each time. + #[salsa::tracked(returns(ref))] pub(crate) fn cross_file_layers(self, db: &dyn Db, view: CollationView) -> CrossFileLayers { match self.package(db) { // A `tests/testthat/` file: sees the whole package plus sourced From 9ed30deff4ef890c41a89302ec07f2b8d4a0170c Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 18:51:15 +0200 Subject: [PATCH 06/13] Warm up attached packages --- crates/oak_db/src/file_imports.rs | 10 +++++ crates/oak_db/src/tests/file_imports.rs | 51 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/crates/oak_db/src/file_imports.rs b/crates/oak_db/src/file_imports.rs index 97fb0a5042..3301df4713 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -312,6 +312,16 @@ fn testthat_load_layers(file: File, db: &dyn Db, package: Package) -> CrossFileL /// first. Reads each file's `attached_packages`, never the caller's own index. /// An attach to a package absent from every root is dropped (no entity). fn predecessor_attach_layers(db: &dyn Db, files: &[File]) -> Vec { + // Warm the indices in forward load order before the LIFO pass below. + // `files` is LIFO (latest predecessor first). Reading attaches in LIFO + // order demands the latest predecessor's index first, which recursively + // demands its own predecessors, and so on. To reduce stack depth, query + // attached packages in the reverse order so that each file's own + // predecessors are already built when its turn comes in the LIFO pass. + for file in files.iter().rev() { + file.attached_packages(db); + } + files .iter() .flat_map(|file| { diff --git a/crates/oak_db/src/tests/file_imports.rs b/crates/oak_db/src/tests/file_imports.rs index 472e7d2c55..36a5a05f64 100644 --- a/crates/oak_db/src/tests/file_imports.rs +++ b/crates/oak_db/src/tests/file_imports.rs @@ -4,6 +4,7 @@ use salsa::Setter; use crate::tests::test_db::file_path; use crate::tests::test_db::library_root; +use crate::tests::test_db::make_package; use crate::tests::test_db::workspace_root; use crate::tests::test_db::TestDb; use crate::DbInputs; @@ -558,3 +559,53 @@ fn test_imports_is_cached_per_file() { assert_eq!(db.executions("imports"), 1); } + +#[test] +fn test_cross_file_layers_cascade_stays_shallow() { + // Building a late-collation file resolves its effects, which reaches back + // through every predecessor's index. Without the forward-prime in + // `predecessor_attach_layers` that walk recurses as deep as the collation, + // which is what overflowed the stack in the IDE. Assert the nesting stays + // flat regardless of collation length. + // + // We assert on recursion depth, not "does it crash", because + // `stacker::maybe_grow` in `build_semantic_index` grows the stack and would + // hide a regression from a crash-based check. Depth is independent of it. + let mut db = TestDb::new(); + + const N: usize = 150; + let owned: Vec<(String, String)> = (0..N) + .map(|i| (format!("w/pkg/R/f{i:04}.R"), "local(1)\n".to_string())) + .collect(); + let files: Vec<(&str, &str)> = owned + .iter() + .map(|(path, contents)| (path.as_str(), contents.as_str())) + .collect(); + let (_pkg, entities) = make_package(&mut db, "pkg", Namespace::default(), &files); + + crate::file::recursion_depth::reset(); + + // Cold build of the last collation file: its effect resolution demands + // every predecessor's index. Forward-priming keeps the nesting shallow. + let _ = entities[N - 1].semantic_index(&db); + + assert!(crate::file::recursion_depth::max() < 10); +} + +#[test] +fn test_cross_file_layers_memoized_across_effect_calls() { + // Each effectful call consults `cross_file_layers(file, view)` while the + // file's index builds. Memoizing it keeps that to one execution per file + // instead of one per call, which was the O(N^2) that froze the LSP. + let mut db = TestDb::new(); + + let body = "local(1)\n".repeat(20); + let (_pkg, entities) = make_package(&mut db, "pkg", Namespace::default(), &[( + "w/pkg/R/a.R", + &body, + )]); + + let _ = entities[0].semantic_index(&db); + + assert_eq!(db.executions("cross_file_layers"), 1); +} From 3ebeafbed43d42a6e513ca962eebb0b121c5e110 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 21:19:33 +0200 Subject: [PATCH 07/13] Add `all_used_files()` for actual dependencies --- crates/oak_db/src/db.rs | 80 +++++++++++++++----- crates/oak_db/src/lib.rs | 3 +- crates/oak_db/src/tests/db.rs | 69 +++++++++++++++++ crates/oak_ide/src/find_references.rs | 7 +- crates/oak_scan/tests/integration/library.rs | 4 +- 5 files changed, 139 insertions(+), 24 deletions(-) diff --git a/crates/oak_db/src/db.rs b/crates/oak_db/src/db.rs index a47f0522e1..081f40f44b 100644 --- a/crates/oak_db/src/db.rs +++ b/crates/oak_db/src/db.rs @@ -104,36 +104,57 @@ pub(crate) fn live_roots_query(db: &dyn Db) -> Vec { roots } -/// All files known to the database, in stable order (workspace, library, orphan). -/// -/// Used as the workspace-wide candidate pool for find-references: callers -/// apply a textual name filter before building indexes. +/// Files reachable from workspace roots and orphan buffers, plus library +/// files that belong to an actual workspace dependency (see +/// [`crate::all_package_dependencies`]). /// -/// Nested roots overlap on disk, so the same `File` is reachable from several -/// roots (open `/proj` and `/proj/sub-pkg` and the outer scan walks into -/// `sub-pkg`). A `seen` set drops the repeats. Unlike `root_by_package` / -/// `root_by_file`, this query exposes no ownership, just a flat set, so it -/// doesn't matter which root a duplicate is attributed to. We keep the first -/// occurrence, which preserves the traversal order above. +/// Same as [`all_known_files`] but excluding library packages that aren't a +/// dependency (direct or indirect) of the workspace. #[salsa::tracked(returns(ref))] -pub fn all_files(db: &dyn Db) -> Vec { +pub fn all_used_files(db: &dyn Db) -> Vec { + let dependencies: FxHashSet = crate::workspace::all_package_dependencies(db) + .iter() + .copied() + .collect(); + let mut seen = FxHashSet::default(); let mut files = Vec::new(); for &root in db.live_roots() { match root { - LiveRoot::Workspace(r) | LiveRoot::Library(r) => { - let root_files = r.scripts(db).iter().chain( - r.packages(db) - .iter() - .flat_map(|&pkg| pkg.files(db).iter().chain(pkg.scripts(db))), - ); - for &file in root_files { + LiveRoot::Workspace(root) => push_root_files(db, &mut files, &mut seen, root, None), + LiveRoot::Library(root) => { + push_root_files(db, &mut files, &mut seen, root, Some(&dependencies)) + }, + LiveRoot::Orphan(orphan) => { + for &file in orphan.files(db) { if seen.insert(file) { files.push(file); } } }, + } + } + + files +} + +/// All files known to the database, in stable order (workspace, library, orphan). +/// +/// Note that this also contains files from all installed packages, _including +/// those that are not dependencies of the workspace_. Only use this for very +/// wide searches. LSP functionality should generally not depend on +/// non-dependencies, prefer `all_dependency_files()` instead. +#[salsa::tracked(returns(ref))] +pub fn all_known_files(db: &dyn Db) -> Vec { + let mut seen = FxHashSet::default(); + let mut files = Vec::new(); + + for &root in db.live_roots() { + match root { + LiveRoot::Workspace(root) | LiveRoot::Library(root) => { + push_root_files(db, &mut files, &mut seen, root, None) + }, LiveRoot::Orphan(orphan) => { for &file in orphan.files(db) { if seen.insert(file) { @@ -147,6 +168,29 @@ pub fn all_files(db: &dyn Db) -> Vec { files } +/// Pushes `root`'s scripts and package files into `files`, skipping ones +/// already in `seen`. When `dependencies` is `Some`, packages not in that set +/// are skipped entirely. +fn push_root_files( + db: &dyn Db, + files: &mut Vec, + seen: &mut FxHashSet, + root: Root, + dependencies: Option<&FxHashSet>, +) { + let root_files = root.scripts(db).iter().chain( + root.packages(db) + .iter() + .filter(|&&pkg| dependencies.is_none_or(|deps| deps.contains(&pkg))) + .flat_map(|&pkg| pkg.files(db).iter().chain(pkg.scripts(db))), + ); + for &file in root_files { + if seen.insert(file) { + files.push(file); + } + } +} + /// Files eligible for the workspace symbol index: workspace-root scripts and /// package files, plus orphan editor buffers. Library roots are excluded, so /// installed package symbols don't leak into e.g. workspace symbols. diff --git a/crates/oak_db/src/lib.rs b/crates/oak_db/src/lib.rs index 4d4e6ccdc2..608f76b069 100644 --- a/crates/oak_db/src/lib.rs +++ b/crates/oak_db/src/lib.rs @@ -19,7 +19,8 @@ mod workspace; #[cfg(test)] mod tests; -pub use db::all_files; +pub use db::all_known_files; +pub use db::all_used_files; pub use db::workspace_files; pub use db::Db; pub use db::DbInputs; diff --git a/crates/oak_db/src/tests/db.rs b/crates/oak_db/src/tests/db.rs index 5247b37d64..d8f15654f0 100644 --- a/crates/oak_db/src/tests/db.rs +++ b/crates/oak_db/src/tests/db.rs @@ -2,6 +2,8 @@ use std::collections::HashSet; use salsa::Setter; +use crate::all_known_files; +use crate::all_used_files; use crate::tests::test_db::file_path; use crate::tests::test_db::library_root; use crate::tests::test_db::workspace_root; @@ -149,3 +151,70 @@ fn test_root_path_index_invalidates_per_root() { let _ = db.file_by_path(&file_path("b/file.R")); assert_eq!(db.executions("root_path_index"), 3); } + +#[test] +fn test_all_used_files_excludes_unrelated_library_files() { + let mut db = TestDb::new(); + + let lib = library_root(&db, "libs"); + let used_pkg = Package::new( + &db, + file_path("libs/used/DESCRIPTION"), + "used".to_string(), + FileRevision::zero(), + FileRevision::zero(), + None, + None, + vec![], + Vec::new(), + ); + let used_file = File::new( + &db, + file_path("libs/used/R/a.R"), + FileRevision::zero(), + None, + Some(used_pkg), + ); + used_pkg.set_files(&mut db).to(vec![used_file]); + + let unused_pkg = Package::new( + &db, + file_path("libs/unused/DESCRIPTION"), + "unused".to_string(), + FileRevision::zero(), + FileRevision::zero(), + None, + None, + vec![], + Vec::new(), + ); + let unused_file = File::new( + &db, + file_path("libs/unused/R/a.R"), + FileRevision::zero(), + None, + Some(unused_pkg), + ); + unused_pkg.set_files(&mut db).to(vec![unused_file]); + + lib.set_packages(&mut db).to(vec![used_pkg, unused_pkg]); + db.library_roots().set_roots(&mut db).to(vec![lib]); + + let root = workspace_root(&db, "proj"); + let script = File::new( + &db, + file_path("proj/script.R"), + FileRevision::zero(), + Some("library(used)\n".to_string()), + None, + ); + root.set_scripts(&mut db).to(vec![script]); + db.workspace_roots().set_roots(&mut db).to(vec![root]); + + // `all_known_files` doesn't know about dependencies, so both installed + // packages show up, referenced or not. + assert_eq!(all_known_files(&db), &vec![script, used_file, unused_file]); + + // `all_used_files` drops `unused`: nothing in the workspace references it. + assert_eq!(all_used_files(&db), &vec![script, used_file]); +} diff --git a/crates/oak_ide/src/find_references.rs b/crates/oak_ide/src/find_references.rs index 999c9705e4..b650f15532 100644 --- a/crates/oak_ide/src/find_references.rs +++ b/crates/oak_ide/src/find_references.rs @@ -1,7 +1,7 @@ use std::collections::HashSet; use biome_rowan::TextSize; -use oak_db::all_files; +use oak_db::all_used_files; use oak_db::Db; use oak_db::Definition; use oak_db::File; @@ -226,9 +226,10 @@ fn find_namespace_references<'db>( results } -/// Every db file whose contents mention `text`. +/// Every db file whose contents mention `text`, scoped to the workspace plus +/// its actual dependencies (not every package under `.libPaths()`). fn all_matching_files(db: &dyn Db, text: &str) -> Vec { - all_files(db) + all_used_files(db) .iter() .filter(|&&f| f.source_text(db).contains(text)) .copied() diff --git a/crates/oak_scan/tests/integration/library.rs b/crates/oak_scan/tests/integration/library.rs index 777b2cadd1..2f60a7e180 100644 --- a/crates/oak_scan/tests/integration/library.rs +++ b/crates/oak_scan/tests/integration/library.rs @@ -497,7 +497,7 @@ fn test_set_package_shorter_root_does_not_steal_from_longer() { #[test] fn test_all_files_emits_shared_file_once_under_deepest_root() { - use oak_db::all_files; + use oak_db::all_known_files; use oak_scan::FileEntry; let mut db = OakDatabase::new(); @@ -539,7 +539,7 @@ fn test_all_files_emits_shared_file_once_under_deepest_root() { let file = p1.files(&db)[0]; assert_eq!(file.root(&db), Some(long)); - assert_eq!(all_files(&db), &vec![file]); + assert_eq!(all_known_files(&db), &vec![file]); } #[test] From 13759af9ea124062a97fbc8a3bff211db1daa109 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 24 Jul 2026 21:19:46 +0200 Subject: [PATCH 08/13] Warm up indexes for all used files --- crates/ark/src/lsp/analysis.rs | 1 + crates/ark/src/lsp/analysis/warmup.rs | 21 +++++++++++++++++++++ crates/ark/src/lsp/main_loop.rs | 17 +++++++++++++---- crates/oak_db/src/file.rs | 12 ++++++++++++ crates/oak_db/src/file_imports.rs | 2 +- crates/oak_db/src/lib.rs | 1 + 6 files changed, 49 insertions(+), 5 deletions(-) diff --git a/crates/ark/src/lsp/analysis.rs b/crates/ark/src/lsp/analysis.rs index 6a372852ea..2e2877acf1 100644 --- a/crates/ark/src/lsp/analysis.rs +++ b/crates/ark/src/lsp/analysis.rs @@ -22,6 +22,7 @@ pub(crate) use pool::AnalysisPool; pub(crate) use refresh::DiagnosticsReady; pub(crate) use refresh::DiagnosticsState; pub(crate) use snapshot::WorldStateSnapshot; +pub(crate) use warmup::warm_semantic_indexes; pub(crate) use warmup::warm_workspace_index; /// Run `f`, swallowing a salsa cancellation as `None`. Any other panic propagates. diff --git a/crates/ark/src/lsp/analysis/warmup.rs b/crates/ark/src/lsp/analysis/warmup.rs index 38466b415c..7772bfd4fe 100644 --- a/crates/ark/src/lsp/analysis/warmup.rs +++ b/crates/ark/src/lsp/analysis/warmup.rs @@ -5,6 +5,9 @@ // // +use oak_db::all_used_files; +use oak_db::warm_file; + use super::pool::AnalysisPool; use crate::lsp; use crate::lsp::indexer; @@ -30,3 +33,21 @@ pub(crate) fn warm_workspace_index(state: &WorldState, pool: &AnalysisPool) { lsp::log_info!("Finished workspace index warmup ({:.0?})", now.elapsed()); }) } + +/// Warm the oak `semantic_index` of every file the workspace depends on, on a +/// background thread. +/// +/// Idempotent, so re-running on every revision is cheap once a file's index is +/// already warm (salsa cache hit). A concurrent write just cancels the +/// in-flight warm; the next revision re-runs it, which is what carries warmup +/// through the startup write-storm and warms a freshly-typed `pkg::` +/// dependency as soon as its sources land. +pub(crate) fn warm_semantic_indexes(state: &WorldState, pool: &AnalysisPool) { + pool.spawn(state.snapshot(), |snapshot| { + let now = std::time::Instant::now(); + for &file in all_used_files(snapshot.db()) { + warm_file(snapshot.db(), file); + } + lsp::log_info!("Warmed semantic indexes ({:.0?})", now.elapsed()); + }) +} diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index 91ce3fdcec..815ebd9d1e 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -571,10 +571,9 @@ impl GlobalState { dispatch_scan_requests(&self.lsp_state.scan_pool, &self.events_tx, followups); - // Warm the workspace index once the scan settles. Editor - // writes don't need to re-warm: they imply an open document, - // and the diagnostics passes they trigger force the same - // memos. + // Warm the workspace symbol index once the scan settles. The + // oak semantic index is warmed separately on every revision (see + // the revision-advanced block below). if !self.lsp_state.oak_scheduler.has_pending_scans() { analysis::warm_workspace_index(&self.world, &self.lsp_state.analysis_pool); } @@ -631,6 +630,16 @@ impl GlobalState { &self.lsp_state.source_pool, &self.events_tx, ); + + // Re-warm the oak semantic indexes on every revision, counting on + // idempotence (warm files are salsa cache hits). Runs on a snapshot, + // so a concurrent write just cancels the in-flight warm and the next + // revision re-runs it. This is what carries warmup through the + // startup write-storm (each source/edit cancels the previous warm, + // and the revision after the storm settles completes it), and it + // means a freshly-typed `pkg::` dependency is warmed as soon as its + // sources land and bump the revision. + analysis::warm_semantic_indexes(&self.world, &self.lsp_state.analysis_pool); } Ok(()) diff --git a/crates/oak_db/src/file.rs b/crates/oak_db/src/file.rs index 389583c20b..ad05875765 100644 --- a/crates/oak_db/src/file.rs +++ b/crates/oak_db/src/file.rs @@ -317,11 +317,23 @@ fn root_by_path(db: &dyn Db, path: &FilePath) -> Option { .map(|(_, r)| r) } +/// Warm the tracked queries an LSP request reads on `file`, so the first +/// request after a scan doesn't pay the cold build. +/// +/// Computing `imports()` builds the file's `semantic_index` and its cross-file +/// import view in one go; the file's collation predecessors get pulled in (and +/// primed shallow) as a side effect. Best-effort, meant to run off the request +/// thread once a scan settles. +pub fn warm_file(db: &dyn Db, file: File) { + file.imports(db); +} + /// Guard against stack overflow when `semantic_index` recurses across files. const STACK_RED_ZONE: usize = 1024 * 1024; const STACK_GROW_BY: usize = 8 * 1024 * 1024; fn build_semantic_index(file: File, db: &dyn Db) -> SemanticIndex { + log::error!("Building index for {}", file.path(db)); #[cfg(test)] let _depth = recursion_depth::enter(); diff --git a/crates/oak_db/src/file_imports.rs b/crates/oak_db/src/file_imports.rs index 3301df4713..1a3820003a 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -46,7 +46,7 @@ pub enum ImportLayer { /// flow-ordered set. /// /// [`SalsaImportsResolver`]: crate::imports::SalsaImportsResolver -#[derive(Debug, Clone, PartialEq, Eq, salsa::Update)] +#[derive(Debug, Clone, PartialEq, Eq, salsa::SalsaValue)] pub(crate) struct CrossFileLayers { pub above: Vec, pub below: Vec, diff --git a/crates/oak_db/src/lib.rs b/crates/oak_db/src/lib.rs index 608f76b069..2d147593c3 100644 --- a/crates/oak_db/src/lib.rs +++ b/crates/oak_db/src/lib.rs @@ -25,6 +25,7 @@ pub use db::workspace_files; pub use db::Db; pub use db::DbInputs; pub use definition::Definition; +pub use file::warm_file; pub use file::File; pub use file_exports::ExportEntry; pub use file_exports::FileExports; From 6c86fc5f373f16942571511d39c5f0e95a64c2c9 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 30 Jul 2026 16:56:01 +0200 Subject: [PATCH 09/13] Remove stray debugging message --- crates/oak_db/src/file.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/oak_db/src/file.rs b/crates/oak_db/src/file.rs index ad05875765..c5f22627c6 100644 --- a/crates/oak_db/src/file.rs +++ b/crates/oak_db/src/file.rs @@ -333,7 +333,6 @@ const STACK_RED_ZONE: usize = 1024 * 1024; const STACK_GROW_BY: usize = 8 * 1024 * 1024; fn build_semantic_index(file: File, db: &dyn Db) -> SemanticIndex { - log::error!("Building index for {}", file.path(db)); #[cfg(test)] let _depth = recursion_depth::enter(); From 9eb48cdf38a0ff5b370705f2c8da4ed21af68554 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 30 Jul 2026 17:27:53 +0200 Subject: [PATCH 10/13] Reformat --- crates/oak_ide/tests/integration/base_sources.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/oak_ide/tests/integration/base_sources.rs b/crates/oak_ide/tests/integration/base_sources.rs index 1629e7c1b8..fe135d8465 100644 --- a/crates/oak_ide/tests/integration/base_sources.rs +++ b/crates/oak_ide/tests/integration/base_sources.rs @@ -42,8 +42,15 @@ const R_VERSION: &str = "4.5.2"; /// R's default search path, `stats` (highest priority) through `base` /// (lowest), see `crate::search::DEFAULT_SEARCH_PATH_PACKAGES` in `oak_db`. -const DEFAULT_SEARCH_PATH_PACKAGES: [&str; 7] = - ["stats", "graphics", "grDevices", "utils", "datasets", "methods", "base"]; +const DEFAULT_SEARCH_PATH_PACKAGES: [&str; 7] = [ + "stats", + "graphics", + "grDevices", + "utils", + "datasets", + "methods", + "base", +]; /// Register every default-search-path package as a library package backed by /// its real source directory under `r_root`. From b2b870943cc9fd1f58ad0ffc9c776d7246f5b5e7 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 31 Jul 2026 09:10:59 +0200 Subject: [PATCH 11/13] Tweak comment --- crates/ark/src/lsp/main_loop.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index 815ebd9d1e..18f48eeb7b 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -632,13 +632,9 @@ impl GlobalState { ); // Re-warm the oak semantic indexes on every revision, counting on - // idempotence (warm files are salsa cache hits). Runs on a snapshot, - // so a concurrent write just cancels the in-flight warm and the next - // revision re-runs it. This is what carries warmup through the - // startup write-storm (each source/edit cancels the previous warm, - // and the revision after the storm settles completes it), and it - // means a freshly-typed `pkg::` dependency is warmed as soon as its - // sources land and bump the revision. + // idempotence (warm files are salsa cache hits). Takes care of + // warmin up the initial workspace as well as any new dependency + // introduced by user edits. analysis::warm_semantic_indexes(&self.world, &self.lsp_state.analysis_pool); } From e5818c672f0ed58377273e69692e4713c584b6ec Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 3 Aug 2026 13:47:22 +0200 Subject: [PATCH 12/13] Dependencies of cursor file have precedence over dependencies of workspace --- crates/oak_db/src/db.rs | 2 +- crates/oak_ide/src/find_references.rs | 28 ++++++--- .../tests/integration/find_references.rs | 22 +++++++ crates/oak_ide/tests/integration/support.rs | 62 ++++++++++++------- 4 files changed, 83 insertions(+), 31 deletions(-) diff --git a/crates/oak_db/src/db.rs b/crates/oak_db/src/db.rs index 081f40f44b..96a7d07b95 100644 --- a/crates/oak_db/src/db.rs +++ b/crates/oak_db/src/db.rs @@ -144,7 +144,7 @@ pub fn all_used_files(db: &dyn Db) -> Vec { /// Note that this also contains files from all installed packages, _including /// those that are not dependencies of the workspace_. Only use this for very /// wide searches. LSP functionality should generally not depend on -/// non-dependencies, prefer `all_dependency_files()` instead. +/// non-dependencies, prefer [`all_used_files()`] instead. #[salsa::tracked(returns(ref))] pub fn all_known_files(db: &dyn Db) -> Vec { let mut seen = FxHashSet::default(); diff --git a/crates/oak_ide/src/find_references.rs b/crates/oak_ide/src/find_references.rs index b650f15532..d61c7e0574 100644 --- a/crates/oak_ide/src/find_references.rs +++ b/crates/oak_ide/src/find_references.rs @@ -94,7 +94,7 @@ fn collect_definition_references<'db>( let files = if locally_scoped { vec![file] } else { - all_matching_files(db, name.text(db).as_str()) + all_matching_files(db, file, name.text(db).as_str()) }; // Rust-Analyzer does a pure text search across all files, then resolves @@ -120,7 +120,7 @@ fn collect_definition_references<'db>( // package-level definition, so include the qualified sites too. Locally // scoped symbols (params, locals) can't be reached through `::`. if !locally_scoped { - collect_package_qualified_uses(db, &target_defs, name, &mut results); + collect_package_qualified_uses(db, file, &target_defs, name, &mut results); } if include_declaration { @@ -152,6 +152,7 @@ fn collect_definition_references<'db>( /// hands the definitions to `collect_definition_references`. fn collect_package_qualified_uses<'db>( db: &'db dyn Db, + cursor_file: File, target_defs: &[Definition<'db>], name: Name<'db>, results: &mut Vec, @@ -166,7 +167,7 @@ fn collect_package_qualified_uses<'db>( let package = package.name(db); let name = name.text(db); - for file in all_matching_files(db, name.as_str()) { + for file in all_matching_files(db, cursor_file, name.as_str()) { for range in file.namespace_uses_of(db, package, name.as_str()) { results.push(FileRange { file, range }); } @@ -176,7 +177,7 @@ fn collect_package_qualified_uses<'db>( fn find_member_references(db: &dyn Db, file: File, name: &str, kind: MemberKind) -> Vec { let mut results = Vec::new(); - for file in all_matching_files(db, name) { + for file in all_matching_files(db, file, name) { for range in file.member_uses_of(db, name, kind) { results.push(FileRange { file, range }); } @@ -216,7 +217,7 @@ fn find_namespace_references<'db>( let package = package.text(db).as_str(); let name = name.text(db).as_str(); - for file in all_matching_files(db, name) { + for file in all_matching_files(db, primary, name) { for range in file.namespace_uses_of(db, package, name) { results.push(FileRange { file, range }); } @@ -226,12 +227,21 @@ fn find_namespace_references<'db>( results } -/// Every db file whose contents mention `text`, scoped to the workspace plus -/// its actual dependencies (not every package under `.libPaths()`). -fn all_matching_files(db: &dyn Db, text: &str) -> Vec { +/// Searches the workspace, its dependencies, and `cursor_file`'s package. +/// +/// The cursor package may not be a workspace dependency, but references within +/// it must still be found. +fn all_matching_files(db: &dyn Db, cursor_file: File, text: &str) -> Vec { + let cursor_package_files = cursor_file + .package(db) + .into_iter() + .flat_map(|package| package.files(db).iter().chain(package.scripts(db))); + + let mut seen = HashSet::new(); all_used_files(db) .iter() - .filter(|&&f| f.source_text(db).contains(text)) + .chain(cursor_package_files) + .filter(|&&file| seen.insert(file) && file.source_text(db).contains(text)) .copied() .collect() } diff --git a/crates/oak_ide/tests/integration/find_references.rs b/crates/oak_ide/tests/integration/find_references.rs index aa5277106d..d41574dbac 100644 --- a/crates/oak_ide/tests/integration/find_references.rs +++ b/crates/oak_ide/tests/integration/find_references.rs @@ -14,6 +14,7 @@ use oak_db::OakDatabase; use oak_ide::find_references; use crate::support::install_library_package; +use crate::support::install_library_package_files; use crate::support::install_workspace_package; use crate::support::offset; use crate::support::pairs; @@ -521,6 +522,27 @@ fn test_cursor_in_installed_package_excludes_other_packages() { ]); } +#[test] +fn test_cursor_in_non_dependency_installed_package_finds_references() { + // `mypkg` is not a workspace dependency, so `all_used_files()` excludes it. + // Reference search must still include the cursor package. + let mut db = OakDatabase::new(); + let files = install_library_package_files(&mut db, "mypkg", &["foo"], &[ + ("a.R", "foo <- function() 1\nfoo()\n"), + ("b.R", "foo()\n"), + ]); + let (a_file, b_file) = (files[0], files[1]); + let _script = upsert(&mut db, "script.R", "1 + 1\n"); + + let refs = find_references(&db, a_file, offset(0), true); + + assert_eq!(pairs(&refs), vec![ + (a_file, range(0, 3)), + (a_file, range(20, 23)), + (b_file, range(0, 3)), + ]); +} + #[test] fn test_cross_package_references_via_library() { // A script attaches `mypkg` and uses its exported `foo`. The use resolves diff --git a/crates/oak_ide/tests/integration/support.rs b/crates/oak_ide/tests/integration/support.rs index 0b70c66819..6934bf33bd 100644 --- a/crates/oak_ide/tests/integration/support.rs +++ b/crates/oak_ide/tests/integration/support.rs @@ -77,7 +77,21 @@ pub fn install_library_package( file_name: &str, contents: &str, ) -> File { - install_pkg(db, RootKind::Library, name, exports, file_name, contents) + let files = install_pkg(db, RootKind::Library, name, exports, &[( + file_name, contents, + )]); + files[0] +} + +/// Install a library package with `files` under `R/`. Returns files in input +/// order. +pub fn install_library_package_files( + db: &mut OakDatabase, + name: &str, + exports: &[&str], + files: &[(&str, &str)], +) -> Vec { + install_pkg(db, RootKind::Library, name, exports, files) } /// Install `name` as a workspace package exporting `exports`, with one file at @@ -89,7 +103,10 @@ pub fn install_workspace_package( file_name: &str, contents: &str, ) -> File { - install_pkg(db, RootKind::Workspace, name, exports, file_name, contents) + let files = install_pkg(db, RootKind::Workspace, name, exports, &[( + file_name, contents, + )]); + files[0] } fn install_pkg( @@ -97,21 +114,19 @@ fn install_pkg( kind: RootKind, name: &str, exports: &[&str], - file_name: &str, - contents: &str, -) -> File { - let (pkg_url, file_url, root_url) = match kind { - RootKind::Library => ( - lib_url(&format!("{name}/DESCRIPTION")), - lib_url(&format!("{name}/R/{file_name}")), - lib_url(name), - ), + sources: &[(&str, &str)], +) -> Vec { + let (pkg_url, root_url) = match kind { + RootKind::Library => (lib_url(&format!("{name}/DESCRIPTION")), lib_url(name)), RootKind::Workspace => ( workspace_url(&format!("{name}/DESCRIPTION")), - workspace_url(&format!("{name}/R/{file_name}")), workspace_url(name), ), }; + let file_url = |file_name: &str| match kind { + RootKind::Library => lib_url(&format!("{name}/R/{file_name}")), + RootKind::Workspace => workspace_url(&format!("{name}/R/{file_name}")), + }; let namespace = Namespace { exports: SortedVec::from_vec(exports.iter().map(|s| s.to_string()).collect()), ..Default::default() @@ -127,14 +142,19 @@ fn install_pkg( Vec::new(), Vec::new(), ); - let file = File::new( - db, - FilePath::from_url(&file_url), - FileRevision::zero(), - Some(contents.to_string()), - Some(pkg), - ); - pkg.set_files(db).to(vec![file]); + let files: Vec = sources + .iter() + .map(|&(file_name, contents)| { + File::new( + db, + FilePath::from_url(&file_url(file_name)), + FileRevision::zero(), + Some(contents.to_string()), + Some(pkg), + ) + }) + .collect(); + pkg.set_files(db).to(files.clone()); let root = Root::new(db, FilePath::from_url(&root_url), kind, Vec::new(), vec![ pkg, ]); @@ -150,5 +170,5 @@ fn install_pkg( db.workspace_roots().set_roots(db).to(vec![root]); }, }; - file + files } From 63bcb905108bf25042a6f0408a7fe011b5d1edee Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 3 Aug 2026 13:52:47 +0200 Subject: [PATCH 13/13] Fix typo --- crates/ark/src/lsp/main_loop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index 18f48eeb7b..110fce4d4e 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -633,7 +633,7 @@ impl GlobalState { // Re-warm the oak semantic indexes on every revision, counting on // idempotence (warm files are salsa cache hits). Takes care of - // warmin up the initial workspace as well as any new dependency + // warming up the initial workspace as well as any new dependency // introduced by user edits. analysis::warm_semantic_indexes(&self.world, &self.lsp_state.analysis_pool); }