From 0c1e24c07dd0b742f8796243588f335be225a701 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Mon, 15 Jul 2024 14:14:10 +0100 Subject: [PATCH] Remove use of `source_text()` --- crates/red_knot_module_resolver/src/path.rs | 6 +- .../red_knot_module_resolver/src/resolver.rs | 59 +++++++++++-------- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/crates/red_knot_module_resolver/src/path.rs b/crates/red_knot_module_resolver/src/path.rs index 4fc2f3554d469a..8a79c666fcd4f5 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; @@ -213,12 +213,12 @@ impl ModuleResolutionPathBuf { #[must_use] pub(crate) fn editable_installation_root( - db: &dyn Db, + system: &dyn System, path: impl Into, ) -> Option { let path = path.into(); // TODO: Add Salsa invalidation to this system call: - db.system() + system .is_directory(&path) .then_some(Self(ModuleResolutionPathBufInner::EditableInstall(path))) } diff --git a/crates/red_knot_module_resolver/src/resolver.rs b/crates/red_knot_module_resolver/src/resolver.rs index 9218597d87437a..e35f28833fb079 100644 --- a/crates/red_knot_module_resolver/src/resolver.rs +++ b/crates/red_knot_module_resolver/src/resolver.rs @@ -5,8 +5,7 @@ use std::sync::Arc; use rustc_hash::FxHasher; use ruff_db::files::{system_path_to_file, File, FilePath}; -use ruff_db::source::{source_text, SourceText}; -use ruff_db::system::{DirectoryEntry, SystemPath, SystemPathBuf}; +use ruff_db::system::{DirectoryEntry, System, SystemPath, SystemPathBuf}; use crate::db::Db; use crate::module::{Module, ModuleKind}; @@ -328,38 +327,31 @@ impl<'db> FusedIterator for SearchPathIterator<'db> {} /// 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> { - db: &'db dyn Db, + system: &'db dyn System, path: SystemPathBuf, - contents: SourceText, + contents: String, site_packages: &'db SystemPath, } impl<'db> PthFile<'db> { - fn new( - db: &'db dyn Db, - pth_path: SystemPathBuf, - file: File, - site_packages: &'db SystemPath, - ) -> Self { - Self { - db, - path: pth_path, - contents: source_text(db.upcast(), file), - site_packages, - } - } - /// 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<'a>(&'a self) -> impl Iterator + 'db where 'a: 'db, { + let PthFile { + system, + path: _, + contents, + site_packages, + } = self; + // 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 files that are executed by Python at startup. // https://docs.python.org/3/library/site.html#module-site - self.contents.lines().filter_map(|line| { + contents.lines().filter_map(move |line| { let line = line.trim(); if line.is_empty() || line.starts_with('#') @@ -368,8 +360,8 @@ impl<'db> PthFile<'db> { { return None; } - let possible_editable_install = SystemPath::absolute(line, self.site_packages); - ModuleResolutionPathBuf::editable_installation_root(self.db, possible_editable_install) + let possible_editable_install = SystemPath::absolute(line, site_packages); + ModuleResolutionPathBuf::editable_installation_root(*system, possible_editable_install) }) } } @@ -398,8 +390,16 @@ 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 = self.directory_iterator.next()?; + let entry_result = directory_iterator.next()?; let Ok(entry) = entry_result else { continue; }; @@ -413,10 +413,21 @@ impl<'db> Iterator for PthFileIterator<'db> { if path.extension() != Some("pth") { continue; } - let Some(file) = system_path_to_file(self.db.upcast(), &path) else { + + // This seemingly "redundant" call ensures that Salsa is aware + // that this query should be invalidated if the `.pth` file is deleted + system_path_to_file(db.upcast(), &path); + + let Ok(contents) = system.read_to_string(&path) else { continue; }; - return Some(PthFile::new(self.db, path, file, self.site_packages)); + + return Some(PthFile { + system, + path, + contents, + site_packages, + }); } } }