diff --git a/crates/red_knot_module_resolver/src/db.rs b/crates/red_knot_module_resolver/src/db.rs index 82da0e6e94d10..9d6d6419117d3 100644 --- a/crates/red_knot_module_resolver/src/db.rs +++ b/crates/red_knot_module_resolver/src/db.rs @@ -1,7 +1,7 @@ use ruff_db::Upcast; use crate::resolver::{ - file_to_module, + editable_install_resolution_paths, file_to_module, internal::{ModuleNameIngredient, ModuleResolverSettings}, resolve_module_query, }; @@ -11,6 +11,7 @@ use crate::typeshed::parse_typeshed_versions; pub struct Jar( ModuleNameIngredient<'_>, ModuleResolverSettings, + editable_install_resolution_paths, resolve_module_query, file_to_module, parse_typeshed_versions, diff --git a/crates/red_knot_module_resolver/src/path.rs b/crates/red_knot_module_resolver/src/path.rs index e529d32bc5095..9ad4463f525a0 100644 --- a/crates/red_knot_module_resolver/src/path.rs +++ b/crates/red_knot_module_resolver/src/path.rs @@ -6,7 +6,7 @@ use std::fmt; use ruff_db::files::{system_path_to_file, vendored_path_to_file, File, FilePath}; -use ruff_db::system::{SystemPath, SystemPathBuf}; +use ruff_db::system::{System, SystemPath, SystemPathBuf}; use ruff_db::vendored::{VendoredPath, VendoredPathBuf}; use crate::db::Db; @@ -73,6 +73,7 @@ enum ModuleResolutionPathBufInner { FirstParty(SystemPathBuf), StandardLibrary(FilePath), SitePackages(SystemPathBuf), + EditableInstall(SystemPathBuf), } impl ModuleResolutionPathBufInner { @@ -134,6 +135,19 @@ impl ModuleResolutionPathBufInner { ); path.push(component); } + Self::EditableInstall(ref mut path) => { + if let Some(extension) = extension { + assert!( + matches!(extension, "pyi" | "py"), + "Extension must be `py` or `pyi`; got `{extension}`" + ); + } + assert!( + path.extension().is_none(), + "Cannot push part {component} to {path}, which already has an extension" + ); + path.push(component); + } } } } @@ -197,6 +211,18 @@ impl ModuleResolutionPathBuf { .then_some(Self(ModuleResolutionPathBufInner::SitePackages(path))) } + #[must_use] + pub(crate) fn editable_installation_root( + system: &dyn System, + path: impl Into, + ) -> Option { + let path = path.into(); + // TODO: Add Salsa invalidation to this system call: + system + .is_directory(&path) + .then_some(Self(ModuleResolutionPathBufInner::EditableInstall(path))) + } + #[must_use] pub(crate) fn is_regular_package(&self, search_path: &Self, resolver: &ResolverState) -> bool { ModuleResolutionPathRef::from(self).is_regular_package(search_path, resolver) @@ -229,6 +255,16 @@ impl ModuleResolutionPathBuf { pub(crate) fn to_file(&self, search_path: &Self, resolver: &ResolverState) -> Option { ModuleResolutionPathRef::from(self).to_file(search_path, resolver) } + + pub(crate) fn as_system_path(&self) -> Option<&SystemPathBuf> { + match &self.0 { + ModuleResolutionPathBufInner::Extra(path) => Some(path), + ModuleResolutionPathBufInner::FirstParty(path) => Some(path), + ModuleResolutionPathBufInner::StandardLibrary(_) => None, + ModuleResolutionPathBufInner::SitePackages(path) => Some(path), + ModuleResolutionPathBufInner::EditableInstall(path) => Some(path), + } + } } impl fmt::Debug for ModuleResolutionPathBuf { @@ -250,6 +286,10 @@ impl fmt::Debug for ModuleResolutionPathBuf { .debug_tuple("ModuleResolutionPathBuf::StandardLibrary") .field(path) .finish(), + ModuleResolutionPathBufInner::EditableInstall(path) => f + .debug_tuple("ModuleResolutionPathBuf::EditableInstall") + .field(path) + .finish(), } } } @@ -272,6 +312,7 @@ enum ModuleResolutionPathRefInner<'a> { FirstParty(&'a SystemPath), StandardLibrary(FilePathRef<'a>), SitePackages(&'a SystemPath), + EditableInstall(&'a SystemPath), } impl<'a> ModuleResolutionPathRefInner<'a> { @@ -306,6 +347,7 @@ impl<'a> ModuleResolutionPathRefInner<'a> { (Self::Extra(path), Self::Extra(_)) => resolver.system().is_directory(path), (Self::FirstParty(path), Self::FirstParty(_)) => resolver.system().is_directory(path), (Self::SitePackages(path), Self::SitePackages(_)) => resolver.system().is_directory(path), + (Self::EditableInstall(path), Self::EditableInstall(_)) => resolver.system().is_directory(path), (Self::StandardLibrary(path), Self::StandardLibrary(stdlib_root)) => { match Self::query_stdlib_version(path, search_path, &stdlib_root, resolver) { TypeshedVersionsQueryResult::DoesNotExist => false, @@ -333,6 +375,7 @@ impl<'a> ModuleResolutionPathRefInner<'a> { (Self::Extra(path), Self::Extra(_)) => is_non_stdlib_pkg(resolver, path), (Self::FirstParty(path), Self::FirstParty(_)) => is_non_stdlib_pkg(resolver, path), (Self::SitePackages(path), Self::SitePackages(_)) => is_non_stdlib_pkg(resolver, path), + (Self::EditableInstall(path), Self::EditableInstall(_)) => is_non_stdlib_pkg(resolver, path), // Unlike the other variants: // (1) Account for VERSIONS // (2) Only test for `__init__.pyi`, not `__init__.py` @@ -358,6 +401,7 @@ impl<'a> ModuleResolutionPathRefInner<'a> { (Self::SitePackages(path), Self::SitePackages(_)) => { system_path_to_file(resolver.db.upcast(), path) } + (Self::EditableInstall(path), Self::EditableInstall(_)) => system_path_to_file(resolver.db.upcast(), path), (Self::StandardLibrary(path), Self::StandardLibrary(stdlib_root)) => { match Self::query_stdlib_version(&path, search_path, &stdlib_root, resolver) { TypeshedVersionsQueryResult::DoesNotExist => None, @@ -374,7 +418,10 @@ impl<'a> ModuleResolutionPathRefInner<'a> { #[must_use] fn to_module_name(self) -> Option { match self { - Self::Extra(path) | Self::FirstParty(path) | Self::SitePackages(path) => { + Self::Extra(path) + | Self::FirstParty(path) + | Self::SitePackages(path) + | Self::EditableInstall(path) => { let parent = path.parent()?; let parent_components = parent.components().map(|component| component.as_str()); let skip_final_part = @@ -421,6 +468,9 @@ impl<'a> ModuleResolutionPathRefInner<'a> { Self::SitePackages(path) => { ModuleResolutionPathBufInner::SitePackages(path.with_extension("pyi")) } + Self::EditableInstall(path) => { + ModuleResolutionPathBufInner::EditableInstall(path.with_extension("pyi")) + } } } @@ -437,6 +487,9 @@ impl<'a> ModuleResolutionPathRefInner<'a> { Self::SitePackages(path) => Some(ModuleResolutionPathBufInner::SitePackages( path.with_extension("py"), )), + Self::EditableInstall(path) => Some(ModuleResolutionPathBufInner::EditableInstall( + path.with_extension("py"), + )), } } @@ -474,6 +527,13 @@ impl<'a> ModuleResolutionPathRefInner<'a> { .then_some(Self::SitePackages(path)) }) } + (Self::EditableInstall(root), FilePathRef::System(absolute_path)) => { + absolute_path.strip_prefix(root).ok().and_then(|path| { + path.extension() + .map_or(true, |ext| matches!(ext, "pyi" | "py")) + .then_some(Self::EditableInstall(path)) + }) + } (Self::Extra(_), FilePathRef::Vendored(_)) => None, (Self::FirstParty(_), FilePathRef::Vendored(_)) => None, (Self::StandardLibrary(root), FilePathRef::Vendored(absolute_path)) => match root { @@ -487,6 +547,7 @@ impl<'a> ModuleResolutionPathRefInner<'a> { } }, (Self::SitePackages(_), FilePathRef::Vendored(_)) => None, + (Self::EditableInstall(_), FilePathRef::Vendored(_)) => None, } } } @@ -562,6 +623,10 @@ impl fmt::Debug for ModuleResolutionPathRef<'_> { .debug_tuple("ModuleResolutionPathRef::StandardLibrary") .field(path) .finish(), + ModuleResolutionPathRefInner::EditableInstall(path) => f + .debug_tuple("ModuleResolutionPathRef::EditableInstall") + .field(path) + .finish(), } } } @@ -582,6 +647,9 @@ impl<'a> From<&'a ModuleResolutionPathBuf> for ModuleResolutionPathRef<'a> { ModuleResolutionPathBufInner::SitePackages(path) => { ModuleResolutionPathRefInner::SitePackages(path) } + ModuleResolutionPathBufInner::EditableInstall(path) => { + ModuleResolutionPathRefInner::EditableInstall(path) + } }; ModuleResolutionPathRef(inner) } @@ -593,6 +661,7 @@ impl PartialEq for ModuleResolutionPathRef<'_> { ModuleResolutionPathRefInner::Extra(path) => path == other, ModuleResolutionPathRefInner::FirstParty(path) => path == other, ModuleResolutionPathRefInner::SitePackages(path) => path == other, + ModuleResolutionPathRefInner::EditableInstall(path) => path == other, ModuleResolutionPathRefInner::StandardLibrary(FilePathRef::System(path)) => { path == other } @@ -625,6 +694,7 @@ impl PartialEq for ModuleResolutionPathRef<'_> { ModuleResolutionPathRefInner::Extra(_) => false, ModuleResolutionPathRefInner::FirstParty(_) => false, ModuleResolutionPathRefInner::SitePackages(_) => false, + ModuleResolutionPathRefInner::EditableInstall(_) => false, ModuleResolutionPathRefInner::StandardLibrary(FilePathRef::System(_)) => false, ModuleResolutionPathRefInner::StandardLibrary(FilePathRef::Vendored(path)) => { path == other @@ -707,6 +777,9 @@ mod tests { ModuleResolutionPathRefInner::SitePackages(path) => { ModuleResolutionPathBufInner::SitePackages(path.to_path_buf()) } + ModuleResolutionPathRefInner::EditableInstall(path) => { + ModuleResolutionPathBufInner::EditableInstall(path.to_path_buf()) + } }; ModuleResolutionPathBuf(inner) } diff --git a/crates/red_knot_module_resolver/src/resolver.rs b/crates/red_knot_module_resolver/src/resolver.rs index 79ffed13aeb50..3e1b1dc2a049c 100644 --- a/crates/red_knot_module_resolver/src/resolver.rs +++ b/crates/red_knot_module_resolver/src/resolver.rs @@ -1,8 +1,12 @@ -use std::ops::Deref; +use std::collections; +use std::hash::BuildHasherDefault; +use std::iter::FusedIterator; use std::sync::Arc; +use rustc_hash::FxHasher; + use ruff_db::files::{File, FilePath}; -use ruff_db::system::SystemPathBuf; +use ruff_db::system::{DirectoryEntry, System, SystemPath, SystemPathBuf}; use crate::db::Db; use crate::module::{Module, ModuleKind}; @@ -12,6 +16,79 @@ use crate::resolver::internal::ModuleResolverSettings; use crate::state::ResolverState; use crate::supported_py_version::TargetVersion; +type SearchPathRoot = Arc; + +/// An ordered sequence of search paths. +/// +/// The sequence respects the invariant maintained by [`sys.path` at runtime] +/// where no two module-resolution paths ever point to the same directory on disk. +/// (Paths may, however, *overlap* -- e.g. you could have both `src/` and `src/foo` +/// as module resolution paths simultaneously.) +/// +/// [`sys.path` at runtime]: https://docs.python.org/3/library/site.html#module-site +#[derive(Debug, PartialEq, Eq, Default, Clone)] +pub(crate) struct SearchPathSequence { + raw_paths: collections::HashSet>, + search_paths: Vec, +} + +impl SearchPathSequence { + fn insert(&mut self, path: SearchPathRoot) -> bool { + // Just assume that all search paths that aren't SystemPaths are unique + if let Some(fs_path) = path.as_system_path() { + if self.raw_paths.contains(fs_path) { + false + } else { + let raw_path = fs_path.to_owned(); + self.search_paths.push(path); + self.raw_paths.insert(raw_path) + } + } else { + self.search_paths.push(path); + true + } + } + + fn contains(&self, path: &SearchPathRoot) -> bool { + if let Some(fs_path) = path.as_system_path() { + self.raw_paths.contains(fs_path) + } else { + self.search_paths.contains(path) + } + } + + fn iter(&self) -> std::slice::Iter { + self.search_paths.iter() + } +} + +impl<'a> IntoIterator for &'a SearchPathSequence { + type IntoIter = std::slice::Iter<'a, SearchPathRoot>; + type Item = &'a SearchPathRoot; + + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + +impl FromIterator for SearchPathSequence { + fn from_iter>(iter: T) -> Self { + let mut sequence = Self::default(); + for item in iter { + sequence.insert(item); + } + sequence + } +} + +impl Extend for SearchPathSequence { + fn extend>(&mut self, iter: T) { + for item in iter { + self.insert(item); + } + } +} + /// Configures the module resolver settings. /// /// Must be called before calling any other module resolution functions. @@ -19,7 +96,7 @@ pub fn set_module_resolution_settings(db: &mut dyn Db, config: RawModuleResoluti // There's no concurrency issue here because we hold a `&mut dyn Db` reference. No other // thread can mutate the `Db` while we're in this call, so using `try_get` to test if // the settings have already been set is safe. - let resolved_settings = config.into_configuration_settings(); + let resolved_settings = config.into_configuration_settings(db.system().current_directory()); if let Some(existing) = ModuleResolverSettings::try_get(db) { existing.set_settings(db).to(resolved_settings); } else { @@ -82,12 +159,14 @@ pub(crate) fn file_to_module(db: &dyn Db, file: File) -> Option { let resolver_settings = module_resolver_settings(db); - let relative_path = resolver_settings - .search_paths() - .iter() - .find_map(|root| root.relativize_path(path))?; + let mut search_paths = resolver_settings.search_paths(db); - let module_name = relative_path.to_module_name()?; + let module_name = loop { + let candidate = search_paths.next()?; + if let Some(relative_path) = candidate.relativize_path(path) { + break relative_path.to_module_name()?; + } + }; // Resolve the module name to see if Python would resolve the name to the same path. // If it doesn't, then that means that multiple modules have the same name in different @@ -133,7 +212,10 @@ pub struct RawModuleResolutionSettings { } impl RawModuleResolutionSettings { - /// Implementation of the typing spec's [module resolution order] + /// Validate and normalize the raw settings given by the user + /// into settings we can use for module resolution + /// + /// This method also implements the typing spec's [module resolution order]. /// /// TODO(Alex): this method does multiple `.unwrap()` calls when it should really return an error. /// Each `.unwrap()` call is a point where we're validating a setting that the user would pass @@ -143,67 +225,297 @@ impl RawModuleResolutionSettings { /// This validation should probably be done outside of Salsa? /// /// [module resolution order]: https://typing.readthedocs.io/en/latest/spec/distributing.html#import-resolution-ordering - fn into_configuration_settings(self) -> ModuleResolutionSettings { + fn into_configuration_settings( + self, + current_directory: &SystemPath, + ) -> ModuleResolutionSettings { let RawModuleResolutionSettings { target_version, extra_paths, workspace_root, - site_packages, + site_packages: site_packages_setting, custom_typeshed, } = self; - let mut paths: Vec = extra_paths + let mut static_search_paths: SearchPathSequence = extra_paths .into_iter() - .map(|fs_path| ModuleResolutionPathBuf::extra(fs_path).unwrap()) + .map(|fs_path| { + Arc::new( + ModuleResolutionPathBuf::extra(SystemPath::absolute( + fs_path, + current_directory, + )) + .unwrap(), + ) + }) .collect(); - paths.push(ModuleResolutionPathBuf::first_party(workspace_root).unwrap()); - - paths.push( - custom_typeshed.map_or_else(ModuleResolutionPathBuf::vendored_stdlib, |custom| { - ModuleResolutionPathBuf::stdlib_from_custom_typeshed_root(&custom).unwrap() - }), - ); + static_search_paths.insert(Arc::new( + ModuleResolutionPathBuf::first_party(SystemPath::absolute( + workspace_root, + current_directory, + )) + .unwrap(), + )); + + static_search_paths.insert(Arc::new(custom_typeshed.map_or_else( + ModuleResolutionPathBuf::vendored_stdlib, + |custom| { + ModuleResolutionPathBuf::stdlib_from_custom_typeshed_root(&SystemPath::absolute( + custom, + current_directory, + )) + .unwrap() + }, + ))); + + let mut site_packages = None; + + if let Some(path) = site_packages_setting { + let site_packages_root = Arc::new( + ModuleResolutionPathBuf::site_packages(SystemPath::absolute( + path, + current_directory, + )) + .unwrap(), + ); + site_packages = Some(site_packages_root.clone()); + static_search_paths.insert(site_packages_root); + } // TODO vendor typeshed's third-party stubs as well as the stdlib and fallback to them as a final step - if let Some(site_packages) = site_packages { - paths.push(ModuleResolutionPathBuf::site_packages(site_packages).unwrap()); - } ModuleResolutionSettings { target_version, - search_paths: OrderedSearchPaths(paths.into_iter().map(Arc::new).collect()), + search_path_settings: ValidatedSearchPathSettings { + static_search_paths, + site_packages, + }, } } } -/// A resolved module resolution order as per the [typing spec] +#[derive(Debug, PartialEq, Eq, Clone)] +struct ValidatedSearchPathSettings { + /// Search paths that have been statically determined purely from reading Ruff's configuration settings. + /// These shouldn't ever change unless the config settings themselves change. + /// + /// Note that `site-packages` *is included* as a search path in this sequence, + /// but it is also stored separately so that we're able to find editable installs later. + static_search_paths: SearchPathSequence, + site_packages: Option, +} + +/// Collect all dynamic search paths: +/// search paths listed in `.pth` files in the `site-packages` directory +/// due to editable installations of third-party packages. +#[salsa::tracked(return_ref)] +pub(crate) fn editable_install_resolution_paths(db: &dyn Db) -> SearchPathSequence { + // This query needs to be re-executed each time a `.pth` file + // is added, modified or removed from the `site-packages` directory. + // However, we don't use Salsa queries to read the source text of `.pth` files; + // we use the APIs on the `System` trait directly. As such, for now we simply ask + // Salsa to recompute this query on each new revision. + // + // TODO: add some kind of watcher for the `site-packages` directory that looks + // for `site-packages/*.pth` files being added/modified/removed; get rid of this. + // When doing so, also make the test + // `deleting_pth_file_on_which_module_resolution_depends_invalidates_cache()` + // more principled! + db.report_untracked_read(); + + let ValidatedSearchPathSettings { + static_search_paths, + site_packages, + } = &module_resolver_settings(db).search_path_settings; + + let mut dynamic_paths = SearchPathSequence::default(); + + if let Some(site_packages) = site_packages { + let site_packages = site_packages + .as_system_path() + .expect("Expected site-packages never to be a VendoredPath!"); + + // As well as modules installed directly into `site-packages`, + // the directory may also contain `.pth` files. + // Each `.pth` file in `site-packages` may contain one or more lines + // containing a (relative or absolute) path. + // Each of these paths may point to an editable install of a package, + // so should be considered an additional search path. + let Ok(pth_file_iterator) = PthFileIterator::new(db, site_packages) else { + return dynamic_paths; + }; + + // The Python documentation specifies that `.pth` files in `site-packages` + // are processed in alphabetical order, so collecting and then sorting is necessary. + // https://docs.python.org/3/library/site.html#module-site + let mut all_pth_files: Vec = pth_file_iterator.collect(); + all_pth_files.sort_by(|a, b| a.path.cmp(&b.path)); + + for pth_file in &all_pth_files { + dynamic_paths.extend( + pth_file + .editable_installations() + .filter_map(|editable_path| { + let possible_search_path = Arc::new(editable_path); + (!static_search_paths.contains(&possible_search_path)) + .then_some(possible_search_path) + }), + ); + } + } + dynamic_paths +} + +/// Iterate over the available module-resolution search paths, +/// following the invariants maintained by [`sys.path` at runtime]: +/// "No item is added to `sys.path` more than once." +/// Dynamic search paths (required for editable installs into `site-packages`) +/// are only calculated lazily. /// -/// [typing spec]: https://typing.readthedocs.io/en/latest/spec/distributing.html#import-resolution-ordering -#[derive(Clone, Debug, Default, Eq, PartialEq)] -pub(crate) struct OrderedSearchPaths(Vec>); +/// [`sys.path` at runtime]: https://docs.python.org/3/library/site.html#module-site +struct SearchPathIterator<'db> { + db: &'db dyn Db, + static_paths: std::slice::Iter<'db, SearchPathRoot>, + dynamic_paths: Option>, +} + +impl<'db> Iterator for SearchPathIterator<'db> { + type Item = &'db SearchPathRoot; + + fn next(&mut self) -> Option { + let SearchPathIterator { + db, + static_paths, + dynamic_paths, + } = self; + + static_paths.next().or_else(|| { + dynamic_paths + .get_or_insert_with(|| editable_install_resolution_paths(*db).into_iter()) + .next() + }) + } +} + +impl<'db> FusedIterator for SearchPathIterator<'db> {} + +/// Represents a single `.pth` file in a `site-packages` directory. +/// One or more lines in a `.pth` file may be a (relative or absolute) +/// path that represents an editable installation of a package. +struct PthFile<'db> { + system: &'db dyn System, + path: SystemPathBuf, + contents: String, + site_packages: &'db SystemPath, +} -impl Deref for OrderedSearchPaths { - type Target = [Arc]; +impl<'db> PthFile<'db> { + /// Yield paths in this `.pth` file that appear to represent editable installations, + /// and should therefore be added as module-resolution search paths. + fn editable_installations(&'db self) -> impl Iterator + 'db { + let PthFile { + system, + path: _, + contents, + site_packages, + } = self; - fn deref(&self) -> &Self::Target { - &self.0 + // Empty lines or lines starting with '#' are ignored by the Python interpreter. + // Lines that start with "import " or "import\t" do not represent editable installs at all; + // instead, these are lines that are executed by Python at startup. + // https://docs.python.org/3/library/site.html#module-site + contents.lines().filter_map(move |line| { + let line = line.trim_end(); + if line.is_empty() + || line.starts_with('#') + || line.starts_with("import ") + || line.starts_with("import\t") + { + return None; + } + let possible_editable_install = SystemPath::absolute(line, site_packages); + ModuleResolutionPathBuf::editable_installation_root(*system, possible_editable_install) + }) + } +} + +/// Iterator that yields a [`PthFile`] instance for every `.pth` file +/// found in a given `site-packages` directory. +struct PthFileIterator<'db> { + db: &'db dyn Db, + directory_iterator: Box> + 'db>, + site_packages: &'db SystemPath, +} + +impl<'db> PthFileIterator<'db> { + fn new(db: &'db dyn Db, site_packages: &'db SystemPath) -> std::io::Result { + Ok(Self { + db, + directory_iterator: db.system().read_directory(site_packages)?, + site_packages, + }) + } +} + +impl<'db> Iterator for PthFileIterator<'db> { + type Item = PthFile<'db>; + + fn next(&mut self) -> Option { + let PthFileIterator { + db, + directory_iterator, + site_packages, + } = self; + + let system = db.system(); + + loop { + let entry_result = directory_iterator.next()?; + let Ok(entry) = entry_result else { + continue; + }; + let file_type = entry.file_type(); + if file_type.is_directory() { + continue; + } + let path = entry.into_path(); + if path.extension() != Some("pth") { + continue; + } + + let Ok(contents) = db.system().read_to_string(&path) else { + continue; + }; + + return Some(PthFile { + system, + path, + contents, + site_packages, + }); + } } } +/// Validated and normalized module-resolution settings. #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct ModuleResolutionSettings { - search_paths: OrderedSearchPaths, + search_path_settings: ValidatedSearchPathSettings, target_version: TargetVersion, } impl ModuleResolutionSettings { - pub(crate) fn search_paths(&self) -> &[Arc] { - &self.search_paths + fn target_version(&self) -> TargetVersion { + self.target_version } - pub(crate) fn target_version(&self) -> TargetVersion { - self.target_version + fn search_paths<'db>(&'db self, db: &'db dyn Db) -> SearchPathIterator<'db> { + SearchPathIterator { + db, + static_paths: self.search_path_settings.static_search_paths.iter(), + dynamic_paths: None, + } } } @@ -245,7 +557,7 @@ fn resolve_name( let resolver_settings = module_resolver_settings(db); let resolver_state = ResolverState::new(db, resolver_settings.target_version()); - for search_path in resolver_settings.search_paths() { + for search_path in resolver_settings.search_paths(db) { let mut components = name.components(); let module_name = components.next_back()?; @@ -388,6 +700,7 @@ mod tests { use ruff_db::files::{system_path_to_file, File, FilePath}; use ruff_db::system::{DbWithTestSystem, OsSystem, SystemPath}; use ruff_db::testing::assert_function_query_was_not_run; + use ruff_db::Db; use crate::db::tests::TestDb; use crate::module::ModuleKind; @@ -1140,4 +1453,259 @@ mod tests { system_path_to_file(&db, stdlib.join("functools.pyi")) ); } + + #[test] + fn editable_install_absolute_path() { + const SITE_PACKAGES: &[FileSpec] = &[("_foo.pth", "/x/src")]; + let x_directory = [("/x/src/foo/__init__.py", ""), ("/x/src/foo/bar.py", "")]; + + let TestCase { mut db, .. } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + db.write_files(x_directory).unwrap(); + + let foo_module_name = ModuleName::new_static("foo").unwrap(); + let foo_bar_module_name = ModuleName::new_static("foo.bar").unwrap(); + + let foo_module = resolve_module(&db, foo_module_name.clone()).unwrap(); + let foo_bar_module = resolve_module(&db, foo_bar_module_name.clone()).unwrap(); + + assert_eq!( + foo_module.file().path(&db), + &FilePath::system("/x/src/foo/__init__.py") + ); + assert_eq!( + foo_bar_module.file().path(&db), + &FilePath::system("/x/src/foo/bar.py") + ); + } + + #[test] + fn editable_install_pth_file_with_whitespace() { + const SITE_PACKAGES: &[FileSpec] = &[ + ("_foo.pth", " /x/src"), + ("_bar.pth", "/y/src "), + ]; + let external_files = [("/x/src/foo.py", ""), ("/y/src/bar.py", "")]; + + let TestCase { mut db, .. } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + db.write_files(external_files).unwrap(); + + // Lines with leading whitespace in `.pth` files do not parse: + let foo_module_name = ModuleName::new_static("foo").unwrap(); + assert_eq!(resolve_module(&db, foo_module_name), None); + + // Lines with trailing whitespace in `.pth` files do: + let bar_module_name = ModuleName::new_static("bar").unwrap(); + let bar_module = resolve_module(&db, bar_module_name.clone()).unwrap(); + assert_eq!( + bar_module.file().path(&db), + &FilePath::system("/y/src/bar.py") + ); + } + + #[test] + fn editable_install_relative_path() { + const SITE_PACKAGES: &[FileSpec] = &[ + ("_foo.pth", "../../x/../x/y/src"), + ("../x/y/src/foo.pyi", ""), + ]; + + let TestCase { db, .. } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + let foo_module_name = ModuleName::new_static("foo").unwrap(); + let foo_module = resolve_module(&db, foo_module_name.clone()).unwrap(); + + assert_eq!( + foo_module.file().path(&db), + &FilePath::system("/x/y/src/foo.pyi") + ); + } + + #[test] + fn editable_install_multiple_pth_files_with_multiple_paths() { + const COMPLEX_PTH_FILE: &str = "\ +/ + +# a comment +/baz + +import not_an_editable_install; do_something_else_crazy_dynamic() + +# another comment +spam + +not_a_directory +"; + + const SITE_PACKAGES: &[FileSpec] = &[ + ("_foo.pth", "../../x/../x/y/src"), + ("_lots_of_others.pth", COMPLEX_PTH_FILE), + ("../x/y/src/foo.pyi", ""), + ("spam/spam.py", ""), + ]; + + let root_files = [("/a.py", ""), ("/baz/b.py", "")]; + + let TestCase { + mut db, + site_packages, + .. + } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + db.write_files(root_files).unwrap(); + + let foo_module_name = ModuleName::new_static("foo").unwrap(); + let a_module_name = ModuleName::new_static("a").unwrap(); + let b_module_name = ModuleName::new_static("b").unwrap(); + let spam_module_name = ModuleName::new_static("spam").unwrap(); + + let foo_module = resolve_module(&db, foo_module_name.clone()).unwrap(); + let a_module = resolve_module(&db, a_module_name.clone()).unwrap(); + let b_module = resolve_module(&db, b_module_name.clone()).unwrap(); + let spam_module = resolve_module(&db, spam_module_name.clone()).unwrap(); + + assert_eq!( + foo_module.file().path(&db), + &FilePath::system("/x/y/src/foo.pyi") + ); + assert_eq!(a_module.file().path(&db), &FilePath::system("/a.py")); + assert_eq!(b_module.file().path(&db), &FilePath::system("/baz/b.py")); + assert_eq!( + spam_module.file().path(&db), + &FilePath::System(site_packages.join("spam/spam.py")) + ); + } + + #[test] + fn module_resolution_paths_cached_between_different_module_resolutions() { + const SITE_PACKAGES: &[FileSpec] = &[("_foo.pth", "/x/src"), ("_bar.pth", "/y/src")]; + let external_directories = [("/x/src/foo.py", ""), ("/y/src/bar.py", "")]; + + let TestCase { mut db, .. } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + db.write_files(external_directories).unwrap(); + + let foo_module_name = ModuleName::new_static("foo").unwrap(); + let bar_module_name = ModuleName::new_static("bar").unwrap(); + + let foo_module = resolve_module(&db, foo_module_name).unwrap(); + assert_eq!( + foo_module.file().path(&db), + &FilePath::system("/x/src/foo.py") + ); + + db.clear_salsa_events(); + let bar_module = resolve_module(&db, bar_module_name).unwrap(); + assert_eq!( + bar_module.file().path(&db), + &FilePath::system("/y/src/bar.py") + ); + let events = db.take_salsa_events(); + assert_function_query_was_not_run::( + &db, + |res| &res.function, + &(), + &events, + ); + } + + #[test] + fn deleting_pth_file_on_which_module_resolution_depends_invalidates_cache() { + const SITE_PACKAGES: &[FileSpec] = &[("_foo.pth", "/x/src")]; + let x_directory = [("/x/src/foo.py", "")]; + + let TestCase { + mut db, + site_packages, + .. + } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + db.write_files(x_directory).unwrap(); + + let foo_module_name = ModuleName::new_static("foo").unwrap(); + let foo_module = resolve_module(&db, foo_module_name.clone()).unwrap(); + assert_eq!( + foo_module.file().path(&db), + &FilePath::system("/x/src/foo.py") + ); + + db.memory_file_system() + .remove_file(site_packages.join("_foo.pth")) + .unwrap(); + + // Why are we touching a random file in the path that's been editably installed, + // rather than the `.pth` file, when the `.pth` file is the one that has been deleted? + // It's because the `.pth` file isn't directly tracked as a dependency by Salsa + // currently (we don't use `system_path_to_file()` to get the file, and we don't use + // `source_text()` to read the source of the file). Instead of using these APIs which + // would automatically add the existence and contents of the file as a Salsa-tracked + // dependency, we use `.report_untracked_read()` to force Salsa to re-parse all + // `.pth` files on each new "revision". Making a random modification to a tracked + // Salsa file forces a new revision. + // + // TODO: get rid of the `.report_untracked_read()` call... + File::touch_path(&mut db, SystemPath::new("/x/src/foo.py")); + + assert_eq!(resolve_module(&db, foo_module_name.clone()), None); + } + + #[test] + fn deleting_editable_install_on_which_module_resolution_depends_invalidates_cache() { + const SITE_PACKAGES: &[FileSpec] = &[("_foo.pth", "/x/src")]; + let x_directory = [("/x/src/foo.py", "")]; + + let TestCase { mut db, .. } = TestCaseBuilder::new() + .with_site_packages_files(SITE_PACKAGES) + .build(); + + db.write_files(x_directory).unwrap(); + + let foo_module_name = ModuleName::new_static("foo").unwrap(); + let foo_module = resolve_module(&db, foo_module_name.clone()).unwrap(); + let src_path = SystemPathBuf::from("/x/src"); + assert_eq!( + foo_module.file().path(&db), + &FilePath::System(src_path.join("foo.py")) + ); + + db.memory_file_system() + .remove_file(src_path.join("foo.py")) + .unwrap(); + db.memory_file_system().remove_directory(&src_path).unwrap(); + File::touch_path(&mut db, &src_path.join("foo.py")); + File::touch_path(&mut db, &src_path); + assert_eq!(resolve_module(&db, foo_module_name.clone()), None); + } + + #[test] + fn no_duplicate_search_paths_added() { + let TestCase { db, .. } = TestCaseBuilder::new() + .with_src_files(&[("foo.py", "")]) + .with_site_packages_files(&[("_foo.pth", "/src")]) + .build(); + + let search_paths: Vec<&SearchPathRoot> = + module_resolver_settings(&db).search_paths(&db).collect(); + + assert!(search_paths.contains(&&Arc::new( + ModuleResolutionPathBuf::first_party("/src").unwrap() + ))); + + assert!(!search_paths.contains(&&Arc::new( + ModuleResolutionPathBuf::editable_installation_root(db.system(), "/src").unwrap() + ))); + } } diff --git a/crates/ruff_db/src/system.rs b/crates/ruff_db/src/system.rs index 168ee1ebe1c08..09c3a8776826a 100644 --- a/crates/ruff_db/src/system.rs +++ b/crates/ruff_db/src/system.rs @@ -150,6 +150,10 @@ impl DirectoryEntry { Self { path, file_type } } + pub fn into_path(self) -> SystemPathBuf { + self.path + } + pub fn path(&self) -> &SystemPath { &self.path }