Consolidate stale-related utils in oak_scan

Author: lionel-

Cargo.lock

@@ -60,16 +60,6 @@ pub trait Db: DbInputs {
     ///   (mirroring `.libPaths()`).
     fn package_by_name(&self, name: &str) -> Option<Package>;
 
-    /// Look up a `Package` by its `DESCRIPTION` URL.
-    ///
-    /// Walks workspace packages, then library packages, then falls back
-    /// to [`StaleRoot`]. Stale matches are intentional: scanner upserts
-    /// use this to find a `Package` entity whose live container was
-    /// dropped on a previous `set_*_paths` call, so the entity gets
-    /// reused on re-add. Analysis paths should not call this — they use
-    /// [`Db::package_by_name`] which is stale-blind.
-    fn package_by_url(&self, url: &UrlId) -> Option<Package>;
-
     /// Resolve the live `Root` that contains `pkg`, if any.
     ///
     /// Returns `None` when the package is only in [`StaleRoot`] (its live
@@ -147,19 +137,6 @@ pub fn package_by_name_query(db: &dyn Db, name: &str) -> Option<Package> {
     None
 }
 
-/// Implementation of [`Db::package_by_url`]. Walks live roots' packages
-/// by `description_url`, then falls back to the stale bucket.
-pub fn package_by_url_query(db: &dyn Db, url: &UrlId) -> Option<Package> {
-    for &root in db.live_roots() {
-        if let LiveRoot::Workspace(r) | LiveRoot::Library(r) = root {
-            if let Some(&pkg) = root_package_url_index(db, r).get(url) {
-                return Some(pkg);
-            }
-        }
-    }
-    stale_package_url_index(db).get(url).copied()
-}
-
 /// Implementation of [`Db::root_by_package`]. Walks all live roots looking for
 /// `pkg` in their `packages` vec, picking the longest-path root on ties.
 pub fn root_by_package_query(db: &dyn Db, pkg: Package) -> Option<Root> {
@@ -232,45 +209,3 @@ fn root_package_index(db: &dyn Db, root: Root) -> FxHashMap<String, Package> {
     }
     map
 }
-
-/// Per-root DESCRIPTION URL -> Package index. Used by
-/// [`package_by_url_query`] for entity-reuse lookups across rescans;
-/// salsa cache invalidates only when this root's packages change.
-#[salsa::tracked(returns(ref))]
-fn root_package_url_index(db: &dyn Db, root: Root) -> FxHashMap<UrlId, Package> {
-    let mut map = FxHashMap::default();
-    for &pkg in root.packages(db) {
-        map.insert(pkg.description_url(db).clone(), pkg);
-    }
-    map
-}
-
-/// Stale file URL -> File index. Reads only `stale_root().files`. Not
-/// consulted by [`file_by_url_query`] — analysis is stale-blind by
-/// design. Scanner upserts use [`stale_file_by_url`] when re-adding a
-/// path.
-#[salsa::tracked(returns(ref))]
-fn stale_url_index(db: &dyn Db) -> FxHashMap<UrlId, File> {
-    let mut map = FxHashMap::default();
-    for &file in db.stale_root().files(db) {
-        map.insert(file.url(db).clone(), file);
-    }
-    map
-}
-
-/// Look up a stale `File` by URL. Public so scanner upsert helpers in
-/// `oak_scan` can fall back to stale after [`Db::file_by_url`] misses.
-pub fn stale_file_by_url(db: &dyn Db, url: &UrlId) -> Option<File> {
-    stale_url_index(db).get(url).copied()
-}
-
-/// Stale DESCRIPTION URL -> Package index. Same role as
-/// [`stale_url_index`] for packages.
-#[salsa::tracked(returns(ref))]
-fn stale_package_url_index(db: &dyn Db) -> FxHashMap<UrlId, Package> {
-    let mut map = FxHashMap::default();
-    for &pkg in db.stale_root().packages(db) {
-        map.insert(pkg.description_url(db).clone(), pkg);
-    }
-    map
-}

crates/oak_db/src/db.rs

@@ -150,9 +150,9 @@ impl OrphanRoot {
 /// completions, goto-def, etc. — they correspond to folders the user
 /// has explicitly removed.
 ///
-/// **Consulted by scanners.** `Db::package_by_url` walks live roots
-/// then falls back to stale. Scanner upsert helpers do the same for
-/// files. On reuse, the entity is moved out of stale back into a live
+/// **Consulted by scanners.** The scanner's package-by-URL lookup walks
+/// live roots then falls back to stale. Scanner upsert helpers do the same
+/// for files. On reuse, the entity is moved out of stale back into a live
 /// container.
 ///
 /// Singleton like `OrphanRoot`. The `files` and `packages` fields are
@@ -177,9 +177,9 @@ impl StaleRoot {
 pub struct Package {
     /// URL of the package's `DESCRIPTION` file. Stable identity across
     /// rescans and workspace / library churn: scanners look up an
-    /// existing `Package` by this URL before creating a new one (see
-    /// [`Db::package_by_url`]). Two packages with the same `Package:`
-    /// name can coexist on disk and the URL distinguishes them.
+    /// existing `Package` by this URL before creating a new one. Two
+    /// packages with the same `Package:` name can coexist on disk and the
+    /// URL distinguishes them.
     ///
     /// The package's owning [`Root`] is not stored as a field. It is
     /// derived from live-graph containment via [`Db::root_by_package`]: a

crates/oak_db/src/inputs.rs

@@ -13,7 +13,6 @@ mod storage;
 #[cfg(test)]
 mod tests;
 
-pub use db::stale_file_by_url;
 pub use db::Db;
 pub use db::DbInputs;
 pub use definition::Definition;

crates/oak_db/src/lib.rs

@@ -62,10 +62,6 @@ impl Db for OakDatabase {
         crate::db::package_by_name_query(self, name)
     }
 
-    fn package_by_url(&self, url: &aether_url::UrlId) -> Option<crate::Package> {
-        crate::db::package_by_url_query(self, url)
-    }
-
     fn root_by_package(&self, pkg: crate::Package) -> Option<crate::Root> {
         crate::db::root_by_package_query(self, pkg)
     }

crates/oak_db/src/storage.rs

@@ -108,10 +108,6 @@ impl Db for TestDb {
         crate::db::package_by_name_query(self, name)
     }
 
-    fn package_by_url(&self, url: &UrlId) -> Option<crate::Package> {
-        crate::db::package_by_url_query(self, url)
-    }
-
     fn root_by_package(&self, pkg: crate::Package) -> Option<crate::Root> {
         crate::db::root_by_package_query(self, pkg)
     }

crates/oak_db/src/tests/test_db.rs

@@ -21,6 +21,7 @@ log.workspace = true
 oak_core.workspace = true
 oak_db.workspace = true
 oak_package_metadata.workspace = true
+rustc-hash.workspace = true
 salsa.workspace = true
 stdext.workspace = true
 url.workspace = true

crates/oak_scan/Cargo.toml

@@ -20,7 +20,6 @@ use std::collections::HashSet;
 use std::path::PathBuf;
 
 use aether_url::UrlId;
-use oak_db::stale_file_by_url;
 use oak_db::Db;
 use oak_db::DbInputs;
 use oak_db::File;
@@ -29,6 +28,11 @@ use oak_db::Root;
 use oak_package_metadata::namespace::Namespace;
 use salsa::Setter;
 
+use crate::lookup::package_by_url;
+use crate::stale::remove_from_stale_files;
+use crate::stale::remove_from_stale_packages;
+use crate::stale::stale_file_by_url;
+
 /// Description of one R file the scanner wants to register.
 ///
 /// `contents` is the on-disk snapshot at scan time. It's used as the
@@ -132,7 +136,7 @@ impl RootExt for Root {
         // in `self.packages` (rescan, common path) or in
         // `stale_root.packages` (resurrection after a previous eviction).
         // Either way we reuse the entity and refresh its metadata fields.
-        let pkg = match db.package_by_url(&description_url) {
+        let pkg = match package_by_url(db, &description_url) {
             Some(p) => {
                 p.set_name(db).to(name);
                 p.set_version(db).to(version);
@@ -222,23 +226,3 @@ fn remove_from_orphan<DB: Db + DbInputs>(db: &mut DB, file: File) {
     files.retain(|f| *f != file);
     orphan.set_files(db).to(files);
 }
-
-fn remove_from_stale_files<DB: Db + DbInputs>(db: &mut DB, file: File) {
-    let stale = db.stale_root();
-    if !stale.files(db).contains(&file) {
-        return;
-    }
-    let mut files = stale.files(db).clone();
-    files.retain(|f| *f != file);
-    stale.set_files(db).to(files);
-}
-
-fn remove_from_stale_packages<DB: Db + DbInputs>(db: &mut DB, pkg: Package) {
-    let stale = db.stale_root();
-    if !stale.packages(db).contains(&pkg) {
-        return;
-    }
-    let mut packages = stale.packages(db).clone();
-    packages.retain(|p| *p != pkg);
-    stale.set_packages(db).to(packages);
-}

crates/oak_scan/src/inputs.rs

@@ -24,6 +24,7 @@
 
 mod inputs;
 mod library;
+mod lookup;
 mod packages;
 mod stale;
 

crates/oak_scan/src/lib.rs

@@ -0,0 +1,47 @@
+//! Package lookup by `DESCRIPTION` URL.
+//!
+//! `oak_db` exposes `file_by_url` and `package_by_name` for analysis, but no
+//! by-URL package lookup. Analysis never needs one: it keys packages by name
+//! and is stale-blind. The scanner does need one, to find an existing
+//! `Package` entity to reuse across rescans and eviction cycles. So the lookup
+//! lives here on the write side, next to its only caller, rather than on
+//! `oak_db`'s public `Db` trait.
+
+use aether_url::UrlId;
+use oak_db::Db;
+use oak_db::LiveRoot;
+use oak_db::Package;
+use oak_db::Root;
+use rustc_hash::FxHashMap;
+
+use crate::stale::stale_package_url_index;
+
+/// Find the `Package` registered at `url` (a `DESCRIPTION` URL), searching live
+/// roots first and the [`oak_db::StaleRoot`] eviction bucket second.
+///
+/// Live roots are walked in lookup order (workspace then library) and the first
+/// hit wins. A stale hit means the package's live container was dropped on an
+/// earlier `set_*_paths` call. The scanner reuses that entity and moves it back
+/// into a live container, which is why this lookup deliberately sees stale
+/// packages where analysis (via `oak_db::Db::package_by_name`) does not.
+pub(crate) fn package_by_url(db: &dyn Db, url: &UrlId) -> Option<Package> {
+    for &root in db.live_roots() {
+        if let LiveRoot::Workspace(r) | LiveRoot::Library(r) = root {
+            if let Some(&pkg) = root_package_url_index(db, r).get(url) {
+                return Some(pkg);
+            }
+        }
+    }
+    stale_package_url_index(db).get(url).copied()
+}
+
+/// Per-root DESCRIPTION URL -> Package index. Salsa caches one map per `Root`
+/// and invalidates only when that root's packages change.
+#[salsa::tracked(returns(ref))]
+fn root_package_url_index(db: &dyn Db, root: Root) -> FxHashMap<UrlId, Package> {
+    let mut map = FxHashMap::default();
+    for &pkg in root.packages(db) {
+        map.insert(pkg.description_url(db).clone(), pkg);
+    }
+    map
+}