From 9cea75293469530afa6416d70bce9b933b8eea9b Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Tue, 26 Aug 2025 14:55:19 -0400 Subject: [PATCH] [ty] Require that we can find a root when listing sub-modules This is similar to a change made in the "list top-level modules" implementation that had been masked by poor Salsa failure modes. Basically, if we can't find a root here, it *must* be a bug. And if we just silently skip over it, we risk voiding Salsa's purity contract, leading to more difficult to debug panics. This did cause one test to fail, but only because the test wasn't properly setting up roots. --- Cargo.lock | 1 + crates/ty_ide/Cargo.toml | 2 +- crates/ty_ide/src/lib.rs | 17 ++++++++++++++++- .../src/module_resolver/module.rs | 8 +++++--- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a1af3dcb1..27e1808bb4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4241,6 +4241,7 @@ name = "ty_ide" version = "0.0.0" dependencies = [ "bitflags 2.9.3", + "camino", "get-size2", "insta", "itertools 0.14.0", diff --git a/crates/ty_ide/Cargo.toml b/crates/ty_ide/Cargo.toml index e00b8db200..08fbb7eedb 100644 --- a/crates/ty_ide/Cargo.toml +++ b/crates/ty_ide/Cargo.toml @@ -33,7 +33,7 @@ smallvec = { workspace = true } tracing = { workspace = true } [dev-dependencies] - +camino = { workspace = true } insta = { workspace = true, features = ["filters"] } [lints] diff --git a/crates/ty_ide/src/lib.rs b/crates/ty_ide/src/lib.rs index 6baea75a8f..b695ee5804 100644 --- a/crates/ty_ide/src/lib.rs +++ b/crates/ty_ide/src/lib.rs @@ -286,10 +286,12 @@ impl HasNavigationTargets for TypeDefinition<'_> { #[cfg(test)] mod tests { + use camino::Utf8Component; use insta::internals::SettingsBindDropGuard; + use ruff_db::Db; use ruff_db::diagnostic::{Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig}; - use ruff_db::files::{File, system_path_to_file}; + use ruff_db::files::{File, FileRootKind, system_path_to_file}; use ruff_db::system::{DbWithWritableSystem, SystemPath, SystemPathBuf}; use ruff_python_trivia::textwrap::dedent; use ruff_text_size::TextSize; @@ -378,6 +380,19 @@ mod tests { db.write_file(path, contents) .expect("write to memory file system to be successful"); + // Add a root for the top-most component. + let top = path.components().find_map(|c| match c { + Utf8Component::Normal(c) => Some(c), + _ => None, + }); + if let Some(top) = top { + let top = SystemPath::new(top); + if db.system().is_directory(top) { + db.files() + .try_add_root(&db, top, FileRootKind::LibrarySearchPath); + } + } + let file = system_path_to_file(&db, path).expect("newly written file to existing"); if let Some(offset) = cursor_offset { diff --git a/crates/ty_python_semantic/src/module_resolver/module.rs b/crates/ty_python_semantic/src/module_resolver/module.rs index 3586f52933..fdafa64323 100644 --- a/crates/ty_python_semantic/src/module_resolver/module.rs +++ b/crates/ty_python_semantic/src/module_resolver/module.rs @@ -161,9 +161,11 @@ fn all_submodule_names_for_package(db: &dyn Db, file: File) -> Option> // tree. When the revision gets bumped, the cache // that Salsa creates does for this routine will be // invalidated. - if let Some(root) = db.files().root(db, parent_directory) { - let _ = root.revision(db); - } + let root = db + .files() + .root(db, parent_directory) + .expect("System search path should have a registered root"); + let _ = root.revision(db); db.system() .read_directory(parent_directory)