Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot]: Add a VendoredFileSystem implementation #11863

Merged
merged 43 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
41a1aff
Refactor `file` and `vendored` methods and make them standalone funct…
MichaReiser Jun 10, 2024
d3c8e83
Make `file_system_path_to_file` return an Option
MichaReiser Jun 10, 2024
1a69745
Improve documentation, rewrite inline comment, rename `file_system_pa…
MichaReiser Jun 13, 2024
43fa7bf
Update crates/ruff_db/src/vfs.rs
MichaReiser Jun 13, 2024
b054315
Don't use non-platform agnostic `is_absolute` method
MichaReiser Jun 13, 2024
383691e
Add `ModuleName` struct with tests
MichaReiser Jun 10, 2024
5a1da62
Module resolver
MichaReiser Jun 10, 2024
01041cd
Port module resolver to salsa
MichaReiser Jun 11, 2024
f16c045
Add `remove_file` and `remove_directory` tests
MichaReiser Jun 11, 2024
56f2caf
Suppress warning about unused `Os` file system that are only used on …
MichaReiser Jun 11, 2024
bcaea19
Change `ModuleName::new` to return an `Option`
MichaReiser Jun 13, 2024
4df3bea
Use `touch` over `set_permission` and `set_revision` in Tests
MichaReiser Jun 13, 2024
4441c9e
[red-knot]: Add a `VendoredFileSystem` implementation
AlexWaygood Jun 13, 2024
ae49c71
Fix many easily addressed review comments
AlexWaygood Jun 14, 2024
bd94c60
More relatively easily addressed comments
AlexWaygood Jun 14, 2024
43e3626
Just use the file hash for `FileRevision`
AlexWaygood Jun 14, 2024
13ee2d2
simplify
AlexWaygood Jun 14, 2024
05846ae
Refactor `file` and `vendored` methods and make them standalone funct…
MichaReiser Jun 10, 2024
88a6aa4
Make `file_system_path_to_file` return an Option
MichaReiser Jun 10, 2024
64b67ef
Improve documentation, rewrite inline comment, rename `file_system_pa…
MichaReiser Jun 13, 2024
cbb8992
Update crates/ruff_db/src/vfs.rs
MichaReiser Jun 13, 2024
00477c7
Add `ModuleName` struct with tests
MichaReiser Jun 10, 2024
de1ec29
Module resolver
MichaReiser Jun 10, 2024
92b3465
Port module resolver to salsa
MichaReiser Jun 11, 2024
2ee6973
Add `remove_file` and `remove_directory` tests
MichaReiser Jun 11, 2024
96d75a7
Suppress warning about unused `Os` file system that are only used on …
MichaReiser Jun 11, 2024
e5e8da6
Change `ModuleName::new` to return an `Option`
MichaReiser Jun 13, 2024
2348fe6
Use `touch` over `set_permission` and `set_revision` in Tests
MichaReiser Jun 13, 2024
eea6023
Merge branch 'salsa-memory-resolver' into vendored-filesystem
AlexWaygood Jun 14, 2024
07e5a80
Reduce clones and make them more explicit
AlexWaygood Jun 15, 2024
514e9b6
Use interior mutability to avoid cloning when unnecessary
AlexWaygood Jun 15, 2024
1f68832
Do not implement `FileSystem`
AlexWaygood Jun 16, 2024
035ea53
Simplify normalization logic
AlexWaygood Jun 17, 2024
5c33610
Update crates/ruff_db/src/vendored.rs
AlexWaygood Jun 17, 2024
ecf97a9
Get rid of unneeded methods
AlexWaygood Jun 17, 2024
be8f952
Return `Result` from `read()`
AlexWaygood Jun 17, 2024
f4f97d4
Replace a wrapper type with a type alias
AlexWaygood Jun 17, 2024
df248ce
fix tests
AlexWaygood Jun 17, 2024
f05bcb2
Bring back some notion of vendored-stub-file "metadata"
AlexWaygood Jun 17, 2024
89ebc91
Merge remote-tracking branch 'upstream/main' into vendored-filesystem
AlexWaygood Jun 18, 2024
13c2802
add some newlines
AlexWaygood Jun 18, 2024
5d511bb
More docs etc.
AlexWaygood Jun 18, 2024
3127ff6
See if bumping `zstd-sys` changes anything
AlexWaygood Jun 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactor file and vendored methods and make them standalone funct…
…ions
  • Loading branch information
MichaReiser committed Jun 14, 2024
commit 05846ae1f6cf13d90a2c3d5af7b68f6cac52e3b5
29 changes: 6 additions & 23 deletions crates/ruff_db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ use std::hash::BuildHasherDefault;
use rustc_hash::FxHasher;
use salsa::DbWithJar;

