[red-knot] Only store absolute paths in Files (#12215)

This commit is contained in:
Micha Reiser 2024-07-09 09:52:13 +02:00 committed by GitHub
parent 3d3ff10bb9
commit b5834d57af
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 206 additions and 84 deletions

View file

@ -60,17 +60,20 @@ impl Files {
/// In these cases, a file with status [`FileStatus::Deleted`] is returned.
#[tracing::instrument(level = "debug", skip(self, db))]
fn system(&self, db: &dyn Db, path: &SystemPath) -> File {
let absolute = SystemPath::absolute(path, db.system().current_directory());
let absolute = FilePath::System(absolute);
*self
.inner
.files_by_path
.entry(FilePath::System(path.to_path_buf()))
.entry(absolute.clone())
.or_insert_with(|| {
let metadata = db.system().path_metadata(path);
match metadata {
Ok(metadata) if metadata.file_type().is_file() => File::new(
db,
FilePath::System(path.to_path_buf()),
absolute,
metadata.permissions(),
metadata.revision(),
FileStatus::Exists,
@ -78,7 +81,7 @@ impl Files {
),
_ => File::new(
db,
FilePath::System(path.to_path_buf()),
absolute,
None,
FileRevision::zero(),
FileStatus::Deleted,
@ -89,10 +92,11 @@ impl Files {
}
/// Tries to look up the file for the given system path, returns `None` if no such file exists yet
fn try_system(&self, path: &SystemPath) -> Option<File> {
fn try_system(&self, db: &dyn Db, path: &SystemPath) -> Option<File> {
let absolute = SystemPath::absolute(path, db.system().current_directory());
self.inner
.files_by_path
.get(&FilePath::System(path.to_path_buf()))
.get(&FilePath::System(absolute))
.map(|entry| *entry.value())
}
@ -224,7 +228,7 @@ impl File {
_ => (FileStatus::Deleted, FileRevision::zero()),
};
let Some(file) = file.or_else(|| db.files().try_system(path)) else {
let Some(file) = file.or_else(|| db.files().try_system(db, path)) else {
return;
};
@ -260,7 +264,7 @@ mod tests {
use crate::vendored::tests::VendoredFileSystemBuilder;
#[test]
fn file_system_existing_file() -> crate::system::Result<()> {
fn system_existing_file() -> crate::system::Result<()> {
let mut db = TestDb::new();
db.write_file("test.py", "print('Hello world')")?;
@ -275,7 +279,7 @@ mod tests {
}
#[test]
fn file_system_non_existing_file() {
fn system_non_existing_file() {
let db = TestDb::new();
let test = system_path_to_file(&db, "test.py");
@ -283,6 +287,21 @@ mod tests {
assert_eq!(test, None);
}
#[test]
fn system_normalize_paths() {
let db = TestDb::new();
assert_eq!(
system_path_to_file(&db, "test.py"),
system_path_to_file(&db, "/test.py")
);
assert_eq!(
system_path_to_file(&db, "/root/.././test.py"),
system_path_to_file(&db, "/root/test.py")
);
}
#[test]
fn stubbed_vendored_file() {
let mut db = TestDb::new();

View file

@ -51,6 +51,9 @@ pub trait System {
.map_or(false, |metadata| metadata.file_type.is_file())
}
/// Returns the current working directory
fn current_directory(&self) -> &SystemPath;
fn as_any(&self) -> &dyn std::any::Any;
}

View file

@ -4,7 +4,7 @@ use std::sync::{Arc, RwLock, RwLockWriteGuard};
use camino::{Utf8Path, Utf8PathBuf};
use filetime::FileTime;
use crate::system::{FileType, Metadata, Result, SystemPath};
use crate::system::{FileType, Metadata, Result, SystemPath, SystemPathBuf};
/// File system that stores all content in memory.
///
@ -27,12 +27,12 @@ impl MemoryFileSystem {
const PERMISSION: u32 = 0o755;
pub fn new() -> Self {
Self::with_cwd("/")
Self::with_current_directory("/")
}
/// Creates a new, empty in memory file system with the given current working directory.
pub fn with_cwd(cwd: impl AsRef<SystemPath>) -> Self {
let cwd = Utf8PathBuf::from(cwd.as_ref().as_str());
pub fn with_current_directory(cwd: impl AsRef<SystemPath>) -> Self {
let cwd = cwd.as_ref().to_path_buf();
assert!(
cwd.starts_with("/"),
@ -46,11 +46,15 @@ impl MemoryFileSystem {
}),
};
fs.create_directory_all(SystemPath::new(&cwd)).unwrap();
fs.create_directory_all(&cwd).unwrap();
fs
}
pub fn current_directory(&self) -> &SystemPath {
&self.inner.cwd
}
#[must_use]
pub fn snapshot(&self) -> Self {
Self {
@ -59,9 +63,9 @@ impl MemoryFileSystem {
}
pub fn metadata(&self, path: impl AsRef<SystemPath>) -> Result<Metadata> {
fn metadata(fs: &MemoryFileSystemInner, path: &SystemPath) -> Result<Metadata> {
let by_path = fs.by_path.read().unwrap();
let normalized = normalize_path(path, &fs.cwd);
fn metadata(fs: &MemoryFileSystem, path: &SystemPath) -> Result<Metadata> {
let by_path = fs.inner.by_path.read().unwrap();
let normalized = fs.normalize_path(path);
let entry = by_path.get(&normalized).ok_or_else(not_found)?;
@ -81,27 +85,27 @@ impl MemoryFileSystem {
Ok(metadata)
}
metadata(&self.inner, path.as_ref())
metadata(self, path.as_ref())
}
pub fn is_file(&self, path: impl AsRef<SystemPath>) -> bool {
let by_path = self.inner.by_path.read().unwrap();
let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
let normalized = self.normalize_path(path.as_ref());
matches!(by_path.get(&normalized), Some(Entry::File(_)))
}
pub fn is_directory(&self, path: impl AsRef<SystemPath>) -> bool {
let by_path = self.inner.by_path.read().unwrap();
let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
let normalized = self.normalize_path(path.as_ref());
matches!(by_path.get(&normalized), Some(Entry::Directory(_)))
}
pub fn read_to_string(&self, path: impl AsRef<SystemPath>) -> Result<String> {
fn read_to_string(fs: &MemoryFileSystemInner, path: &SystemPath) -> Result<String> {
let by_path = fs.by_path.read().unwrap();
let normalized = normalize_path(path, &fs.cwd);
fn read_to_string(fs: &MemoryFileSystem, path: &SystemPath) -> Result<String> {
let by_path = fs.inner.by_path.read().unwrap();
let normalized = fs.normalize_path(path);
let entry = by_path.get(&normalized).ok_or_else(not_found)?;
@ -111,12 +115,12 @@ impl MemoryFileSystem {
}
}
read_to_string(&self.inner, path.as_ref())
read_to_string(self, path.as_ref())
}
pub fn exists(&self, path: &SystemPath) -> bool {
let by_path = self.inner.by_path.read().unwrap();
let normalized = normalize_path(path, &self.inner.cwd);
let normalized = self.normalize_path(path);
by_path.contains_key(&normalized)
}
@ -146,7 +150,7 @@ impl MemoryFileSystem {
pub fn write_file(&self, path: impl AsRef<SystemPath>, content: impl ToString) -> Result<()> {
let mut by_path = self.inner.by_path.write().unwrap();
let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
let normalized = self.normalize_path(path.as_ref());
get_or_create_file(&mut by_path, &normalized)?.content = content.to_string();
@ -156,7 +160,7 @@ impl MemoryFileSystem {
pub fn remove_file(&self, path: impl AsRef<SystemPath>) -> Result<()> {
fn remove_file(fs: &MemoryFileSystem, path: &SystemPath) -> Result<()> {
let mut by_path = fs.inner.by_path.write().unwrap();
let normalized = normalize_path(path, &fs.inner.cwd);
let normalized = fs.normalize_path(path);
match by_path.entry(normalized) {
std::collections::btree_map::Entry::Occupied(entry) => match entry.get() {
@ -178,7 +182,7 @@ impl MemoryFileSystem {
/// Creates a new file if the file at `path` doesn't exist.
pub fn touch(&self, path: impl AsRef<SystemPath>) -> Result<()> {
let mut by_path = self.inner.by_path.write().unwrap();
let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
let normalized = self.normalize_path(path.as_ref());
get_or_create_file(&mut by_path, &normalized)?.last_modified = FileTime::now();
@ -188,7 +192,7 @@ impl MemoryFileSystem {
/// Creates a directory at `path`. All enclosing directories are created if they don't exist.
pub fn create_directory_all(&self, path: impl AsRef<SystemPath>) -> Result<()> {
let mut by_path = self.inner.by_path.write().unwrap();
let normalized = normalize_path(path.as_ref(), &self.inner.cwd);
let normalized = self.normalize_path(path.as_ref());
create_dir_all(&mut by_path, &normalized)
}
@ -202,7 +206,7 @@ impl MemoryFileSystem {
pub fn remove_directory(&self, path: impl AsRef<SystemPath>) -> Result<()> {
fn remove_directory(fs: &MemoryFileSystem, path: &SystemPath) -> Result<()> {
let mut by_path = fs.inner.by_path.write().unwrap();
let normalized = normalize_path(path, &fs.inner.cwd);
let normalized = fs.normalize_path(path);
// Test if the directory is empty
// Skip the directory path itself
@ -228,6 +232,11 @@ impl MemoryFileSystem {
remove_directory(self, path.as_ref())
}
fn normalize_path(&self, path: impl AsRef<SystemPath>) -> Utf8PathBuf {
let normalized = SystemPath::absolute(path, &self.inner.cwd);
normalized.into_utf8_path_buf()
}
}
impl Default for MemoryFileSystem {
@ -246,7 +255,7 @@ impl std::fmt::Debug for MemoryFileSystem {
struct MemoryFileSystemInner {
by_path: RwLock<BTreeMap<Utf8PathBuf, Entry>>,
cwd: Utf8PathBuf,
cwd: SystemPathBuf,
}
#[derive(Debug)]
@ -292,42 +301,6 @@ fn directory_not_empty() -> std::io::Error {
std::io::Error::new(std::io::ErrorKind::Other, "directory not empty")
}
/// Normalizes the path by removing `.` and `..` components and transform the path into an absolute path.
///
/// Adapted from https://github.com/rust-lang/cargo/blob/fede83ccf973457de319ba6fa0e36ead454d2e20/src/cargo/util/paths.rs#L61
fn normalize_path(path: &SystemPath, cwd: &Utf8Path) -> Utf8PathBuf {
let path = camino::Utf8Path::new(path.as_str());
let mut components = path.components().peekable();
let mut ret =
if let Some(c @ (camino::Utf8Component::Prefix(..) | camino::Utf8Component::RootDir)) =
components.peek().cloned()
{
components.next();
Utf8PathBuf::from(c.as_str())
} else {
cwd.to_path_buf()
};
for component in components {
match component {
camino::Utf8Component::Prefix(..) => unreachable!(),
camino::Utf8Component::RootDir => {
ret.push(component);
}
camino::Utf8Component::CurDir => {}
camino::Utf8Component::ParentDir => {
ret.pop();
}
camino::Utf8Component::Normal(c) => {
ret.push(c);
}
}
}
ret
}
fn create_dir_all(
paths: &mut RwLockWriteGuard<BTreeMap<Utf8PathBuf, Entry>>,
normalized: &Utf8Path,

View file

@ -1,12 +1,27 @@
use crate::system::{FileType, Metadata, Result, System, SystemPath, SystemPathBuf};
use filetime::FileTime;
use std::any::Any;
use crate::system::{FileType, Metadata, Result, System, SystemPath};
use std::sync::Arc;
#[derive(Default, Debug)]
pub struct OsSystem;
pub struct OsSystem {
inner: Arc<OsSystemInner>,
}
#[derive(Default, Debug)]
struct OsSystemInner {
cwd: SystemPathBuf,
}
impl OsSystem {
pub fn new(cwd: impl AsRef<SystemPath>) -> Self {
Self {
inner: Arc::new(OsSystemInner {
cwd: cwd.as_ref().to_path_buf(),
}),
}
}
#[cfg(unix)]
fn permissions(metadata: &std::fs::Metadata) -> Option<u32> {
use std::os::unix::fs::PermissionsExt;
@ -20,7 +35,9 @@ impl OsSystem {
}
pub fn snapshot(&self) -> Self {
Self
Self {
inner: self.inner.clone(),
}
}
}
@ -44,6 +61,10 @@ impl System for OsSystem {
path.as_std_path().exists()
}
fn current_directory(&self) -> &SystemPath {
&self.inner.cwd
}
fn as_any(&self) -> &dyn Any {
self
}

View file

@ -306,6 +306,94 @@ impl SystemPath {
pub fn from_std_path(path: &Path) -> Option<&SystemPath> {
Some(SystemPath::new(Utf8Path::from_path(path)?))
}
/// Makes a path absolute and normalizes it without accessing the file system.
///
/// Adapted from [cargo](https://github.com/rust-lang/cargo/blob/fede83ccf973457de319ba6fa0e36ead454d2e20/src/cargo/util/paths.rs#L61)
///
/// # Examples
///
/// ## Posix paths
///
/// ```
/// # #[cfg(unix)]
/// # fn main() {
/// use ruff_db::system::{SystemPath, SystemPathBuf};
///
/// // Relative to absolute
/// let absolute = SystemPath::absolute("foo/./bar", "/tmp");
/// assert_eq!(absolute, SystemPathBuf::from("/tmp/foo/bar"));
///
/// // Path's going past the root are normalized to the root
/// let absolute = SystemPath::absolute("../../../", "/tmp");
/// assert_eq!(absolute, SystemPathBuf::from("/"));
///
/// // Absolute to absolute
/// let absolute = SystemPath::absolute("/foo//test/.././bar.rs", "/tmp");
/// assert_eq!(absolute, SystemPathBuf::from("/foo/bar.rs"));
/// # }
/// # #[cfg(not(unix))]
/// # fn main() {}
/// ```
///
/// ## Windows paths
///
/// ```
/// # #[cfg(windows)]
/// # fn main() {
/// use ruff_db::system::{SystemPath, SystemPathBuf};
///
/// // Relative to absolute
/// let absolute = SystemPath::absolute(r"foo\.\bar", r"C:\tmp");
/// assert_eq!(absolute, SystemPathBuf::from(r"C:\tmp\foo\bar"));
///
/// // Path's going past the root are normalized to the root
/// let absolute = SystemPath::absolute(r"..\..\..\", r"C:\tmp");
/// assert_eq!(absolute, SystemPathBuf::from(r"C:\"));
///
/// // Absolute to absolute
/// let absolute = SystemPath::absolute(r"C:\foo//test\..\./bar.rs", r"C:\tmp");
/// assert_eq!(absolute, SystemPathBuf::from(r"C:\foo\bar.rs"));
/// # }
/// # #[cfg(not(windows))]
/// # fn main() {}
/// ```
pub fn absolute(path: impl AsRef<SystemPath>, cwd: impl AsRef<SystemPath>) -> SystemPathBuf {
fn absolute(path: &SystemPath, cwd: &SystemPath) -> SystemPathBuf {
let path = &path.0;
let mut components = path.components().peekable();
let mut ret = if let Some(
c @ (camino::Utf8Component::Prefix(..) | camino::Utf8Component::RootDir),
) = components.peek().cloned()
{
components.next();
Utf8PathBuf::from(c.as_str())
} else {
cwd.0.to_path_buf()
};
for component in components {
match component {
camino::Utf8Component::Prefix(..) => unreachable!(),
camino::Utf8Component::RootDir => {
ret.push(component);
}
camino::Utf8Component::CurDir => {}
camino::Utf8Component::ParentDir => {
ret.pop();
}
camino::Utf8Component::Normal(c) => {
ret.push(c);
}
}
}
SystemPathBuf::from_utf8_path_buf(ret)
}
absolute(path.as_ref(), cwd.as_ref())
}
}
/// An owned, mutable path on [`System`](`super::System`) (akin to [`String`]).
@ -366,6 +454,10 @@ impl SystemPathBuf {
self.0.push(&path.as_ref().0);
}
pub fn into_utf8_path_buf(self) -> Utf8PathBuf {
self.0
}
#[inline]
pub fn as_path(&self) -> &SystemPath {
SystemPath::new(&self.0)

View file

@ -34,8 +34,8 @@ impl TestSystem {
}
}
fn use_os_system(&mut self) {
self.inner = TestFileSystem::Os(OsSystem);
fn use_os_system(&mut self, os: OsSystem) {
self.inner = TestFileSystem::Os(os);
}
}
@ -75,6 +75,13 @@ impl System for TestSystem {
}
}
fn current_directory(&self) -> &SystemPath {
match &self.inner {
TestFileSystem::Stub(fs) => fs.current_directory(),
TestFileSystem::Os(fs) => fs.current_directory(),
}
}
fn as_any(&self) -> &dyn Any {
self
}
@ -132,8 +139,8 @@ pub trait DbWithTestSystem: Db + Sized {
/// This useful for testing advanced file system features like permissions, symlinks, etc.
///
/// Note that any files written to the memory file system won't be copied over.
fn use_os_system(&mut self) {
self.test_system_mut().use_os_system();
fn use_os_system(&mut self, os: OsSystem) {
self.test_system_mut().use_os_system(os);
}
/// Returns the memory file system.