Restrict Salsa dependencies during source resolution

crates/ark/src/lsp/diagnostics.rs

@@ -1271,8 +1271,8 @@ mod tests {
 
     use aether_path::FilePath;
     use harp::eval::RParseEvalOptions;
-    use oak_db::Db;
     use oak_db::OakDatabase;
+    use oak_db::SourceDb;
     use oak_scan::DbScan;
     use tempfile::TempDir;
     use tower_lsp_server::ls_types as lsp_types;

crates/ark/src/lsp/find_references.rs

@@ -1,6 +1,6 @@
 use aether_lsp_utils::proto::from_proto;
 use aether_lsp_utils::proto::to_proto;
-use oak_db::Db;
+use oak_db::SourceDb;
 use stdext::result::ResultExt;
 use tower_lsp_server::ls_types::Location;
 use tower_lsp_server::ls_types::ReferenceParams;

crates/ark/src/lsp/goto_definition.rs

@@ -1,7 +1,7 @@
 use aether_lsp_utils::proto::from_proto;
 use aether_lsp_utils::proto::to_proto;
 use aether_lsp_utils::proto::PositionEncoding;
-use oak_db::Db;
+use oak_db::SourceDb;
 use oak_ide::NavigationTarget;
 use stdext::result::ResultExt;
 use tower_lsp_server::ls_types::GotoDefinitionParams;

crates/ark/src/lsp/rename.rs

@@ -3,7 +3,7 @@ use std::collections::HashMap;
 use aether_lsp_utils::proto::from_proto;
 use aether_lsp_utils::proto::to_proto;
 use anyhow::Context;
-use oak_db::Db;
+use oak_db::SourceDb;
 use tower_lsp_server::ls_types as lsp_types;
 use tower_lsp_server::ls_types::PrepareRenameResponse;
 use tower_lsp_server::ls_types::RenameParams;

crates/ark/src/lsp/tests/goto_definition.rs

@@ -3,8 +3,8 @@ use std::sync::Arc;
 
 use aether_path::FilePath;
 use assert_matches::assert_matches;
-use oak_db::Db;
 use oak_db::OakDatabase;
+use oak_db::SourceDb;
 use oak_scan::DbScan;
 use tower_lsp_server::ls_types as lsp_types;
 use tower_lsp_server::ls_types::GotoDefinitionParams;

crates/ark/src/lsp/tests/rename.rs

@@ -1,8 +1,8 @@
 use aether_path::FilePath;
-use oak_db::Db;
 use oak_db::DbInputs;
 use oak_db::Root;
 use oak_db::RootKind;
+use oak_db::SourceDb;
 use salsa::Setter;
 use tower_lsp_server::ls_types as lsp_types;
 use tower_lsp_server::ls_types::PrepareRenameResponse;

crates/ark/src/lsp/tests/sources.rs

@@ -9,8 +9,8 @@ use std::sync::mpsc::Sender;
 use std::sync::Arc;
 use std::sync::Mutex;
 
-use oak_db::Db;
 use oak_db::OakDatabase;
+use oak_db::SourceDb;
 use oak_scan::DbScan;
 use serde_json::json;
 use serde_json::Value;

crates/ark/src/lsp/tests/state_handlers.rs

@@ -11,8 +11,8 @@ use std::path::PathBuf;
 
 use aether_path::AbsPathBuf;
 use aether_path::FilePath;
-use oak_db::Db;
 use oak_db::DbInputs;
+use oak_db::SourceDb;
 use oak_scan::DbScan;
 use oak_scan::ScanRequest;
 use oak_scan::ScanScheduler;

crates/oak_db/src/db.rs

@@ -18,10 +18,8 @@ use crate::WorkspaceRoots;
 /// ([`crate::OakDatabase`], the test db) supplies singleton input handles
 /// and file reads.
 ///
-/// Kept separate from [`Db`] (the query trait) so input accessors and derived
-/// queries live on different traits. Mirrors rust-analyzer's `SourceDatabase`
-/// / `DefDatabase` split: input plumbing on the base trait, derived queries
-/// on the query trait.
+/// [`SourceDb`] adds lookups over these inputs. [`Db`] additionally permits
+/// recursive semantic queries.
 #[salsa::db]
 pub trait DbInputs: salsa::Database {
     /// Workspace folders opened by the editor.
@@ -43,18 +41,15 @@ pub trait DbInputs: salsa::Database {
     fn read_to_string(&self, path: &Utf8Path) -> io::Result<String>;
 }
 
-/// Salsa database trait used throughout `oak_db`. Tracked queries take `&dyn
-/// Db`, so query code never names the concrete db type.
+/// Database access for source text, syntax, membership, and package metadata.
+/// Queries accepting this trait cannot directly call semantic queries that
+/// require [`Db`].
 ///
-/// Methods aren't memoized at this level: they delegate to free helpers
-/// (`file_by_path_query` etc.) that walk per-root indices which *are* memoized,
-/// so salsa records dep edges through those.
-///
-/// Each concrete db type provides its own forwarding `impl Db`, which is
-/// what lets `db.file_by_path(path)` work on both `&dyn Db` (via the trait
-/// method) and concrete db references (via the type's impl).
+/// Lookup methods delegate to tracked per-root indices, preserving their
+/// invalidation boundaries. Recovery helpers also use this trait: keep its
+/// implementations and their transitive dependencies free of semantic queries.
 #[salsa::db]
-pub trait Db: DbInputs {
+pub trait SourceDb: DbInputs {
     /// Look up the `File` interned at `path`, if any.
     ///
     /// Walks the per-root URL indices in workspace-then-library order,
@@ -92,8 +87,12 @@ pub trait Db: DbInputs {
     fn live_roots(&self) -> &[LiveRoot];
 }
 
+/// Unrestricted database access for recursive semantic queries.
+#[salsa::db]
+pub trait Db: SourceDb {}
+
 #[salsa::tracked(returns(ref))]
-pub(crate) fn live_roots_query(db: &dyn Db) -> Vec<LiveRoot> {
+pub(crate) fn live_roots_query(db: &dyn SourceDb) -> Vec<LiveRoot> {
     let mut roots: Vec<LiveRoot> = db
         .workspace_roots()
         .roots(db)
@@ -154,7 +153,7 @@ pub fn all_used_files(db: &dyn Db) -> Vec<File> {
 /// wide searches. LSP functionality should generally not depend on
 /// non-dependencies, prefer [`all_used_files()`] instead.
 #[salsa::tracked(returns(ref))]
-pub fn all_known_files(db: &dyn Db) -> Vec<File> {
+pub fn all_known_files(db: &dyn SourceDb) -> Vec<File> {
     let mut seen = FxHashSet::default();
     let mut files = Vec::new();
 
@@ -180,7 +179,7 @@ pub fn all_known_files(db: &dyn Db) -> Vec<File> {
 /// already in `seen`. When `dependencies` is `Some`, packages not in that set
 /// are skipped entirely.
 fn push_root_files(
-    db: &dyn Db,
+    db: &dyn SourceDb,
     files: &mut Vec<File>,
     seen: &mut FxHashSet<File>,
     root: Root,
@@ -203,7 +202,7 @@ fn push_root_files(
 /// package files, plus orphan editor buffers. Library roots are excluded, so
 /// installed package symbols don't leak into e.g. workspace symbols.
 #[salsa::tracked(returns(ref))]
-pub fn workspace_files(db: &dyn Db) -> Vec<File> {
+pub fn workspace_files(db: &dyn SourceDb) -> Vec<File> {
     let mut files: Vec<File> = Vec::new();
 
     for &root in db.live_roots() {
@@ -220,7 +219,7 @@ pub fn workspace_files(db: &dyn Db) -> Vec<File> {
 /// The scripts held directly by workspace roots, in root order.
 /// Like [`workspace_files`] but without package files.
 #[salsa::tracked(returns(ref))]
-pub(crate) fn workspace_scripts(db: &dyn Db) -> Vec<File> {
+pub(crate) fn workspace_scripts(db: &dyn SourceDb) -> Vec<File> {
     db.workspace_roots()
         .roots(db)
         .iter()
@@ -231,15 +230,15 @@ pub(crate) fn workspace_scripts(db: &dyn Db) -> Vec<File> {
 /// Every file owned by a workspace root, including package files. Orphan
 /// buffers are excluded because directory loading is rooted on disk.
 #[salsa::tracked(returns(ref))]
-pub(crate) fn workspace_root_files(db: &dyn Db) -> Vec<File> {
+pub(crate) fn workspace_root_files(db: &dyn SourceDb) -> Vec<File> {
     let mut files: Vec<File> = Vec::new();
     for &root in db.workspace_roots().roots(db) {
         collect_root_files(db, &mut files, root);
     }
     files
 }
 
-fn collect_root_files(db: &dyn Db, files: &mut Vec<File>, r: Root) {
+fn collect_root_files(db: &dyn SourceDb, files: &mut Vec<File>, r: Root) {
     let owned = |f: File| root_by_file(db, f) == Some(r);
     files.extend(r.scripts(db).iter().copied().filter(|&f| owned(f)));
 
@@ -249,13 +248,13 @@ fn collect_root_files(db: &dyn Db, files: &mut Vec<File>, r: Root) {
     }
 }
 
-/// Implementation of [`Db::file_by_path`]. Walks the per-root indices.
+/// Implementation of [`SourceDb::file_by_path`]. Walks the per-root indices.
 ///
 /// Not itself salsa-tracked (its `&FilePath` argument isn't a salsa
 /// entity), but every step is: each [`root_path_index`] call returns a
 /// cached map, so adding a file to one root invalidates only that
 /// root's index.
-pub(crate) fn file_by_path_query(db: &dyn Db, path: &FilePath) -> Option<File> {
+pub(crate) fn file_by_path_query(db: &dyn SourceDb, path: &FilePath) -> Option<File> {
     for &root in db.live_roots() {
         let hit = match root {
             LiveRoot::Workspace(r) | LiveRoot::Library(r) => {
@@ -270,10 +269,10 @@ pub(crate) fn file_by_path_query(db: &dyn Db, path: &FilePath) -> Option<File> {
     None
 }
 
-/// Implementation of [`Db::package_by_name`]. Same shape as
+/// Implementation of [`SourceDb::package_by_name`]. Same shape as
 /// [`file_by_path_query`]; orphan has no packages, so it contributes
 /// nothing to the walk.
-pub(crate) fn package_by_name_query(db: &dyn Db, name: &str) -> Option<Package> {
+pub(crate) fn package_by_name_query(db: &dyn SourceDb, name: &str) -> Option<Package> {
     for &root in db.live_roots() {
         if let LiveRoot::Workspace(r) | LiveRoot::Library(r) = root {
             if let Some(&pkg) = root_package_index(db, r).get(name) {
@@ -284,9 +283,9 @@ pub(crate) fn package_by_name_query(db: &dyn Db, name: &str) -> Option<Package>
     None
 }
 
-/// Implementation of [`Db::root_by_package`]. Walks all live roots looking for
+/// Implementation of [`SourceDb::root_by_package`]. Walks all live roots looking for
 /// `pkg` in their `packages` vec, picking the longest-path root on ties.
-pub(crate) fn root_by_package_query(db: &dyn Db, pkg: Package) -> Option<Root> {
+pub(crate) fn root_by_package_query(db: &dyn SourceDb, pkg: Package) -> Option<Root> {
     let mut best: Option<(Root, usize)> = None;
     for &root in db.live_roots() {
         let (LiveRoot::Workspace(r) | LiveRoot::Library(r)) = root else {
@@ -315,7 +314,7 @@ pub(crate) fn root_by_package_query(db: &dyn Db, pkg: Package) -> Option<Root> {
 ///
 /// Returns `None` for orphan files (they live in no workspace or library
 /// root). [`File::root`] handles that case with a path-prefix fallback.
-pub(crate) fn root_by_file(db: &dyn Db, file: File) -> Option<Root> {
+pub(crate) fn root_by_file(db: &dyn SourceDb, file: File) -> Option<Root> {
     let mut best: Option<(Root, usize)> = None;
 
     let path = file.path(db);
@@ -343,7 +342,7 @@ pub(crate) fn root_by_file(db: &dyn Db, file: File) -> Option<Root> {
 /// letter), which would silently collapse all depths to zero and degrade
 /// the tiebreaker into "first found wins". Depth is a structural property
 /// of the URL hierarchy, so the URL itself is the right source.
-fn root_depth(db: &dyn Db, root: Root) -> usize {
+fn root_depth(db: &dyn SourceDb, root: Root) -> usize {
     root.path(db)
         .to_url()
         .path_segments()
@@ -356,7 +355,7 @@ fn root_depth(db: &dyn Db, root: Root) -> usize {
 /// `pkg.scripts` reachable from this root. Adding or removing a file
 /// in *this* root invalidates this entry; other roots stay cached.
 #[salsa::tracked(returns(ref))]
-fn root_path_index(db: &dyn Db, root: Root) -> FxHashMap<FilePath, File> {
+fn root_path_index(db: &dyn SourceDb, root: Root) -> FxHashMap<FilePath, File> {
     let mut map = FxHashMap::default();
     for &file in root.scripts(db) {
         map.insert(file.path(db).clone(), file);
@@ -374,7 +373,7 @@ fn root_path_index(db: &dyn Db, root: Root) -> FxHashMap<FilePath, File> {
 
 /// Orphan URL -> File index. Reads only `orphan_root().files`.
 #[salsa::tracked(returns(ref))]
-fn orphan_path_index(db: &dyn Db) -> FxHashMap<FilePath, File> {
+fn orphan_path_index(db: &dyn SourceDb) -> FxHashMap<FilePath, File> {
     let mut map = FxHashMap::default();
     for &file in db.orphan_root().files(db) {
         map.insert(file.path(db).clone(), file);
@@ -385,7 +384,7 @@ fn orphan_path_index(db: &dyn Db) -> FxHashMap<FilePath, File> {
 /// Per-root name -> Package index. Same granularity as
 /// [`root_path_index`].
 #[salsa::tracked(returns(ref))]
-fn root_package_index(db: &dyn Db, root: Root) -> FxHashMap<String, Package> {
+fn root_package_index(db: &dyn SourceDb, root: Root) -> FxHashMap<String, Package> {
     let mut map = FxHashMap::default();
     for &pkg in root.packages(db) {
         map.insert(pkg.name(db).clone(), pkg);

crates/oak_db/src/directory.rs

@@ -9,14 +9,14 @@
 use camino::Utf8Path;
 
 use crate::db::workspace_root_files;
-use crate::Db;
 use crate::File;
+use crate::SourceDb;
 
 /// Returns workspace files directly under `dir`, in `list.files()` load order.
 ///
 /// `sourceDir()`, Shiny's `loadSupport()`, and non-package `R/` collation use
 /// this order. [`collation_basename_key()`] mirrors their session-locale sort.
-pub(crate) fn files_in_directory(db: &dyn Db, dir: &Utf8Path) -> Vec<File> {
+pub(crate) fn files_in_directory(db: &dyn SourceDb, dir: &Utf8Path) -> Vec<File> {
     let mut files: Vec<File> = workspace_root_files(db)
         .iter()
         .copied()
@@ -31,7 +31,7 @@ pub(crate) fn files_in_directory(db: &dyn Db, dir: &Utf8Path) -> Vec<File> {
 ///
 /// `list.files(recursive = TRUE)` sorts nested scripts by relative path. Case
 /// folding matches [`collation_basename_key()`].
-pub(crate) fn files_in_directory_recursive(db: &dyn Db, dir: &Utf8Path) -> Vec<File> {
+pub(crate) fn files_in_directory_recursive(db: &dyn SourceDb, dir: &Utf8Path) -> Vec<File> {
     let mut keyed: Vec<(String, File)> = workspace_root_files(db)
         .iter()
         .copied()
@@ -50,7 +50,7 @@ pub(crate) fn files_in_directory_recursive(db: &dyn Db, dir: &Utf8Path) -> Vec<F
 ///
 /// Package installation instead forces `LC_COLLATE=C`, where raw byte order
 /// determines collation.
-pub(crate) fn collation_basename_key(file: File, db: &dyn Db) -> Option<String> {
+pub(crate) fn collation_basename_key(file: File, db: &dyn SourceDb) -> Option<String> {
     file.path(db)
         .file_name()
         .map(|name| name.to_ascii_lowercase())

crates/oak_db/src/file.rs

@@ -17,6 +17,7 @@ use crate::FileRevision;
 use crate::Name;
 use crate::Package;
 use crate::Root;
+use crate::SourceDb;
 
 /// A source file tracked by Salsa.
 ///
@@ -79,7 +80,7 @@ impl File {
     ///
     /// A virtual path or an unreadable file yields empty text (matches ty).
     #[salsa::tracked(returns(ref), lru = 128)]
-    pub fn source_text(self, db: &dyn Db) -> String {
+    pub fn source_text(self, db: &dyn SourceDb) -> String {
         if let Some(text) = self.source_text_override(db) {
             return text.clone();
         }
@@ -122,7 +123,7 @@ impl File {
     /// memory cleanly. Derived queries (e.g. `semantic_index`) store
     /// `AstPtr`s rather than tree nodes, so they don't pin an evicted tree.
     #[salsa::tracked(returns(ref), lru = 128)]
-    pub(crate) fn parse(self, db: &dyn Db) -> OakParse {
+    pub(crate) fn parse(self, db: &dyn SourceDb) -> OakParse {
         OakParse::new(aether_parser::parse(
             self.source_text(db).as_str(),
             aether_parser::RParserOptions::default(),
@@ -302,7 +303,7 @@ impl File {
     /// The root containing this file, if any.
     ///
     /// Packaged files ask the db which live root holds the package via
-    /// [`Db::root_by_package`]. That branch covers library files too, which
+    /// [`SourceDb::root_by_package`]. That branch covers library files too, which
     /// normally have a package. It also keeps the common case cheap: it
     /// depends on each root's package list, not its full file set.
     ///
@@ -319,7 +320,7 @@ impl File {
     /// Callers that need to distinguish workspace from library roots
     /// inspect `root.kind(db)`.
     #[salsa::tracked(returns(copy))]
-    pub fn root(self, db: &dyn Db) -> Option<Root> {
+    pub fn root(self, db: &dyn SourceDb) -> Option<Root> {
         if let Some(pkg) = self.package(db) {
             return db.root_by_package(pkg);
         }
@@ -332,7 +333,7 @@ impl File {
 /// every workspace folder. Private helper: the only caller is
 /// [`File::root`], as the fallback for an orphan file no scan has reached
 /// yet (path prefix is all we have until a scan lands).
-fn root_by_path(db: &dyn Db, path: &FilePath) -> Option<Root> {
+fn root_by_path(db: &dyn SourceDb, path: &FilePath) -> Option<Root> {
     // Virtual documents (e.g. untitled scheme) don't have roots
     let path = path.as_path()?;
     db.workspace_roots()
@@ -374,35 +375,28 @@ fn build_semantic_index_inner(file: File, db: &dyn Db) -> SemanticIndex {
 
 fn attached_packages_cycle_result<'db>(
     db: &'db dyn Db,
-    id: salsa::Id,
+    _id: salsa::Id,
     file: File,
 ) -> Vec<Name<'db>> {
     record(db, Recovery::AttachedPackages(file));
-    attached_packages_fallback(db, id, file)
+    Vec::new()
 }
 
 fn attached_packages_anywhere_cycle_result<'db>(
     db: &'db dyn Db,
-    id: salsa::Id,
+    _id: salsa::Id,
     file: File,
 ) -> Vec<Name<'db>> {
     record(db, Recovery::AttachedPackagesAnywhere(file));
-    attached_packages_fallback(db, id, file)
-}
-
-/// Return no attaches. [`File::semantic_index`] recovery rebuilds with
-/// `NoopImportsResolver`, which emits [`SemanticDiagnostic::SourceCycle`] and
-/// also reports no attaches.
-fn attached_packages_fallback<'db>(db: &'db dyn Db, _id: salsa::Id, file: File) -> Vec<Name<'db>> {
-    log::warn!(
-        "Cyclic attaches detected at {}. Reporting no attached packages.",
-        file.path(db),
-    );
     Vec::new()
 }
 
 fn semantic_index_cycle_result(db: &dyn Db, _id: salsa::Id, file: File) -> SemanticIndex {
     record(db, Recovery::SemanticIndex(file));
+    semantic_index_fallback(db, file)
+}
+
+fn semantic_index_fallback(db: &dyn SourceDb, file: File) -> SemanticIndex {
     log::warn!(
         "Cyclic `source()` detected at {}. Rebuilding without cross-file resolution.",
         file.path(db),