use crate::file_system::{FileSystem, FileSystemPath};
use crate::file_system::FileSystem;
use crate::parsed::parsed_module;
use crate::source::{line_index, source_text};
use crate::vfs::{VendoredPath, Vfs, VfsFile};
use crate::vfs::{Vfs, VfsFile};

pub mod file_system;
pub mod parsed;
Expand All @@ -20,25 +20,6 @@ pub struct Jar(VfsFile, source_text, line_index, parsed_module);

/// Database that gives access to the virtual filesystem, source code, and parsed AST.
pub trait Db: DbWithJar<Jar> {
/// Interns a file system path and returns a salsa `File` ingredient.
///
/// The operation is guaranteed to always succeed, even if the path doesn't exist, isn't accessible, or if the path points to a directory.
/// In these cases, a file with status [`FileStatus::Deleted`](vfs::FileStatus::Deleted) is returned.
fn file(&self, path: &FileSystemPath) -> VfsFile
where
Self: Sized,
{
self.vfs().file(self, path)
}

/// Interns a vendored file path. Returns `None` if no such vendored file exists and `Some` otherwise.
fn vendored_file(&self, path: &VendoredPath) -> Option<VfsFile>
where
Self: Sized,
{
self.vfs().vendored(self, path)
}

fn file_system(&self) -> &dyn FileSystem;

fn vfs(&self) -> &Vfs;
Expand All @@ -51,11 +32,13 @@ pub trait Upcast<T: ?Sized> {

#[cfg(test)]
mod tests {
use std::sync::Arc;

use salsa::DebugWithDb;

use crate::file_system::{FileSystem, MemoryFileSystem};
use crate::vfs::{VendoredPathBuf, Vfs};
use crate::{Db, Jar};
use salsa::DebugWithDb;
use std::sync::Arc;

/// Database that can be used for testing.
///
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_db/src/parsed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,17 @@ mod tests {
use crate::parsed::parsed_module;
use crate::tests::TestDb;
use crate::vfs::VendoredPath;
use crate::Db;
use crate::vfs::{file_system_path_to_file, vendored_path_to_file};

#[test]
fn python_file() -> crate::file_system::Result<()> {
let mut db = TestDb::new();
let path = FileSystemPath::new("test.py");
let path = "test.py";

db.file_system_mut()
.write_file(path, "x = 10".to_string())?;

let file = db.file(path);
let file = file_system_path_to_file(&db, path);

let parsed = parsed_module(&db, file);

Expand All @@ -97,7 +97,7 @@ mod tests {
db.file_system_mut()
.write_file(path, "%timeit a = b".to_string())?;

let file = db.file(path);
let file = file_system_path_to_file(&db, path);

let parsed = parsed_module(&db, file);

Expand All @@ -122,7 +122,7 @@ else:
from posixpath import __all__ as __all__"#,
)]);

let file = db.vendored_file(VendoredPath::new("path.pyi")).unwrap();
let file = vendored_path_to_file(&db, VendoredPath::new("path.pyi")).unwrap();

let parsed = parsed_module(&db, file);

Expand Down
8 changes: 4 additions & 4 deletions crates/ruff_db/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ mod tests {
use crate::file_system::FileSystemPath;
use crate::source::{line_index, source_text};
use crate::tests::TestDb;
use crate::Db;
use crate::vfs::file_system_path_to_file;

#[test]
fn re_runs_query_when_file_revision_changes() -> crate::file_system::Result<()> {
Expand All @@ -73,7 +73,7 @@ mod tests {
db.file_system_mut()
.write_file(path, "x = 10".to_string())?;

let file = db.file(path);
let file = file_system_path_to_file(&db, path);

assert_eq!(&*source_text(&db, file), "x = 10");

Expand All @@ -95,7 +95,7 @@ mod tests {
db.file_system_mut()
.write_file(path, "x = 10".to_string())?;

let file = db.file(path);
let file = file_system_path_to_file(&db, path);

assert_eq!(&*source_text(&db, file), "x = 10");

Expand All @@ -122,7 +122,7 @@ mod tests {
db.file_system_mut()
.write_file(path, "x = 10\ny = 20".to_string())?;

let file = db.file(path);
let file = file_system_path_to_file(&db, path);
let index = line_index(&db, file);
let text = source_text(&db, file);

Expand Down
52 changes: 41 additions & 11 deletions crates/ruff_db/src/vfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,39 @@ use crate::{Db, FxDashMap};

mod path;

/// Interns a file system path and returns a salsa `File` ingredient.
///
/// The operation is guaranteed to always succeed, even if the path doesn't exist, isn't accessible, or if the path points to a directory.
/// In these cases, a file with status [`FileStatus::Deleted`](vfs::FileStatus::Deleted) is returned.
#[inline]
pub fn file_system_path_to_file(db: &dyn Db, path: impl AsRef<FileSystemPath>) -> VfsFile {
db.vfs().file_system(db, path.as_ref())
}

/// Interns a vendored file path. Returns `None` if no such vendored file exists and `Some` otherwise.
#[inline]
pub fn vendored_path_to_file(db: &dyn Db, path: impl AsRef<VendoredPath>) -> Option<VfsFile> {
db.vfs().vendored(db, path.as_ref())
}

/// Interns a virtual file system path and returns a salsa `File` ingredient.
///
/// * For a [`VfsPath::FileSystem`] path, the function always returns `Some` even if the path doesn't exist,
/// isn't accessible, or if the path points to a directory.
/// In these cases, the [`VfsFile::status`] is set to [`FileStatus::Deleted`] to signify that the file isn't accessible.
///
/// * For a [`VfsPath::Vendored`] path, the function returns `Some` if a vendored file for the given
/// path exists and `None` otherwise.
///
/// See [`file_system_path_to_file`] and [`vendored_path_to_file`] if you always have either a file system or vendored path.
#[inline]
pub fn vfs_path_to_file(db: &dyn Db, path: &VfsPath) -> Option<VfsFile> {
match path {
VfsPath::FileSystem(path) => Some(file_system_path_to_file(db, path)),
VfsPath::Vendored(path) => vendored_path_to_file(db, path),
}
}

/// Virtual file system that supports files from different sources.
///
/// The [`Vfs`] supports accessing files from:
Expand Down Expand Up @@ -64,7 +97,7 @@ impl Vfs {
///
/// The operation always succeeds even if the path doesn't exist on disk, isn't accessible or if the path points to a directory.
/// In these cases, a file with status [`FileStatus::Deleted`] is returned.
pub fn file(&self, db: &dyn Db, path: &FileSystemPath) -> VfsFile {
fn file_system(&self, db: &dyn Db, path: &FileSystemPath) -> VfsFile {
*self
.inner
.files_by_path
Expand Down Expand Up @@ -95,7 +128,7 @@ impl Vfs {

/// Looks up a vendored file by its path. Returns `Some` if a vendored file for the given path
/// exists and `None` otherwise.
pub fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Option<VfsFile> {
fn vendored(&self, db: &dyn Db, path: &VendoredPath) -> Option<VfsFile> {
let file = match self
.inner
.files_by_path
Expand Down Expand Up @@ -260,10 +293,9 @@ impl VendoredVfs {

#[cfg(test)]
mod tests {
use crate::file_system::{FileRevision, FileSystemPath};
use crate::file_system::FileRevision;
use crate::tests::TestDb;
use crate::vfs::{FileStatus, VendoredPath};
use crate::Db;
use crate::vfs::{file_system_path_to_file, vendored_path_to_file, FileStatus};

#[test]
fn file_system_existing_file() -> crate::file_system::Result<()> {
Expand All @@ -272,7 +304,7 @@ mod tests {
db.file_system_mut()
.write_files([("test.py", "print('Hello world')")])?;

let test = db.file(FileSystemPath::new("test.py"));
let test = file_system_path_to_file(&db, "test.py");

assert_eq!(test.status(&db), FileStatus::Exists);
assert_eq!(test.permissions(&db), Some(0o755));
Expand All @@ -286,7 +318,7 @@ mod tests {
fn file_system_non_existing_file() {
let db = TestDb::new();

let test = db.file(FileSystemPath::new("test.py"));
let test = file_system_path_to_file(&db, "test.py");

assert_eq!(test.status(&db), FileStatus::Deleted);
assert_eq!(test.permissions(&db), None);
Expand All @@ -301,9 +333,7 @@ mod tests {
db.vfs_mut()
.stub_vendored([("test.py", "def foo() -> str")]);

let test = db
.vendored_file(VendoredPath::new("test.py"))
.expect("Vendored file to exist.");
let test = vendored_path_to_file(&db, "test.py").expect("Vendored file to exist.");

assert_eq!(test.status(&db), FileStatus::Exists);
assert_eq!(test.permissions(&db), Some(0o444));
Expand All @@ -315,6 +345,6 @@ mod tests {
fn stubbed_vendored_file_non_existing() {
let db = TestDb::new();

assert_eq!(db.vendored_file(VendoredPath::new("test.py")), None);
assert_eq!(vendored_path_to_file(&db, "test.py"), None);
}
}